[ovs-git] [openvswitch/ovs] 893b69: datapath: fix flow stats accounting when node 0 is...

GitHub noreply at github.com
Wed Apr 19 21:21:08 UTC 2017


  Branch: refs/heads/branch-2.7
  Home:   https://github.com/openvswitch/ovs
  Commit: 893b699e2ef8bdd1721f04cc6e3c3a03209e164a
      https://github.com/openvswitch/ovs/commit/893b699e2ef8bdd1721f04cc6e3c3a03209e164a
  Author: Thadeu Lima de Souza Cascardo <cascardo at redhat.com>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M datapath/flow.c
    M datapath/flow_table.c

  Log Message:
  -----------
  datapath: fix flow stats accounting when node 0 is not possible

Upstream commit:
    commit 40773966ccf1985a1b2bb570a03cbeaf1cbd4e00
    Author: Thadeu Lima de Souza Cascardo <cascardo at redhat.com>
    Date:   Thu Sep 15 19:11:52 2016 -0300

    openvswitch: fix flow stats accounting when node 0 is not possible

    On a system with only node 1 as possible, all statistics is going to be
    accounted on node 0 as it will have a single writer.

    However, when getting and clearing the statistics, node 0 is not going
    to be considered, as it's not a possible node.

    Tested that statistics are not zero on a system with only node 1
    possible. Also compile-tested with CONFIG_NUMA off.

    Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at redhat.com>
    Acked-by: Pravin B Shelar <pshelar at ovn.org>
    Signed-off-by: David S. Miller <davem at davemloft.net>

This patch contained a memory leak that is fixed in this backport.
The next patch silently fixed that in upstream, too.

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Signed-off-by: Joe Stringer <joe at ovn.org>


  Commit: 22f7190bc3e0a5e01d609f488de40fe55414239b
      https://github.com/openvswitch/ovs/commit/22f7190bc3e0a5e01d609f488de40fe55414239b
  Author: Jiri Benc <jbenc at redhat.com>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M datapath/vport.c
    M datapath/vport.h

  Log Message:
  -----------
  datapath: remove unused functions

Upstream commit:
    commit f33eb0cf9984f79e8643eaac888e4b6a06a8e221
    Author: Jiri Benc <jbenc at redhat.com>
    Date:   Wed Oct 19 11:26:36 2016 +0200

    openvswitch: remove unused functions

    ovs_vport_deferred_free is not used anywhere. It's the only caller of
    free_vport_rcu thus this one can be removed, too.

    Signed-off-by: Jiri Benc <jbenc at redhat.com>
    Acked-by: Pravin B Shelar <pshelar at ovn.org>
    Signed-off-by: David S. Miller <davem at davemloft.net>
    Signed-off-by: Jarno Rajahalme <jarno at ovn.org>

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Signed-off-by: Joe Stringer <joe at ovn.org>


  Commit: 4438ad07b03e3ec1ab339b6ed6cd218566106dac
      https://github.com/openvswitch/ovs/commit/4438ad07b03e3ec1ab339b6ed6cd218566106dac
  Author: Jiri Benc <jbenc at redhat.com>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M datapath/datapath.c
    M datapath/vport-netdev.c
    M datapath/vport.c

  Log Message:
  -----------
  datapath: remove unnecessary EXPORT_SYMBOLs

