Billy O'Mahony [Tue, 1 Aug 2017 21:38:43 +0000 (14:38 -0700)]
dpif-netdev: Assign ports to pmds on non-local numa node.
Previously if there is no available (non-isolated) pmd on the numa node
for a port then the port is not polled at all. This can result in a
non-operational system until such time as nics are physically
repositioned. It is preferable to operate with a pmd on the 'wrong' numa
node albeit with lower performance. Local pmds are still chosen when
available.
Signed-off-by: Billy O'Mahony <billy.o.mahony@intel.com> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Co-authored-by: Ilya Maximets <i.maximets@samsung.com> Tested-by: Ian Stokes <ian.stokes@intel.com> Acked-by: Ian Stokes <ian.stokes@intel.com> Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Tue, 1 Aug 2017 21:22:17 +0000 (14:22 -0700)]
dpif-netdev: Don't uninit emc on reload.
There are many reasons for reloading of pmd threads:
* reconfiguration of one of the ports.
* Adjusting of static_tx_qid.
* Adding new tx/rx ports.
In many cases EMC is still useful after reload and uninit
will only lead to unnecessary upcalls/classifier lookups.
Such behaviour slows down the datapath. Uninit itself slows
down the reload path. All this factors leads to additional
unexpected latencies/drops on events not directly connected
to current PMD thread.
Lets not uninitialize emc cache on reload path.
'emc_cache_slow_sweep()' and replacements should free all
the old/unwanted entries.
Ilya Maximets [Tue, 1 Aug 2017 20:46:33 +0000 (13:46 -0700)]
dpif-netdev: Incremental addition/deletion of PMD threads.
Currently, change of 'pmd-cpu-mask' is very heavy operation.
It requires destroying of all the PMD threads and creating
them back. After that, all the threads will sleep until
ports' redistribution finished.
This patch adds ability to not stop the datapath while
adjusting number/placement of PMD threads. All not affected
threads will forward traffic without any additional latencies.
id-pool created for static tx queue ids to keep them sequential
in a flexible way. non-PMD thread will always have
static_tx_qid = 0 as it was before.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Tested-by: Mark Kavanagh <mark.b.kavanagh@intel.com> Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com> Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Wed, 2 Aug 2017 14:20:57 +0000 (19:50 +0530)]
ovn: Fix the failing "2335: ovn -- ACL logging" test case
The test case is failing mainly because of timing issue. Looking into the
ovn-controller.log it is evident that the last packet injected just before the
AT_CHECK, is still not processed by ovn-controller.
Fixes: d383eed59589 ("ovn: Add support for ACL logging.") Suggested-by: Lance Richardson <lrichard@redhat.com> Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Justin Pettit <jpettit@ovn.org>
Roi Dayan [Thu, 27 Jul 2017 11:40:02 +0000 (14:40 +0300)]
dpif-netlink-rtnl: Fix false errors on interfaces without tunnel config
When we skip adding a port using rtnetlink and not because of an error we
need to return EOPNOTSUPP to avoid logging an error message.
Fixes: 2fd3d5eda508 ("dpif-netlink-rtnl: Support layer3 GRE") Signed-off-by: Roi Dayan <roid@mellanox.com> Reviewed-by: Paul Blakey <paulb@mellanox.com> Acked-by: Eric Garver <e@erig.me> Acked-by: Simon Horman <simon.horman@netronome.com> Signed-off-by: Joe Stringer <joe@ovn.org>
Eric Garver [Tue, 1 Aug 2017 22:47:18 +0000 (18:47 -0400)]
dpif-netlink-rtnl: Fix VXLAN port create for regular VXLAN
When VXLAN-GPE was introduced we added IFLA_VXLAN_GPE to the policy
parsing, but did not mark it as optional. The kernel only returns this
netlink attribute if it's actually configured.
This also adds a missing entry for IFLA_VXLAN_GBP. Apparently we have no
system-traffic test coverage there.
Fixes: c33c412fb139 ("dpif-netlink-rtnl: Support VXLAN-GPE") Fixes: 825e45e0109f ("dpif-netlink-rtnl: Support VXLAN creation") Reported-by: Joe Stringer <joe@ovn.org> Signed-off-by: Eric Garver <e@erig.me> Signed-off-by: Joe Stringer <joe@ovn.org>
Joe Stringer [Tue, 1 Aug 2017 00:16:11 +0000 (17:16 -0700)]
ofpbuf: Fix parameter for const initializer.
Clang 4.0 complains:
In file included from include/openvswitch/cxxtest.cc:11:0:
../include/openvswitch/ofpbuf.h: In function ‘ofpbuf ofpbuf_const_initializer(const void*, size_t)’:
../include/openvswitch/ofpbuf.h:107:5: warning: narrowing conversion of ‘size’ from ‘size_t {aka long unsigned int}’ to ‘uint32_t {aka unsigned int}’ inside { } [-Wnarrowing]
};
^
../include/openvswitch/ofpbuf.h:107:5: warning: narrowing conversion of ‘size’ from ‘size_t {aka long unsigned int}’ to ‘uint32_t {aka unsigned int}’ inside { } [-Wnarrowing]
This is because the ofpbuf struct's "size" parameter is a uint32_t,
while ofpbuf_const_initializer() takes a size_t for the size. Fix this
function to take a uint32_t instead.
Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
odp-util: Remove unnecessary optimization in odp_key_to_dp_packet()
The optimization logic in odp_key_to_dp_packet() used to be useful if the
number of wanted key attributes are small. However, as the expected key
attributes increase, and the optimization logic need to check all the
netlink attributes if one of the wanted key attributes is missing, the
benefit of the optimization logic is minimal. Therefore, this patch removes
the optimization.
Suggested-by: Joe Stringer <joe@ovn.org> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
Ben Pfaff [Mon, 31 Jul 2017 19:36:48 +0000 (12:36 -0700)]
odp-util: Make checks for exact or wildcard masks more precise.
Checking whether an ODP mask is all-0s or all-1s is a little more
complicated than one might expect because the structures sometimes have
trailing padding. The function odp_mask_is_exact() was fairly careful
about this, but odp_mask_attr_is_wildcard() didn't take padding into
consideration at all, which caused test failures on Travis and on some
machines because of uninitialized padding.
This commit fixes the problem by unifying the two different functions so
that both of them are careful about checking only significant bytes. It
also adds support for the ct_orig_tuples for IPv4 and IPv6, which also
have trailing padding but weren't special cased before.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Andy Zhou <azhou@ovn.org>
Ben Pfaff [Mon, 31 Jul 2017 16:40:57 +0000 (09:40 -0700)]
odp-util: Drop special case for OVS_KEY_ATTR_TUNNEL for exact mask checks.
This special case isn't actually necessary. Commit 48954dab23ee
("odp-util: Remove last use of odp_tun_key_from_attr for formatting.")
retained it "as a safety measure" but that isn't really needed.
This makes an upcoming change more straightforward.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Andy Zhou <azhou@ovn.org>
Ben Pfaff [Fri, 28 Jul 2017 21:43:57 +0000 (14:43 -0700)]
odp-util: More carefully validate attribute length in odp_flow_format().
odp_flow_format() passes masks to odp_mask_attr_is_wildcard() without
first checking that they are the correct length. This is OK for the
moment because odp_mask_attr_is_wildcard() doesn't care that the length
is correct. An upcoming commit will change odp_mask_attr_is_wildcard()
to make it pickier, so this prepares for that change.
This adds a few comments to make it a little harder to get length
validation wrong in the future.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Andy Zhou <azhou@ovn.org>
Mark Michelson [Thu, 27 Jul 2017 18:34:23 +0000 (13:34 -0500)]
ovn: Restrict encap modification to its creating chassis
This patch extends RBAC restrictiveness of the encap table in
the ovn southbound database by only allowing modification by the
chassis that created the encap.
Signed-off-by: Mark Michelson <mmichels@redhat.com> Reported-by: Lance Richardson <lrichard@redhat.com> Acked-by: Lance Richardson <lrichard@redhat.com> Signed-off-by: Russell Bryant <russell@ovn.org>
Mark Michelson [Wed, 26 Jul 2017 21:28:12 +0000 (16:28 -0500)]
tests: Use ovn-nbctl lsp-set-dhcpvX-options
Existing OVN tests manually added DHCP options to the
Logical_Switch_Port database. There is a shortcut CLI command for doing
the same thing, so we may as well use it and get the extra test coverage
as a result.
Signed-off-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Russell Bryant <russell@ovn.org>
OVN offers a shortcut to set DHCPv4 options on a logical switch port,
but it did not offer the same for DHCPv6. This commit adds a similar
option for DHCPv6.
Signed-off-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Russell Bryant <russell@ovn.org>
Tunnel metadata was only stored if the tunnel destination was set. It's
possible, for example, that a flow could set the tunnel id field before
recirculation and then set the destination field afterwards. The
previous behavior is that the tunnel id would be lost during
recirculation under such a circumstance. This changes the behavior to
always copy the tunnel metadata, regardless of whether the tunnel
destination is set. It also adds a unit test.
Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
ofproto-dpif-rid: Store tunnel metadata in frozen metadata directly.
"recirc_id_node" contains a 'state_metadata_tunnel' member field. The
"frozen_metadata" structure used by "recird_id_node" had a 'tunnel'
member that always pointed to 'state_metadata_tunnel". This commit just
stores the tunnel information directly in "frozen_metadata" instead of
accessing it through a pointer.
This makes the code a bit simpler and easier to reason about.
Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Thu, 27 Jul 2017 23:48:54 +0000 (16:48 -0700)]
travis: Explicitly disable LLVM for sparse build.
Newer travis environments claim to have LLVM support (llvm-config exists
and works) but in reality don't, which prevents sparse from building and
later parts of the build from succeeding.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Russell Bryant [Thu, 27 Jul 2017 00:30:34 +0000 (20:30 -0400)]
Documentation/conf.py: Fix line length.
A previous commit introduced a line that was greater than 79
characters long, causing a flake8 warning to be emitted.
Reported-by: Joe Stringer <joe@ovn.org> Fixes: 5ca89127382d ("docs: Refer to correct package name for sphinx theme.") Signed-off-by: Russell Bryant <russell@ovn.org>
openvswitch: fix potential out of bound access in parse_ct
Before the 'type' is validated, we shouldn't use it to fetch the
ovs_ct_attr_lens's minlen and maxlen, else, out of bound access
may happen.
Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action") Signed-off-by: Liping Zhang <zlpnobody@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net>
Pick up an upstream bug fix.
Fixes: a94ebc39996b ("datapath: Add conntrack action") Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
Joe Stringer [Wed, 26 Jul 2017 19:49:48 +0000 (12:49 -0700)]
system-userspace-macros: Fix ethtool with new kernels.
The latest net-next kernels have removed the UFO feature, which results
in older ethtool reporting the following error:
Cannot get device udp-fragmentation-offload settings: Operation not
supported
Currently, we rely on no errors being reported, and if there is an error
then a failure is reported. However, in this case we can safely ignore
the stderr output. We still check the return code so if something is
truly fatal, a failure will still be reported; otherwise, we will not
fail the test due to the above.
Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Andy Zhou <azhou@ovn.org>
Localnet port is not an endpoint, and have no security requirements
to use localnet port at present. So, for performance consideration, we
could do not use ct for localnet port.
The more specific discussion can be found from
https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335048.html
Signed-off-by: wangqianyu <wang.qianyu@zte.com.cn> Acked-by: Han Zhou <zhouhan@gmail.com> Signed-off-by: Russell Bryant <russell@ovn.org>
Currently to check more than one patch or file it's required
to invoke script for each file separately.
Fix that by iterating over all the passed filenames.
Note: If '-f' option passed, all the files treated as usual files.
Without '-f' all the files treated as patch files.
Avoid null pointer dereference in fdb_calculate_active_tunnels()
when integration bridge isn't present. This is easily encountered
by executing "make sandbox SANDBOXFLAGS=--ovn".
Fixes: 3475695ea61c ("ovn: l3ha, enable bfd between tunnel endpoints") Signed-off-by: Lance Richardson <lrichard@redhat.com> Signed-off-by: Russell Bryant <russell@ovn.org>
Andy Zhou [Tue, 25 Jul 2017 18:28:37 +0000 (11:28 -0700)]
bond: Adjust bond hash masks
Commit 42781e77035d (bond: Unify hash functions in hash action and entry
lookup.) changed the BM_TCP's hash function, but did not update
hash mask fields accordingly. Found by inspection.
netdev-dummy: Fix setting length in recieve command.
Currently, if '--len' option passed to 'netdev-dummy/receive' command,
only 'size' field of dp_packet will changes.
This is incorrect behaviour, because memory for that size is not
allocated and also packet headers not fixed to reflect the new size.
This leads to flow_extract() failure, because it checks the
'ip->tot_len' and stops further parsing if it doesn't match the
dp_packet_size(). As a result packets created while processing of the
'receive' command can't be parsed to the same flow.
Additionally this may lead to wrong memory accesses in case someone
will try to read or modify packets data.
Fix that by creating right packets using recently introduced
'flow_compose_size()'.
CC: Andy Zhou <azhou@ovn.org> Fixes: d8ada2368cbe ("netdev-dummy: Add --len option for netdev-dummy/receive command") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Andy Zhou <azhou@ovn.org>
This allows to compose packets with different real lenghts from
odp flows i.e. memory will be allocated for requested packet
size and all required headers like ip->tot_len filled correctly.
Will be used in netdev-dummy to properly handle '--len' option.
Suggested-by: Andy Zhou <azhou@ovn.org> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Andy Zhou <azhou@ovn.org>
Mark Michelson [Fri, 21 Jul 2017 20:46:00 +0000 (15:46 -0500)]
stream-ssl: Fix memory leak in error scenario
ssl_new_stream() takes ownership of the passed-in 'name' parameter.
In error scenarios, the name is leaked. I was able to trigger this
leak by attempting to connect to an ovsdb over SSL and specifying
non-existent certificate, private key, and CA cert files.
This patch fixes the problem by freeing 'name' in the error label.
Signed-off-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Russell Bryant <russell@ovn.org>
Since introduction of 'hash_mac()' function in
commit 7e36ac42e33a ("lib/packet.h: add hash_mac()"), there is no
need to have additional wrapper for mac address hashing.
Lets use 'hash_mac()' directly and remove 'bond_hash_src()' to
simplify the code.
Suggested-by: Andy Zhou <azhou@ovn.org> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Andy Zhou <azhou@ovn.org>
bond: Unify hash functions in hash action and entry lookup.
'lookup_bond_entry' currently uses 'flow_hash_symmetric_l4' while
OVS_ACTION_ATTR_HASH uses 'flow_hash_5tuple'. This may lead to
inconsistency in slave choosing for the new flows. In general,
there is no point to unify hash functions, because it's not
required for correct work, but it's logically wrong to use
different hash functions there.
Unfortunately we're not able to use RSS hash here, because we have
no packet at this point, but we may reduce inconsistency by using
'flow_hash_5tuple' instead of 'flow_hash_symmetric_l4' because
symmetric quality is not needed.
'flow_hash_symmetric_l4' was used previously just because there
was no other implemented hash function at the moment and L2
fields was additionally involved in hash calculation. Now we
have 5tuple hash and L2 not used anymore, so, we may replace the
old function.
'flow_hash_5tuple' is preferable solution because it in 2 - 8 times
(depending on the flow) faster than symmetric function.
So, this change will also speed up handling of the new flows and
statistics accounting.
Additionally function 'bond_hash_tcp()' was removed for the reasons
of code simplification and possible additional speed up.
Co-authored-by: Andy Zhou <azhou@ovn.org> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Andy Zhou <azhou@ovn.org>
vswitch.xml: Fix L2 balancing mentioning for balance-tcp bond.
L2 fields are not used in userspace hash action since
commit 4f150744921f ("dpif-netdev: Use miniflow as a flow key.").
In kernel datapath RSS (which is not include L2 by default for
most of the NICs) was used from the beginning. This means that
if recirculation is in use, L2 fields are not used for flow
balancing.
Fix the documentation accordingly.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Andy Zhou <azhou@ovn.org>
Russell Bryant [Mon, 24 Jul 2017 20:52:30 +0000 (16:52 -0400)]
ovn-architecture: Remove outdated comment.
This outdated comment said that support for hardware gateways that
support the vtep schema would come later. This was actually
implemented a long time ago.
Signed-off-by: Russell Bryant <russell@ovn.org> Acked-by: Miguel Angel Ajo <majopela@redhat.com>
When there is an established connection in direction A->B, it is
possible to receive a packet on port B which then executes
ct(commit,force) without first performing ct() - ie, a lookup.
In this case, we would expect that this packet can delete the
existing entry so that we can commit a connection with direction B->A.
However, currently we only perform a check in skb_nfct_cached() for
whether OVS_CS_F_TRACKED is set and OVS_CS_F_INVALID is not set, ie
that a lookup previously occurred. In the above scenario, a lookup
has not occurred but we should still be able to statelessly look
up the existing entry and potentially delete the entry if it is
in the opposite direction.
This patch extends the check to also hint that if the action has the
force flag set, then we will lookup the existing entry so that the
force check at the end of skb_nfct_cached has the ability to delete
the connection.
Fixes: dd41d330b03 ("openvswitch: Add force commit.") CC: Pravin Shelar <pshelar@nicira.com> CC: dev@openvswitch.org Signed-off-by: Joe Stringer <joe@ovn.org> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Co-authored-by: Joe Stringer <joe@ovn.org> Signed-off-by: Joe Stringer <joe@ovn.org> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
openvswitch: fix mis-ordered comment lines for ovs_skb_cb
I was trying to wrap my head around meaning of mru, and realised
that the second line of the comment defining it had somehow
ended up after the line defining cutlen, leading to much confusion.
Reorder the lines to make sense.
Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
When compiling OvS-master on 4.4.0-81 kernel,
there is a warning:
CC [M] /root/ovs/datapath/linux/datapath.o
/root/ovs/datapath/linux/datapath.c: In function
'ovs_flow_cmd_set':
/root/ovs/datapath/linux/datapath.c:1221:1: warning:
the frame size of 1040 bytes is larger than 1024 bytes
[-Wframe-larger-than=]
This patch factors out match-init and action-copy to avoid
"Wframe-larger-than=1024" warning. Because mask is only
used to get actions, we new a function to save some
stack space.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
Switches and modern SR-IOV enabled NICs may multiplex traffic from Port
representators and control messages over single set of hardware queues.
Control messages and muxed traffic may need ordered delivery.
Those requirements make it hard to comfortably use TC infrastructure today
unless we have a way of attaching metadata to skbs at the upper device.
Because single set of queues is used for many netdevs stopping TC/sched
queues of all of them reliably is impossible and lower device has to
retreat to returning NETDEV_TX_BUSY and usually has to take extra locks on
the fastpath.
This patch attempts to enable port/representative devs to attach metadata
to skbs which carry port id. This way representatives can be queueless and
all queuing can be performed at the lower netdev in the usual way.
Traffic arriving on the port/representative interfaces will be have
metadata attached and will subsequently be queued to the lower device for
transmission. The lower device should recognize the metadata and translate
it to HW specific format which is most likely either a special header
inserted before the network headers or descriptor/metadata fields.
Metadata is associated with the lower device by storing the netdev pointer
along with port id so that if TC decides to redirect or mirror the new
netdev will not try to interpret it.
This is mostly for SR-IOV devices since switches don't have lower netdevs
today.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com> Signed-off-by: Simon Horman <horms@verge.net.au> Signed-off-by: David S. Miller <davem@davemloft.net>
Upstream: 3fcece12bc1b ("net: store port/representator id in metadata_dst") Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Greg Rose <gvrose8192@gmail.com>
There is no good reason to keep the flags twice in vxlan_dev and
vxlan_config.
Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net> Signed-off-by: David S. Miller <davem@davemloft.net>
Applied using HAVE_VXLAN_DEV_CFG compatibility flag defined in
acinclude.m4.
Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
Greg Rose [Fri, 21 Jul 2017 23:46:08 +0000 (16:46 -0700)]
compat: Implement upstream net device free change.
Upstream commit cf124db566e6 ("net: Fix inconsistent teardown and
release of private netdev state.") removed the destructor member
of the net_device structure and replaced it with a boolean flag
indicating that the net device resource needs freeing. Use
compat flag HAVE_NEEDS_FREE_NETDEV to indicate whether the new
flag should be used.
Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
Apply it to the tree (with one manual fixup to keep the
comment in vxlan.c, which spatch removed.)
Signed-off-by: Johannes Berg <johannes.berg@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Use e45a79da863c ("skbuff/mac80211: introduce and use skb_put_zero()")
as the basis for the backported function.
Upstream: de77b966ce8a ("networking: convert many more places to skb_put_zero()") Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Greg Rose <gvrose8192@gmail.com>
net: Fix inconsistent teardown and release of private netdev state.
Network devices can allocate reasources and private memory using
netdev_ops->ndo_init(). However, the release of these resources
can occur in one of two different places.
Either netdev_ops->ndo_uninit() or netdev->destructor().
The decision of which operation frees the resources depends upon
whether it is necessary for all netdev refs to be released before it
is safe to perform the freeing.
netdev_ops->ndo_uninit() presumably can occur right after the
NETDEV_UNREGISTER notifier completes and the unicast and multicast
address lists are flushed.
netdev->destructor(), on the other hand, does not run until the
netdev references all go away.
Further complicating the situation is that netdev->destructor()
almost universally does also a free_netdev().
This creates a problem for the logic in register_netdevice().
Because all callers of register_netdevice() manage the freeing
of the netdev, and invoke free_netdev(dev) if register_netdevice()
fails.
If netdev_ops->ndo_init() succeeds, but something else fails inside
of register_netdevice(), it does call ndo_ops->ndo_uninit(). But
it is not able to invoke netdev->destructor().
This is because netdev->destructor() will do a free_netdev() and
then the caller of register_netdevice() will do the same.
However, this means that the resources that would normally be released
by netdev->destructor() will not be.
Over the years drivers have added local hacks to deal with this, by
invoking their destructor parts by hand when register_netdevice()
fails.
Many drivers do not try to deal with this, and instead we have leaks.
Let's close this hole by formalizing the distinction between what
private things need to be freed up by netdev->destructor() and whether
the driver needs unregister_netdevice() to perform the free_netdev().
netdev->priv_destructor() performs all actions to free up the private
resources that used to be freed by netdev->destructor(), except for
free_netdev().
netdev->needs_free_netdev is a boolean that indicates whether
free_netdev() should be done at the end of unregister_netdevice().
Now, register_netdevice() can sanely release all resources after
ndo_ops->ndo_init() succeeds, by invoking both ndo_ops->ndo_uninit()
and netdev->priv_destructor().
And at the end of unregister_netdevice(), we invoke
netdev->priv_destructor() and optionally call free_netdev().
Signed-off-by: David S. Miller <davem@davemloft.net>
Applied the portion of the commit applicable to openvswitch.
Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
openvswitch: more accurate checksumming in queue_userspace_packet()
if skb carries an SCTP packet and ip_summed is CHECKSUM_PARTIAL, it needs
CRC32c in place of Internet Checksum: use skb_csum_hwoffload_help to avoid
corrupting such packets while queueing them towards userspace.
Signed-off-by: Davide Caratti <dcaratti@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Joe Stringer <joe@ovn.org>
netfilter: introduce nf_conntrack_helper_put helper function
And convert module_put invocation to nf_conntrack_helper_put, this is
prepared for the followup patch, which will add a refcnt for cthelper,
so we can reject the deleting request when cthelper is in use.
Signed-off-by: Liping Zhang <zlpnobody@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Applied with additional use of HAVE_NF_CONNTRACK_HELPER_PUT compatibility
flag defined in acinclude.m4.
Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
tests: Check ip command whether support udp6zerocsum.
The version of ip-route may not support udp6zerocsum for
vxlan6 or geneve6. If we run the kernel check, there may
be always error message. Before running the test units,
we check the ip command.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Acked-by: Eric Garver <e@erig.me> Signed-off-by: Joe Stringer <joe@ovn.org>
System-tests: Improve reliability of an icmp test.
One SNAT test is based on a single ping being successful;
to make the result more predictable, static arp binding is now used.
To put less stress on the stack a single arp binding is used for
the reverse direction mapping. This does not change the goal of the
test, but significantly increases the reliability; I ran the test
100 times without failure.
Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
Three of the SNAT tests allow for wget retries, which occasionally
happen. However, these tests did not allow for SNAT address
variability for the retries, which is now tolerated.
Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
Russell Bryant [Sun, 16 Jul 2017 19:39:56 +0000 (15:39 -0400)]
docs: Note currently used L3 gateway HA approach.
The OVN gateway HA design document is very useful in its current form.
It describes a range of options OVN could take to provide gateway HA.
Leave all the useful discussion in place and add a note to indicate
how the current implementation lines up with the options described.
I plan to follow up with an additional patch to describe the current L3
gateway HA implementation in the ovn-architecture document.
Signed-off-by: Russell Bryant <russell@ovn.org> Acked-by: Miguel Angel Ajo <majopela@redhat.com>
Ovs and kernel stack would add frag_queue to same netns_frags list.
As result, ovs and kernel may access the fraq_queue without correct
lock. Also the struct ipq may be different on kernel(older than 4.3),
which leads to invalid pointer access.
The fix creates specific netns_frags for ovs.
Signed-off-by: wangzhike <wangzhike@jd.com> Signed-off-by: Joe Stringer <joe@ovn.org>
rhel/systemd: Set ovs-vswitchd timeout to 5 minutes
During initialization, it's possible that the startup time takes longer
than the systemd default provided. Set this to be 5 minutes. If we
take longer than 5 minutes, maybe something is wrong.
As an example of long initialization, enable DPDK, and allocate large
numbers of hugepages before starting ovs-vswitchd. The vswitchd can
take two or more minutes to start. During that time, systemd will decide
that the startup time took too long, and kill the parent process, leading
eventually to an error like:
ovs|00011|daemon_unix|EMER|pipe write failed (Broken pipe)
And a systemd log like:
ovs-vswitchd.service start operation timed out. Terminating.
The 5 minutes setting has been observed to work on a system where 400G
of hugepages were allocated.
Russell Bryant [Sun, 16 Jul 2017 20:07:12 +0000 (16:07 -0400)]
ovn-architecture: Add notes on L3 gateway HA.
Add some comments to the ovn-architecture document that distributed
gateway ports can also be made highly available. Provide a brief
overview of the approach and point to the gateway HA design document
for a more detailed discussion of the approach taken.
Signed-off-by: Russell Bryant <russell@ovn.org> Acked-by: Miguel Angel Ajo <majopela@redhat.com>
odp-execute: Reuse rss hash in OVS_ACTION_ATTR_HASH.
If RSS hash exists in a packet it can be reused instead of
5 tuple hash re-calculation in OVS_ACTION_ATTR_HASH. This
leads to increasing the performance of sending packets to
the OVS bonding in userspace datapath up to 10-15%.
Additionally fixed unit test 'select group with dp_hash
selection method' to not depend on dp_hash value.
dpif-netdev: Indicate support for various ct features.
The userspace datapath uses a structure to indicate supported features
that affects debug output. This commit updates that structure to
indicate that "ct_state_nat", "ct_orig_tuple", and "ct_orig_tuple6" are
supported.
tunneling: Avoid datapath-recirc by combining recirc actions at xlate.
This patch set removes the recirculation of encapsulated tunnel packets
if possible. It is done by computing the post tunnel actions at the time of
translation. The combined nested action set are programmed in the datapath
using CLONE action.
The following test results shows the performance improvement offered by
this optimization for tunnel encap.
tunneling: Calculate and update packet l4_offset in tunnel push.
The following tunnel combine patch series avoids the packets recirculation
after the tunnel push. So it is necessary to populate all relevant packet meta
data fields for the following combined action-set.
In this eg: the last tunnel_pop operation uses the l4_offset in the packet to
validate the packets. So it must be calculated and updated in the packet before
executing the action. Since there is no recirculation now on, this calculation
is doing as part of tunnel_push.
Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com> Signed-off-by: Zoltán Balogh <zoltan.balogh@ericsson.com> Co-authored-by: Zoltán Balogh <zoltan.balogh@ericsson.com> Signed-off-by: Joe Stringer <joe@ovn.org>
xlate: Clear tunnel mask along with other fields while combine actions.
The tunnel mask in the translation state should be cleared along with other
context fields. It is necessary in 'apply_nested_clone_actions' as it will be
used to combine post tunnel output actions with tunnel push. This will assure
right openflow state while executing the translation.
Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com> Signed-off-by: Zoltán Balogh <zoltan.balogh@ericsson.com> Co-authored-by: Zoltán Balogh <zoltan.balogh@ericsson.com> Signed-off-by: Joe Stringer <joe@ovn.org>
Joe Stringer [Tue, 18 Jul 2017 22:32:44 +0000 (15:32 -0700)]
dpif-netlink: For non-Ethernet, use Ethertype from packet_type.
For non-Ethernet flows, when fixing up the netlink message we need make
sure to pass down a valid Ethertype. The kernel does not understand
packet_type so it's implicitly encoded by the absence of _ETHERNET and
exact match of _ETHERTYPE. Without this change match_validate in the
kernel complains when trying to match packets from L3 tunnels.
e.g.
openvswitch: netlink: Unexpected mask (mask=110088, allowed=3d9804c)
The mask use to always be set in xlate_wc_init() and xlate_wc_finish(),
but that changed for non-Ethernet frames with the commit listed below.
Fixes: 3d4b2e6eb74e ("userspace: Add OXM field MFF_PACKET_TYPE") Signed-off-by: Joe Stringer <joe@ovn.org> Co-authored-by: Eric Garver <e@erig.me> Acked-by: Eric Garver <e@erig.me>
Joe Stringer [Tue, 18 Jul 2017 22:32:43 +0000 (15:32 -0700)]
dpif-netlink: Use netlink helpers for packet_type.
Rather than open-coding access to netlink attribute pointers in
put_exclude_packet_type(), make use of the netlink attribute helpers.
This simplifies the following bugfix.
Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Eric Garver <e@erig.me>
Yang, Yi Y [Fri, 7 Jul 2017 03:02:13 +0000 (11:02 +0800)]
datapath: enable VxLAN-gpe port creation in compat mode
In compat mode, ovs can't create L3 VxLAN-gpe port in old
kernels if port creation failed by rtnetlink, this patch
enables old kernels to create L3 VxLAN-gpe port.
Signed-off-by: Yi Yang <yi.y.yang@intel.com> Signed-off-by: Joe Stringer <joe@ovn.org>
odp-util: Document size of OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4.
This attribute is exclusive of OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6 so it
doesn't take up additional space (IPv6 is larger), but it's still worth
documenting.
Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Joe Stringer <joe@ovn.org>
projects / ovs.git / log