Upstream commit:
    commit 76e4cc7731a1e0c07e202999b9834f9d9be66de4
    Author: Jiri Benc <jbenc at redhat.com>
    Date:   Wed Oct 19 11:26:37 2016 +0200

    openvswitch: remove unnecessary EXPORT_SYMBOLs

    Some symbols exported to other modules are really used only by
    openvswitch.ko. Remove the exports.

    Tested by loading all 4 openvswitch modules, nothing breaks.

    Signed-off-by: Jiri Benc <jbenc at redhat.com>
    Acked-by: Pravin B Shelar <pshelar at ovn.org>
    Signed-off-by: David S. Miller <davem at davemloft.net>

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Signed-off-by: Joe Stringer <joe at ovn.org>


  Commit: 13a95373292b52d0387745fc22e7fe6a9c30c0a0
      https://github.com/openvswitch/ovs/commit/13a95373292b52d0387745fc22e7fe6a9c30c0a0
  Author: Pablo Neira Ayuso <pablo at netfilter.org>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M datapath/conntrack.c
    M datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h

  Log Message:
  -----------
  datapath: handle NF_REPEAT from nf_conntrack_in()

Upstream commit:
    commit 08733a0cb7decce40bbbd0331a0449465f13c444
    Author: Pablo Neira Ayuso <pablo at netfilter.org>
    Date:   Thu Nov 3 10:56:43 2016 +0100

    netfilter: handle NF_REPEAT from nf_conntrack_in()

    NF_REPEAT is only needed from nf_conntrack_in() under a very specific
    case required by the TCP protocol tracker, we can handle this case
    without returning to the core hook path. Handling of NF_REPEAT from the
    nf_reinject() is left untouched.

    Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org>

[Committer notes]
    Shift the functionality into the compat code, protected by v4.10
    version check. This allows the datapath/conntrack.c to match
    upstream.

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Signed-off-by: Joe Stringer <joe at ovn.org>


  Commit: d6d69fa1baa9307254ef93dac21fc17e15633443
      https://github.com/openvswitch/ovs/commit/d6d69fa1baa9307254ef93dac21fc17e15633443
  Author: Jarno Rajahalme <jarno at ovn.org>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M datapath/conntrack.c
    M tests/system-traffic.at

  Log Message:
  -----------
  datapath: Use inverted tuple in ovs_ct_find_existing() if NATted.

Upstream commit:

    commit 9ff464db50e437eef131f719cc2e9902eea9c607
    Author: Jarno Rajahalme <jarno at ovn.org>
    Date:   Thu Feb 9 11:21:53 2017 -0800

    openvswitch: Use inverted tuple in ovs_ct_find_existing() if NATted.

    The conntrack lookup for existing connections fails to invert the
    packet 5-tuple for NATted packets, and therefore fails to find the
    existing conntrack entry.  Conntrack only stores 5-tuples for incoming
    packets, and there are various situations where a lookup on a packet
    that has already been transformed by NAT needs to be made.  Looking up
    an existing conntrack entry upon executing packet received from the
    userspace is one of them.

    This patch fixes ovs_ct_find_existing() to invert the packet 5-tuple
    for the conntrack lookup whenever the packet has already been
    transformed by conntrack from its input form as evidenced by one of
    the NAT flags being set in the conntrack state metadata.

    Fixes: 05752523e565 ("openvswitch: Interface with NAT.")
    Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
    Acked-by: Joe Stringer <joe at ovn.org>
    Acked-by: Pravin B Shelar <pshelar at ovn.org>
    Signed-off-by: David S. Miller <davem at davemloft.net>

This patch also adds a test case to OVS system tests to verify the
behavior.

The following is a more thorough explanation of what is going on:

When we have evidence that an existing conntrack entry could exist, we
must invert the tuple if NAT has already been applied, as the current
packet headers do not match any tuple stored in conntrack.  For
example, if a packet from private address X to a public address B is
source-NATted to A, the conntrack entry will have the following tuples
(ignoring the protocol and port numbers) after the conntrack entry is
committed:

Original direction tuple: (X,B)
Reply direction tuple: (B,A)

Now, if a reply packet is already transformed back to the private
address space (e.g., with a CT(nat) action), the tuple corresponding
to the current packet headers is:

Current packet tuple: (B,X)

This does not match either of the conntrack tuples above.  Normally
this does not matter, as the conntrack lookup was already done using
the tuple (B,A), but if the current packet does not match any flow in
the OVS datapath, the packet is sent to userspace via an upcall,
during which the packet's skb is freed, and the conntrack entry
pointer in the skb is lost.  When the packet is reintroduced to the
datapath, any further conntrack action will need to perform a new
conntrack lookup to find the entry again.  Prior to this patch this
second lookup failed.  The datapath flow setup corresponding to the
upcall can succeed, however, allowing all further packets in the reply
direction to re-use the conntrack entry pointer in the skb, so
typically the lookup failure only causes a packet drop.

The solution is to invert the tuple derived from the current packet
headers in case the conntrack state stored in the packet metadata
indicates that the packet has been transformed by NAT:

Inverted tuple: (X,B)

With this the conntrack entry can be found, matching the original
direction tuple.

This same logic also works for the original direction packets:

Current packet tuple (after reverse NAT): (A,B)
Inverted tuple: (B,A)

While the current packet tuple (A,B) does not match either of the
conntrack tuples, the inverted one (B,A) does match the reply
direction tuple.

Since the inverted tuple matches the reverse direction tuple the
direction of the packet must be reversed as well.

Fixes: c5f6c06b58d6 ("datapath: Interface with NAT.")
Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Acked-by: Joe Stringer <joe at ovn.org>


  Commit: 657cb3807e39d930938b395bb52379bea568e87a
      https://github.com/openvswitch/ovs/commit/657cb3807e39d930938b395bb52379bea568e87a
  Author: Jarno Rajahalme <jarno at ovn.org>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M datapath/conntrack.c

  Log Message:
  -----------
  datapath: Do not trigger events for unconfirmed connections.

Upstream commit:

    commit 193e30967897f3a8b6f9f137ac30571d832c2c5c
    Author: Jarno Rajahalme <jarno at ovn.org>
    Date:   Thu Feb 9 11:21:54 2017 -0800

    openvswitch: Do not trigger events for unconfirmed connections.
    Receiving change events before the 'new' event for the connection has
    been received can be confusing.  Avoid triggering change events for
    setting conntrack mark or labels before the conntrack entry has been
    confirmed.

    Fixes: 182e3042e15d ("openvswitch: Allow matching on conntrack mark")
    Fixes: c2ac66735870 ("openvswitch: Allow matching on conntrack label")
    Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
    Acked-by: Joe Stringer <joe at ovn.org>
    Acked-by: Pravin B Shelar <pshelar at ovn.org>
    Signed-off-by: David S. Miller <davem at davemloft.net>

Upstream commit:

    commit 2317c6b51e4249dbfa093e1b88cab0a9f0564b7f
    Author: Jarno Rajahalme <jarno at ovn.org>
    Date:   Fri Feb 17 18:11:58 2017 -0800

    openvswitch: Set event bit after initializing labels.

    Connlabels are included in conntrack netlink event messages only if
    the IPCT_LABEL bit is set in the event cache (see
    ctnetlink_conntrack_event()).  Set it after initializing labels for a
    new connection.

    Found upon further system testing, where it was noticed that labels
    were missing from the conntrack events.

    Fixes: 193e30967897 ("openvswitch: Do not trigger events for unconfirmed con
nections.")
    Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
    Acked-by: Pravin B Shelar <pshelar at ovn.org>
    Signed-off-by: David S. Miller <davem at davemloft.net>

Fixes: 372ce9737d2b ("datapath: Allow matching on conntrack mark")
Fixes: 038e34abaa31 ("datapath: Allow matching on conntrack label")
Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Acked-by: Joe Stringer <joe at ovn.org>


  Commit: 3d48cf1918710aeb79c6a30fb4aee242679ad4d0
      https://github.com/openvswitch/ovs/commit/3d48cf1918710aeb79c6a30fb4aee242679ad4d0
  Author: Jarno Rajahalme <jarno at ovn.org>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M datapath/conntrack.c
    M datapath/linux/compat/include/linux/openvswitch.h

  Log Message:
  -----------
  datapath: Unionize ovs_key_ct_label with a u32 array.

Upstream commit:

    commit cb80d58fae76d8ea93555149b2b16e19b89a1f4f
    Author: Jarno Rajahalme <jarno at ovn.org>
    Date:   Thu Feb 9 11:21:55 2017 -0800

    openvswitch: Unionize ovs_key_ct_label with a u32 array.

    Make the array of labels in struct ovs_key_ct_label an union, adding a
    u32 array of the same byte size as the existing u8 array.  It is
    faster to loop through the labels 32 bits at the time, which is also
    the alignment of netlink attributes.

    Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
    Acked-by: Joe Stringer <joe at ovn.org>
    Acked-by: Pravin B Shelar <pshelar at ovn.org>
    Signed-off-by: David S. Miller <davem at davemloft.net>

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Acked-by: Joe Stringer <joe at ovn.org>


  Commit: 67a380c89a79c4441d8042abfe2b49c6957af6b4
      https://github.com/openvswitch/ovs/commit/67a380c89a79c4441d8042abfe2b49c6957af6b4
  Author: Jarno Rajahalme <jarno at ovn.org>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M datapath/conntrack.c

  Log Message:
  -----------
  datapath: Simplify labels length logic.

Upstream commit:

    commit b87cec3814ccc7f6afb0a1378ee7e5110d07cdd3
    Author: Jarno Rajahalme <jarno at ovn.org>
    Date:   Thu Feb 9 11:21:56 2017 -0800

    openvswitch: Simplify labels length logic.

    Since 23014011ba42 ("netfilter: conntrack: support a fixed size of 128
    distinct labels"), the size of conntrack labels extension has fixed to
    128 bits, so we do not need to check for labels sizes shorter than 128
    at run-time.  This patch simplifies labels length logic accordingly,
    but allows the conntrack labels size to be increased in the future
    without breaking the build.  In the event of conntrack labels
    increasing in size OVS would still be able to deal with the 128 first
    label bits.

    Suggested-by: Joe Stringer <joe at ovn.org>
    Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
    Acked-by: Pravin B Shelar <pshelar at ovn.org>
    Acked-by: Joe Stringer <joe at ovn.org>
    Signed-off-by: David S. Miller <davem at davemloft.net>

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Acked-by: Joe Stringer <joe at ovn.org>


  Commit: 9acd159291c30cc7804afd9d2d3ca525107bc492
      https://github.com/openvswitch/ovs/commit/9acd159291c30cc7804afd9d2d3ca525107bc492
  Author: Jarno Rajahalme <jarno at ovn.org>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M datapath/conntrack.c

  Log Message:
  -----------
  datapath: Refactor labels initialization.

Upstream commit:

    Refactoring conntrack labels initialization makes changes in later
    patches easier to review.

    Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
    Acked-by: Pravin B Shelar <pshelar at ovn.org>
    Acked-by: Joe Stringer <joe at ovn.org>
    Signed-off-by: David S. Miller <davem at davemloft.net>

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Acked-by: Joe Stringer <joe at ovn.org>


  Commit: 539eb6ab49740f98833288d50f0103775cf4a480
      https://github.com/openvswitch/ovs/commit/539eb6ab49740f98833288d50f0103775cf4a480
  Author: Jarno Rajahalme <jarno at ovn.org>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M datapath/conntrack.c

  Log Message:
  -----------
  datapath: Inherit master's labels.

Upstream commit:

    commit 09aa98ad496d6b11a698b258bc64d7f64c55d682
    Author: Jarno Rajahalme <jarno at ovn.org>
    Date:   Thu Feb 9 11:21:58 2017 -0800

    openvswitch: Inherit master's labels.

    We avoid calling into nf_conntrack_in() for expected connections, as
    that would remove the expectation that we want to stick around until
    we are ready to commit the connection.  Instead, we do a lookup in the
    expectation table directly.  However, after a successful expectation
    lookup we have set the flow key label field from the master
    connection, whereas nf_conntrack_in() does not do this.  This leads to
    master's labels being inherited after an expectation lookup, but those
    labels not being inherited after the corresponding conntrack action
    with a commit flag.

    This patch resolves the problem by changing the commit code path to
    also inherit the master's labels to the expected connection.
    Resolving this conflict in favor of inheriting the labels allows more
    information be passed from the master connection to related
    connections, which would otherwise be much harder if the 32 bits in
    the connmark are not enough.  Labels can still be set explicitly, so
    this change only affects the default values of the labels in presense
    of a master connection.

    Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
    Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
    Acked-by: Pravin B Shelar <pshelar at ovn.org>
    Acked-by: Joe Stringer <joe at ovn.org>
    Signed-off-by: David S. Miller <davem at davemloft.net>

Fixes: a94ebc39996b ("datapath: Add conntrack action")
Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Acked-by: Joe Stringer <joe at ovn.org>


  Commit: b45b7038f7fe80efdfcec13cd7455b31527dcced
      https://github.com/openvswitch/ovs/commit/b45b7038f7fe80efdfcec13cd7455b31527dcced
  Author: Jarno Rajahalme <jarno at ovn.org>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M datapath/conntrack.c

  Log Message:
  -----------
  datapath: Avoid struct copy on conntrack labels.

Older kernels have variable sized labels, and the struct itself
contains only the length, so we must memcpy the bits explicitly.

The modified system test fails on older kernels without this change.

[Committer Notes]
Dropped system-traffic changes as they modify a test not on branch-2.7.

VMware-BZ: #1841876
Fixes: 09aa98ad496d ("datapath: Inherit master's labels.")
Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Acked-by: Andy Zhou <azhou at ovn.org>


  Commit: 7ffcf0db87b817a8212028dc24b01aa3822e6f3a
      https://github.com/openvswitch/ovs/commit/7ffcf0db87b817a8212028dc24b01aa3822e6f3a
  Author: Jarno Rajahalme <jarno at ovn.org>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M lib/nx-match.c
    M lib/nx-match.h
    M lib/ofp-util.c

  Log Message:
  -----------
  ofp-util: Ignore unknown fields in ofputil_decode_packet_in2().

The decoder of packet_in messages should not fail on encountering
unknown metadata fields.  This allows the switch to add new features
without breaking controllers.  The controllers should, however, copy
the metadata fields from the packet_int to packet_out so that the
switch gets back the full metadata.  OVN is already doing this.

[Committer notes]

Changed mf_are_match_prereqs_ok() -> mf_are_prereqs_ok().

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Acked-by: Joe Stringer <joe at ovn.org>


  Commit: 076567f1b966947aa75ed51fd1668520f8cd6196
      https://github.com/openvswitch/ovs/commit/076567f1b966947aa75ed51fd1668520f8cd6196
  Author: Yi-Hung Wei <yihung.wei at gmail.com>
  Date:   2017-04-19 (Wed, 19 Apr 2017)

  Changed paths:
    M lib/nx-match.c
    M lib/nx-match.h
    M lib/ofp-util.c

  Log Message:
  -----------
  nx-match: Fix oxm decode.

decode_nx_packet_in2() may be used by the switch to parse NXT_RESUME messages,
where we need exact match on the oxm header. Therefore, change
oxm_decode_loose() to oxm_decode() that takes an extra argument to indicate whether
we want strict or loose match.

Fixes: 5a18e1055b88 ("ofp-util: Ignore unknown fields in ofputil_decode_packet_in2().")
Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
Signed-off-by: Joe Stringer <joe at ovn.org>


Compare: https://github.com/openvswitch/ovs/compare/41de933d95a7...076567f1b966


More information about the git mailing list