Re: [ovs-dev] [PATCH] vswitchd: Add 'appctl quit' command

[Andy Zhou]

On Tue, Apr 25, 2017 at 8:43 PM, Ben Pfaff  wrote:
> On Tue, Apr 25, 2017 at 05:02:14PM -0700, Andy Zhou wrote:
>> vswitchd can gracefully shutdown after receiving the 'appctl exit'
>> command.  But the datapath resources vswitchd created can
>> linger on. This is not a problem, and in fact desireable when the
>> shutdown is transient in nature. Since restarted vswitchd usually
>> sees a similar configuration. By keeping the datapath resources
>> minimize the perturbation to the running data traffic.
>>
>> However, when vswitchd shutdown is permanent, there should be a
>> way to release the all datapath resources, so that they won't
>> be linger on forever.  Add 'appctl quit' command for such purpose.
>>
>> Signed-off-by: Andy Zhou 
>
> Thanks for working on this.
>
> I have two categories of comments here.  The first category is a general
> kind of unease over what problems this solves.  I think it's better if
> we don't have to make the distinction between two ways to exit, and so
> I'd like to hear more specifically about the problems that this solves.

The specific use case that triggered this patch was a user wanted to
shut down OVS in the
root name space, then restart OVS in a specific name space with the
same or very similar configuration.
They found that having the same bridge and ports existing in both the
root name space
and another name space to be confusing. They are also concerned about potential
kernel resource leak in the root name space.

They could have solved this by explicitly remove all bridges before
stopping OVS services, but they chose not to rely on this approach (I don't know
the details). They are mostly interested in having a whole sale way
of releasing all kernel resources. Ideally, they'd like this to be provided via
the the distribution scripts.

They worked around this issue by issuing the 'ovs-dpctl del-dp
ovs-system' command
after stopping OVS services. It solves their problem at hand.

While 'ovs-dpctl del-dp ovs-system' will remove all resources known to
OVS kernel module,
OVS userspace may create additional kernel resources outside the OVS
kernel module
down the road, For example:

- light weight tunnel devices created by rtnetlink messages.
- (future) reatlimiter created by NFT netlink messages.
- CT/NAT states
- TC Queues.

Given that, It seems a better model would be for vswitchd to manage DP
resource deletion,
although this patch does not address most, if at all, of the issues
listed above, this
command can be expanded down the road.

What do you think? I am definitely certainly open any comments and suggestions.

>
> The second category is just about naming.  The two names "quit" and
> "exit" don't suggest different operations and so I think that they are
> likely to lead to confusion.  I know that I, personally, will never
> remember which is which.  I'd suggest, instead, making the existing
> "exit" take an option to explain what kind of shutdown is desired.
>

That's a fair point. May be we can add an option to exit, e.g. 'appctl
exit  clean-dp'?
> Thanks,
>
> Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] vswitchd: Add 'appctl quit' command

[Ben Pfaff]

On Tue, Apr 25, 2017 at 05:02:14PM -0700, Andy Zhou wrote:
> vswitchd can gracefully shutdown after receiving the 'appctl exit'
> command.  But the datapath resources vswitchd created can
> linger on. This is not a problem, and in fact desireable when the
> shutdown is transient in nature. Since restarted vswitchd usually
> sees a similar configuration. By keeping the datapath resources
> minimize the perturbation to the running data traffic.
> 
> However, when vswitchd shutdown is permanent, there should be a
> way to release the all datapath resources, so that they won't
> be linger on forever.  Add 'appctl quit' command for such purpose.
> 
> Signed-off-by: Andy Zhou 

Thanks for working on this.

I have two categories of comments here.  The first category is a general
kind of unease over what problems this solves.  I think it's better if
we don't have to make the distinction between two ways to exit, and so
I'd like to hear more specifically about the problems that this solves.

The second category is just about naming.  The two names "quit" and
"exit" don't suggest different operations and so I think that they are
likely to lead to confusion.  I know that I, personally, will never
remember which is which.  I'd suggest, instead, making the existing
"exit" take an option to explain what kind of shutdown is desired.

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] vswitchd: Add 'appctl quit' command

[Greg Rose]

On Tue, 2017-04-25 at 17:02 -0700, Andy Zhou wrote:
> vswitchd can gracefully shutdown after receiving the 'appctl exit'
> command.  But the datapath resources vswitchd created can
> linger on. This is not a problem, and in fact desireable when the
> shutdown is transient in nature. Since restarted vswitchd usually
> sees a similar configuration. By keeping the datapath resources
> minimize the perturbation to the running data traffic.
> 
> However, when vswitchd shutdown is permanent, there should be a
> way to release the all datapath resources, so that they won't
> be linger on forever.  Add 'appctl quit' command for such purpose.

Otherwise looks fine to me but s/quiting/quitting/

Thanks,

- Greg

> 
> Signed-off-by: Andy Zhou 
> ---
>  NEWS   |  1 +
>  ofproto/ofproto-dpif.c | 11 +++
>  ofproto/ofproto-provider.h |  2 +-
>  ofproto/ofproto.c  |  2 +-
>  vswitchd/bridge.c  |  4 ++--
>  vswitchd/bridge.h  |  4 +++-
>  vswitchd/ovs-vswitchd.8.in |  3 +++
>  vswitchd/ovs-vswitchd.c|  7 ---
>  8 files changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index ea97d84a2dea..3f65654f5124 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -26,6 +26,7 @@ Post-v2.7.0
>   * Bundles now support hashing by just nw_src or nw_dst.
>   * The "learn" action now supports a "limit" option (see ovs-ofctl(8)).
>   * The port status bit OFPPS_LIVE now reflects link aliveness.
> +   - Add the command 'ovs-appctl quit' (see ovs-vswitchd(8)).
>  
>  v2.7.0 - 21 Feb 2017
>  -
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index c73c2738c91c..bd2eaa60d36b 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -645,7 +645,7 @@ dealloc(struct ofproto *ofproto_)
>  }
>  
>  static void
> -close_dpif_backer(struct dpif_backer *backer)
> +close_dpif_backer(struct dpif_backer *backer, bool del)
>  {
>  ovs_assert(backer->refcount > 0);
>  
> @@ -661,6 +661,9 @@ close_dpif_backer(struct dpif_backer *backer)
>  shash_find_and_delete(_dpif_backers, backer->type);
>  free(backer->type);
>  free(backer->dp_version_string);
> +if (del) {
> +dpif_delete(backer->dpif);
> +}
>  dpif_close(backer->dpif);
>  free(backer);
>  }
> @@ -772,7 +775,7 @@ open_dpif_backer(const char *type, struct dpif_backer 
> **backerp)
>  if (error) {
>  VLOG_ERR("failed to listen on datapath of type %s: %s",
>   type, ovs_strerror(error));
> -close_dpif_backer(backer);
> +close_dpif_backer(backer, false);
>  return error;
>  }
>  
> @@ -1452,7 +1455,7 @@ add_internal_flows(struct ofproto_dpif *ofproto)
>  }
>  
>  static void
> -destruct(struct ofproto *ofproto_)
> +destruct(struct ofproto *ofproto_, bool del)
>  {
>  struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
>  struct ofproto_async_msg *am;
> @@ -1505,7 +1508,7 @@ destruct(struct ofproto *ofproto_)
>  
>  seq_destroy(ofproto->ams_seq);
>  
> -close_dpif_backer(ofproto->backer);
> +close_dpif_backer(ofproto->backer, del);
>  }
>  
>  static int
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index b7b12cdfd5f4..ef993d0afc4d 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -828,7 +828,7 @@ struct ofproto_class {
>   */
>  struct ofproto *(*alloc)(void);
>  int (*construct)(struct ofproto *ofproto);
> -void (*destruct)(struct ofproto *ofproto);
> +void (*destruct)(struct ofproto *ofproto, bool del);
>  void (*dealloc)(struct ofproto *ofproto);
>  
>  /* Performs any periodic activity required by 'ofproto'.  It should:
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index ca0f3e49bd67..7bc7b7f99d0d 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -1651,7 +1651,7 @@ ofproto_destroy(struct ofproto *p, bool del)
>  free(usage);
>  }
>  
> -p->ofproto_class->destruct(p);
> +p->ofproto_class->destruct(p, del);
>  
>  /* We should not postpone this because it involves deleting a listening
>   * socket which we may want to reopen soon. 'connmgr' may be used by 
> other
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index ebb6249416fa..e741f34f19ec 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -496,14 +496,14 @@ bridge_init(const char *remote)
>  }
>  
>  void
> -bridge_exit(void)
> +bridge_exit(bool delete_datpath)
>  {
>  struct bridge *br, *next_br;
>  
>  if_notifier_destroy(ifnotifier);
>  seq_destroy(ifaces_changed);
>  HMAP_FOR_EACH_SAFE (br, next_br, node, _bridges) {
> -bridge_destroy(br, false);
> +bridge_destroy(br, delete_datpath);
>  }
>  ovsdb_idl_destroy(idl);
>  }
> diff --git a/vswitchd/bridge.h b/vswitchd/bridge.h
> index 3783a21e3b4c..7835611568cf 100644
> --- a/vswitchd/bridge.h
> +++ b/vswitchd/bridge.h
> @@ -16,10 

[ovs-dev] [PATCH v2 4/5] datapath: Fix ovs_flow_key_update()

[Yi-Hung Wei]

Upstream commit:
commit 6f56f6186c18e3fd54122b73da68e870687b8c59
Author: Yi-Hung Wei 
Date:   Thu Mar 30 12:36:03 2017 -0700

ovs_flow_key_update() is called when the flow key is invalid, and it is
used to update and revalidate the flow key. Commit 329f45bc4f19
("openvswitch: add mac_proto field to the flow key") introduces mac_proto
field to flow key and use it to determine whether the flow key is valid.
However, the commit does not update the code path in ovs_flow_key_update()
to revalidate the flow key which may cause BUG_ON() on execute_recirc().
This patch addresses the aforementioned issue.

Fixes: 329f45bc4f19 ("openvswitch: add mac_proto field to the flow key")
Signed-off-by: Yi-Hung Wei 
Acked-by: Jiri Benc 
Signed-off-by: David S. Miller 

Signed-off-by: Yi-Hung Wei 
Acked-by: Simon Horman 
---
 datapath/flow.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/datapath/flow.c b/datapath/flow.c
index 1d40b2400406..c4f63b0b5d76 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -531,7 +531,7 @@ static int key_extract(struct sk_buff *skb, struct 
sw_flow_key *key)
 
/* Link layer. */
clear_vlan(key);
-   if (key->mac_proto == MAC_PROTO_NONE) {
+   if (ovs_key_mac_proto(key) == MAC_PROTO_NONE) {
if (unlikely(eth_type_vlan(skb->protocol)))
return -EINVAL;
 
@@ -751,7 +751,13 @@ static int key_extract(struct sk_buff *skb, struct 
sw_flow_key *key)
 
 int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key)
 {
-   return key_extract(skb, key);
+   int res;
+
+   res = key_extract(skb, key);
+   if (!res)
+   key->mac_proto &= ~SW_FLOW_KEY_INVALID;
+
+   return res;
 }
 
 static int key_extract_mac_proto(struct sk_buff *skb)
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 5/5] system-traffic: Add test for mpls actions

[Yi-Hung Wei]

Add ping test to verify the behavior of mpls_push/pop actions. In this
test, we use the resubmit action to trigger recirulation for making sure
the flow key is revalidated after mpls_push/pop. This test depends on
commit 5ba0c107c51e ("datapath: Fix ovs_flow_key_update()") to behave
correctly.

Signed-off-by: Yi-Hung Wei 
Acked-by: Simon Horman 
---
 tests/system-traffic.at | 36 
 1 file changed, 36 insertions(+)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index c04277351f31..26b93edf6392 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -369,6 +369,42 @@ 
icmp,vlan_tci=0x,dl_src=ae:c6:7e:54:8d:4d,dl_dst=50:54:00:00:00:0b,nw_src=10
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([datapath - mpls actions])
+OVS_TRAFFIC_VSWITCHD_START([_ADD_BR([br1])])
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p1, at_ns1, br1, "10.1.1.2/24")
+
+AT_CHECK([ip link add patch0 type veth peer name patch1])
+on_exit 'ip link del patch0'
+
+AT_CHECK([ip link set dev patch0 up])
+AT_CHECK([ip link set dev patch1 up])
+AT_CHECK([ovs-vsctl add-port br0 patch0])
+AT_CHECK([ovs-vsctl add-port br1 patch1])
+
+AT_DATA([flows.txt], [dnl
+table=0,priority=100,dl_type=0x0800 
actions=push_mpls:0x8847,set_mpls_label:3,resubmit(,1)
+table=0,priority=100,dl_type=0x8847,mpls_label=3 
actions=pop_mpls:0x0800,resubmit(,1)
+table=0,priority=10 actions=resubmit(,1)
+table=1,priority=10 actions=normal
+])
+
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+AT_CHECK([ovs-ofctl add-flows br1 flows.txt])
+
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], 
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 -w 2 10.1.1.1 | FORMAT_PING], 
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
 AT_SETUP([datapath - basic truncate action])
 AT_SKIP_IF([test $HAVE_NC = no])
 OVS_TRAFFIC_VSWITCHD_START()
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 2/5] datapath: set network header correctly on key extract

[Yi-Hung Wei]

Upstream commit:
commit f7d49bce8e741e1e6aa14ce4db1b6cea7e4be4e8
Author: Jiri Benc 
Date:   Fri Sep 30 19:08:05 2016 +0200

openvswitch: mpls: set network header correctly on key extract

After the 48d2ab609b6b ("net: mpls: Fixups for GSO"), MPLS handling in
openvswitch was changed to have network header pointing to the start of the
MPLS headers and inner_network_header pointing after the MPLS headers.

However, key_extract was missed by the mentioned commit, causing incorrect
headers to be set when a MPLS packet just enters the bridge or after it is
recirculated.

Fixes: 48d2ab609b6b ("net: mpls: Fixups for GSO")
Signed-off-by: Jiri Benc 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Signed-off-by: Yi-Hung Wei 
---
 datapath/flow.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/datapath/flow.c b/datapath/flow.c
index 2bc1ad0d5304..1d40b2400406 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -666,12 +666,7 @@ static int key_extract(struct sk_buff *skb, struct 
sw_flow_key *key)
} else if (eth_p_mpls(key->eth.type)) {
size_t stack_len = MPLS_HLEN;
 
-   /* In the presence of an MPLS label stack the end of the L2
-* header and the beginning of the L3 header differ.
-*
-* Advance network_header to the beginning of the L3
-* header. mac_len corresponds to the end of the L2 header.
-*/
+   skb_set_inner_network_header(skb, skb->mac_len);
while (1) {
__be32 lse;
 
@@ -679,12 +674,12 @@ static int key_extract(struct sk_buff *skb, struct 
sw_flow_key *key)
if (unlikely(error))
return 0;
 
-   memcpy(, skb_network_header(skb), MPLS_HLEN);
+   memcpy(, skb_inner_network_header(skb), MPLS_HLEN);
 
if (stack_len == MPLS_HLEN)
memcpy(>mpls.top_lse, , MPLS_HLEN);
 
-   skb_set_network_header(skb, skb->mac_len + stack_len);
+   skb_set_inner_network_header(skb, skb->mac_len + 
stack_len);
if (lse & htonl(MPLS_LS_S_MASK))
break;
 
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 3/5] datapath: correctly fragment packet with mpls headers

[Yi-Hung Wei]

Upstream commit:
commit c66549ffd05831abf6cf19ce0571ad868e39
Author: Jiri Benc 
Date:   Wed Oct 5 15:01:57 2016 +0200

openvswitch: correctly fragment packet with mpls headers

If mpls headers were pushed to a defragmented packet, the refragmentation no
longer works correctly after 48d2ab609b6b ("net: mpls: Fixups for GSO"). The
network header has to be shifted after the mpls headers for the
fragmentation and restored afterwards.

Fixes: 48d2ab609b6b ("net: mpls: Fixups for GSO")
Signed-off-by: Jiri Benc 
Signed-off-by: David S. Miller 

Signed-off-by: Yi-Hung Wei 
---
 datapath/actions.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index 966df03f4f82..b2f790c2b35d 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -62,7 +62,8 @@ struct ovs_frag_data {
struct vport *vport;
struct ovs_gso_cb cb;
__be16 inner_protocol;
-   __u16 vlan_tci;
+   u16 network_offset; /* valid only for MPLS */
+   u16 vlan_tci;
__be16 vlan_proto;
unsigned int l2_len;
u8 mac_proto;
@@ -740,6 +741,12 @@ static int ovs_vport_output(OVS_VPORT_OUTPUT_PARAMS)
skb_postpush_rcsum(skb, skb->data, data->l2_len);
skb_reset_mac_header(skb);
 
+   if (eth_p_mpls(skb->protocol)) {
+   skb->inner_network_header = skb->network_header;
+   skb_set_network_header(skb, data->network_offset);
+   skb_reset_mac_len(skb);
+   }
+
ovs_vport_send(vport, skb, data->mac_proto);
return 0;
 }
@@ -759,7 +766,7 @@ static struct dst_ops ovs_dst_ops = {
  * ovs_vport_output(), which is called once per fragmented packet.
  */
 static void prepare_frag(struct vport *vport, struct sk_buff *skb,
-   u8 mac_proto)
+u16 orig_network_offset, u8 mac_proto)
 {
unsigned int hlen = skb_network_offset(skb);
struct ovs_frag_data *data;
@@ -769,6 +776,7 @@ static void prepare_frag(struct vport *vport, struct 
sk_buff *skb,
data->vport = vport;
data->cb = *OVS_GSO_CB(skb);
data->inner_protocol = ovs_skb_get_inner_protocol(skb);
+   data->network_offset = orig_network_offset;
data->vlan_tci = skb->vlan_tci;
data->vlan_proto = skb->vlan_proto;
data->mac_proto = mac_proto;
@@ -783,6 +791,13 @@ static void ovs_fragment(struct net *net, struct vport 
*vport,
 struct sk_buff *skb, u16 mru,
 struct sw_flow_key *key)
 {
+   u16 orig_network_offset = 0;
+
+   if (eth_p_mpls(skb->protocol)) {
+   orig_network_offset = skb_network_offset(skb);
+   skb->network_header = skb->inner_network_header;
+   }
+
if (skb_network_offset(skb) > MAX_L2_LEN) {
OVS_NLERR(1, "L2 header too long to fragment");
goto err;
@@ -792,7 +807,8 @@ static void ovs_fragment(struct net *net, struct vport 
*vport,
struct dst_entry ovs_dst;
unsigned long orig_dst;
 
-   prepare_frag(vport, skb, ovs_key_mac_proto(key));
+   prepare_frag(vport, skb, orig_network_offset,
+ ovs_key_mac_proto(key));
dst_init(_dst, _dst_ops, NULL, 1,
 DST_OBSOLETE_NONE, DST_NOCOUNT);
ovs_dst.dev = vport->dev;
@@ -811,7 +827,7 @@ static void ovs_fragment(struct net *net, struct vport 
*vport,
if (!v6ops)
goto err;
 
-   prepare_frag(vport, skb,
+   prepare_frag(vport, skb, orig_network_offset,
 ovs_key_mac_proto(key));
memset(_rt, 0, sizeof(ovs_rt));
dst_init(_rt.dst, _dst_ops, NULL, 1,
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 1/5] datapath: Fixups for MPLS GSO

[Yi-Hung Wei]

This patch backports the following two upstream commits to fix MPLS GSO in
ovs datapath. Starting from upstream commit 48d2ab609b6b ("net: mpls: Fixups
for GSO"), the mpls_gso kernel module relies on the fact that
skb_network_header() points to the mpls header and skb_inner_network_header()
points to the L3 header so that it can derive the length of mpls header
correctly, and the upstream commit updates how ovs datapath marks the skb
header when push and pop mpls. However, the old mpls_gso kernel module
assumes that the skb_network_header() points to the L3 header, and the old
mpls_gso kernel module will misbehave if the ovs datapath marks the
skb_network_header() in the new way since it will treat mpls header as the L3
header.

Because of the functional signature of mpls_gso_segment() does not change,
this backport patch uses the new mpls_hdr() to determine if the kernel that
ovs datapath is compiled with has the new or legacy mpls_gso kernel module.
It has been tested on kernel 4.4 and 4.9.

Upstream commit:
commit 48d2ab609b6bbecb7698487c8579bc40de9d6dfa
Author: David Ahern 
Date:   Wed Aug 24 20:10:44 2016 -0700

net: mpls: Fixups for GSO

As reported by Lennert the MPLS GSO code is failing to properly segment
large packets. There are a couple of problems:

1. the inner protocol is not set so the gso segment functions for inner
   protocol layers are not getting run, and

2  MPLS labels for packets that use the "native" (non-OVS) MPLS code
   are not properly accounted for in mpls_gso_segment.

The MPLS GSO code was added for OVS. It is re-using skb_mac_gso_segment
to call the gso segment functions for the higher layer protocols. That
means skb_mac_gso_segment is called twice -- once with the network
protocol set to MPLS and again with the network protocol set to the
inner protocol.

This patch sets the inner skb protocol addressing item 1 above and sets
the network_header and inner_network_header to mark where the MPLS labels
start and end. The MPLS code in OVS is also updated to set the two
network markers.

>From there the MPLS GSO code uses the difference between the network
header and the inner network header to know the size of the MPLS header
that was pushed. It then pulls the MPLS header, resets the mac_len and
protocol for the inner protocol and then calls skb_mac_gso_segment
to segment the skb.

Afterward the inner protocol segmentation is done the skb protocol
is set to mpls for each segment and the network and mac headers
restored.

Reported-by: Lennert Buytenhek 
Signed-off-by: David Ahern 
Signed-off-by: David S. Miller 

Upstream commit:
commit 85de4a2101acb85c3b1dde465e84596ccca99f2c
Author: Jiri Benc 
Date:   Fri Sep 30 19:08:07 2016 +0200

openvswitch: use mpls_hdr

skb_mpls_header is equivalent to mpls_hdr now. Use the existing helper
instead.

Signed-off-by: Jiri Benc 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Signed-off-by: Yi-Hung Wei 
---
 acinclude.m4 |  1 +
 datapath/actions.c   | 33 +++-
 datapath/linux/compat/include/net/mpls.h | 23 --
 3 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 9f8e30d9b47a..b75fe979773a 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -479,6 +479,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
 [OVS_GREP_IFELSE([$KSRC/include/net/dst_cache.h], 
[dst_cache],
  
[OVS_DEFINE([USE_UPSTREAM_TUNNEL])])])])
 
+  OVS_GREP_IFELSE([$KSRC/include/net/mpls.h], [mpls_hdr])
   OVS_GREP_IFELSE([$KSRC/include/linux/net.h], [sock_create_kern.*net],
   [OVS_DEFINE([HAVE_SOCK_CREATE_KERN_NET])])
   OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [ndo_fill_metadata_dst])
diff --git a/datapath/actions.c b/datapath/actions.c
index 75f17709aec0..966df03f4f82 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -186,7 +186,7 @@ static void update_ethertype(struct sk_buff *skb, struct 
ethhdr *hdr,
 static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 const struct ovs_action_push_mpls *mpls)
 {
-   __be32 *new_mpls_lse;
+   struct mpls_shim_hdr *new_mpls_lse;
 
/* Networking stack do not allow simultaneous Tunnel and MPLS GSO. */
if (skb->encapsulation)
@@ -195,20 +195,26 @@ static int push_mpls(struct sk_buff *skb, struct 
sw_flow_key *key,
if (skb_cow_head(skb, MPLS_HLEN) < 0)
return -ENOMEM;
 
+   if (!ovs_skb_get_inner_protocol(skb)) {
+   skb_set_inner_network_header(skb, skb->mac_len);
+ 

Re: [ovs-dev] [PATCH 1/3] datapath: Fixups for MPLS GSO

[Yi-Hung Wei]

Hi Simon,

Thanks for your review. Please find my relies below.
I will sent out v2 based on your review.

On Tue, Apr 25, 2017 at 12:43 AM, Simon Horman
 wrote:
> Hi Yi-Hung,
>
> thanks for taking on this difficult piece of work and apologies for the
> delay in responding.
>
> On Mon, Apr 03, 2017 at 04:24:36PM -0700, Yi-Hung Wei wrote:
>> This commit backports the following upstream commits to fix MPLS GSO in ovs
>> datapath. It has been tested on kernel 4.4 and 4.9.
>
> Thanks also for noting the versions this has been tested against. I expect
> there testing against other versions will show some residual problems but
> I think that fixing 4.4 and 4.9 is a good step forwards.
>
> I see that this patch backports 4 upstream patches. I am curious to know
> if you considered backporting them individually. That would have made
> reviewing a little easier for me.
Thanks for your suggestion. I pull two independent patches out of this
patch in v2.
Commit 48d2ab609b6b("net: mpls: Fixups for GSO") and 85de4a2101ac
("openvswitch: use mpls_hdr") are backported together since I am using the
mpls_hdr() in the later commit to backport some logic in the first commit.

>
>> Upstream commit:
>> commit 48d2ab609b6bbecb7698487c8579bc40de9d6dfa
>> Author: David Ahern 
>> Date:   Wed Aug 24 20:10:44 2016 -0700
>
> ...
>
>> Upstream commit:
>> commit f7d49bce8e741e1e6aa14ce4db1b6cea7e4be4e8
>> Author: Jiri Benc 
>> Date:   Fri Sep 30 19:08:05 2016 +0200
>
> ...
>
>> Upstream commit:
>> commit 85de4a2101acb85c3b1dde465e84596ccca99f2c
>> Author: Jiri Benc 
>> Date:   Fri Sep 30 19:08:07 2016 +0200
>>
>> openvswitch: use mpls_hdr
>>
>> skb_mpls_header is equivalent to mpls_hdr now. Use the existing helper
>> instead.
>>
>> Signed-off-by: Jiri Benc 
>> Acked-by: Pravin B Shelar 
>> Signed-off-by: David S. Miller 
>
> ...
>
>> Upstream commit:
>> commit c66549ffd05831abf6cf19ce0571ad868e39
>> Author: Jiri Benc 
>> Date:   Wed Oct 5 15:01:57 2016 +0200
>
> ...
>
>> diff --git a/acinclude.m4 b/acinclude.m4
>> index 744d8f89525c..82ca4a28015c 100644
>> --- a/acinclude.m4
>> +++ b/acinclude.m4
>> @@ -479,6 +479,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
>>  [OVS_GREP_IFELSE([$KSRC/include/net/dst_cache.h], 
>> [dst_cache],
>>   
>> [OVS_DEFINE([USE_UPSTREAM_TUNNEL])])])])
>>
>> +  OVS_GREP_IFELSE([$KSRC/include/linux/mpls.h], [mpls_hdr])
>
> Should the path be $KSRC/include/net/mpls.h?
>
> I am looking at 9095e10edd28 ("mpls: move mpls_hdr to a common location")
Yes, you're right. Thanks for finding this bug.

>
>>OVS_GREP_IFELSE([$KSRC/include/linux/net.h], [sock_create_kern.*net],
>>[OVS_DEFINE([HAVE_SOCK_CREATE_KERN_NET])])
>>OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], 
>> [ndo_fill_metadata_dst])
>> diff --git a/datapath/actions.c b/datapath/actions.c
>> index 06080b24ea5a..ecc5136a3529 100644
>> --- a/datapath/actions.c
>> +++ b/datapath/actions.c
>
> ...
>
>> @@ -169,20 +170,26 @@ static int push_mpls(struct sk_buff *skb, struct 
>> sw_flow_key *key,
>>   if (skb_cow_head(skb, MPLS_HLEN) < 0)
>>   return -ENOMEM;
>>
>> + if (!ovs_skb_get_inner_protocol(skb)) {
>> + skb_set_inner_network_header(skb, skb->mac_len);
>> + ovs_skb_set_inner_protocol(skb, skb->protocol);
>> + }
>> +
>>   skb_push(skb, MPLS_HLEN);
>>   memmove(skb_mac_header(skb) - MPLS_HLEN, skb_mac_header(skb),
>>   skb->mac_len);
>>   skb_reset_mac_header(skb);
>> +#ifdef HAVE_MPLS_HDR
>> + skb_set_network_header(skb, skb->mac_len);
>> +#endif
>
> It is not clear to me why this call to skb_set_network_header() is
> guarded by HAVE_MPLS_HDR here and moreover not elsewhere in this patch.

This patch uses HAVE_MPLS_HDR to determine if the kernel that ovs datapath is
compiled with has the new or the old mpls_gso kernel module. Because
starting from
commit 48d2ab609b6b ("net: mpls: Fixups for GSO"), the mpls_gso kernel module
relies on the fact that skb_network_header() points to the mpls header and
skb_inner_network_header() points to the L3 header so that it can
derive the length of
mpls header correctly. However, the old mpls_gso kernel module assumes that the
skb_network_header() points to the L3 header, and the old mpls_gso
kernel module will
misbehave if the ovs datapath marks the skb_network_header() in the
new way since it will
treat mpls header as the L3 header. Therefore, we only need to set the
network_header in
newer kernel.

I use HAVE_MPLS_HDR to determine whether we have new or old mpls_gso
module, cause
I want to avoid using kernel version number, and since these two
patches are related, and they
are both in 4.9 kernel.

The other part of the code 

[ovs-dev] [PATCH] vswitchd: Add 'appctl quit' command

[Andy Zhou]

vswitchd can gracefully shutdown after receiving the 'appctl exit'
command.  But the datapath resources vswitchd created can
linger on. This is not a problem, and in fact desireable when the
shutdown is transient in nature. Since restarted vswitchd usually
sees a similar configuration. By keeping the datapath resources
minimize the perturbation to the running data traffic.

However, when vswitchd shutdown is permanent, there should be a
way to release the all datapath resources, so that they won't
be linger on forever.  Add 'appctl quit' command for such purpose.

Signed-off-by: Andy Zhou 
---
 NEWS   |  1 +
 ofproto/ofproto-dpif.c | 11 +++
 ofproto/ofproto-provider.h |  2 +-
 ofproto/ofproto.c  |  2 +-
 vswitchd/bridge.c  |  4 ++--
 vswitchd/bridge.h  |  4 +++-
 vswitchd/ovs-vswitchd.8.in |  3 +++
 vswitchd/ovs-vswitchd.c|  7 ---
 8 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/NEWS b/NEWS
index ea97d84a2dea..3f65654f5124 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ Post-v2.7.0
  * Bundles now support hashing by just nw_src or nw_dst.
  * The "learn" action now supports a "limit" option (see ovs-ofctl(8)).
  * The port status bit OFPPS_LIVE now reflects link aliveness.
+   - Add the command 'ovs-appctl quit' (see ovs-vswitchd(8)).
 
 v2.7.0 - 21 Feb 2017
 -
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c73c2738c91c..bd2eaa60d36b 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -645,7 +645,7 @@ dealloc(struct ofproto *ofproto_)
 }
 
 static void
-close_dpif_backer(struct dpif_backer *backer)
+close_dpif_backer(struct dpif_backer *backer, bool del)
 {
 ovs_assert(backer->refcount > 0);
 
@@ -661,6 +661,9 @@ close_dpif_backer(struct dpif_backer *backer)
 shash_find_and_delete(_dpif_backers, backer->type);
 free(backer->type);
 free(backer->dp_version_string);
+if (del) {
+dpif_delete(backer->dpif);
+}
 dpif_close(backer->dpif);
 free(backer);
 }
@@ -772,7 +775,7 @@ open_dpif_backer(const char *type, struct dpif_backer 
**backerp)
 if (error) {
 VLOG_ERR("failed to listen on datapath of type %s: %s",
  type, ovs_strerror(error));
-close_dpif_backer(backer);
+close_dpif_backer(backer, false);
 return error;
 }
 
@@ -1452,7 +1455,7 @@ add_internal_flows(struct ofproto_dpif *ofproto)
 }
 
 static void
-destruct(struct ofproto *ofproto_)
+destruct(struct ofproto *ofproto_, bool del)
 {
 struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
 struct ofproto_async_msg *am;
@@ -1505,7 +1508,7 @@ destruct(struct ofproto *ofproto_)
 
 seq_destroy(ofproto->ams_seq);
 
-close_dpif_backer(ofproto->backer);
+close_dpif_backer(ofproto->backer, del);
 }
 
 static int
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index b7b12cdfd5f4..ef993d0afc4d 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -828,7 +828,7 @@ struct ofproto_class {
  */
 struct ofproto *(*alloc)(void);
 int (*construct)(struct ofproto *ofproto);
-void (*destruct)(struct ofproto *ofproto);
+void (*destruct)(struct ofproto *ofproto, bool del);
 void (*dealloc)(struct ofproto *ofproto);
 
 /* Performs any periodic activity required by 'ofproto'.  It should:
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index ca0f3e49bd67..7bc7b7f99d0d 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1651,7 +1651,7 @@ ofproto_destroy(struct ofproto *p, bool del)
 free(usage);
 }
 
-p->ofproto_class->destruct(p);
+p->ofproto_class->destruct(p, del);
 
 /* We should not postpone this because it involves deleting a listening
  * socket which we may want to reopen soon. 'connmgr' may be used by other
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index ebb6249416fa..e741f34f19ec 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -496,14 +496,14 @@ bridge_init(const char *remote)
 }
 
 void
-bridge_exit(void)
+bridge_exit(bool delete_datpath)
 {
 struct bridge *br, *next_br;
 
 if_notifier_destroy(ifnotifier);
 seq_destroy(ifaces_changed);
 HMAP_FOR_EACH_SAFE (br, next_br, node, _bridges) {
-bridge_destroy(br, false);
+bridge_destroy(br, delete_datpath);
 }
 ovsdb_idl_destroy(idl);
 }
diff --git a/vswitchd/bridge.h b/vswitchd/bridge.h
index 3783a21e3b4c..7835611568cf 100644
--- a/vswitchd/bridge.h
+++ b/vswitchd/bridge.h
@@ -16,10 +16,12 @@
 #ifndef VSWITCHD_BRIDGE_H
 #define VSWITCHD_BRIDGE_H 1
 
+#include 
+
 struct simap;
 
 void bridge_init(const char *remote);
-void bridge_exit(void);
+void bridge_exit(bool delete_datpath);
 
 void bridge_run(void);
 void bridge_wait(void);
diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
index 8496aa68af97..bb855861992b 100644
--- a/vswitchd/ovs-vswitchd.8.in
+++ 

Re: [ovs-dev] [PATCH] MAINTAINERS: Update committer documentation refs.

[Joe Stringer]

On 25 April 2017 at 12:49, Ben Pfaff  wrote:
> On Tue, Apr 25, 2017 at 11:20:43AM -0700, Joe Stringer wrote:
>> These references have moved since the MAINTAINERS file was introduced.
>> Update them.
>>
>> Signed-off-by: Joe Stringer 
>
> Thanks!
>
> Acked-by: Ben Pfaff 

Thanks, applied to master and branch-2.7.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Liderazgo y Coaching de Millenials

[Superando los Retos 2017 - En Línea]

 

En línea y en Vivo / Para todo su Equipo con una sola Conexión 

Liderazgo y Coaching de Millenials 

Superando los Retos
11 de mayo - Online en Vivo - 10:00 a 13:00 y de 15:00 a 18:00 Hrs
 
Tener madera de líder o bien haber adquirido las habilidades para ser uno 
bueno, es sin lugar a dudas necesario y útil para conducir a nuestros equipos 
de trabajo para alcanzar metas, sin embargo, esto ya no es suficiente en estos 
tiempos. Los “millennials” o “generación Y” han llegado con sus 
características, gustos, estilos, manera de actuar y de ver la vida; vienen 
empujando, influyendo y transformando el mundo en que vivimos y el sector 
laboral es uno de ellos. 
TEMARIO: 


1. Mitos y realidades del “Millennial”. 

2. Anatomía de la “Generación Y” o “Millennial”.

3. Marco generacional: de los “boomers” a la generación “Y”.

4. Coaching para Milennials.



...¡Y mucho más!


 
¿Requiere la información a la Brevedad?
responda este email con la palabra: 
Info - Millenials.
Junto con los siguientes datos: Nombre, Empresa, Teléfono y se lo enviaremos a 
la brevedad.

centro telefónico: 018002129393
 

Lic. Arturo López
Coordinador de Evento


 
¿Demasiados mensajes en su cuenta? Responda este mensaje indicando que solo 
desea recibir CALENDARIO y sólo recibirá un correo al mes. Si desea cancelar la 
suscripción, solicite su BAJA. 
 

 

 



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 2/4] doc: Convert ovs-vlan-test to rST

[Lance Richardson]

> From: "Ben Pfaff" 
> To: "Leif Madsen" 
> Cc: "ovs dev" 
> Sent: Monday, 24 April, 2017 5:02:28 PM
> Subject: Re: [ovs-dev] [RFC 2/4] doc: Convert ovs-vlan-test to rST
> 
> On Mon, Apr 24, 2017 at 04:37:51PM -0400, Leif Madsen wrote:
> > On Mon, Apr 24, 2017 at 3:57 PM, Ben Pfaff  wrote:
> > 
> > > On Mon, Apr 24, 2017 at 03:52:53PM -0400, Leif Madsen wrote:
> > > > I think this change might have broken packaging :)
> > > >
> > > > I just tested, and with the removal / renaming of the man8 pages for
> > > > ovs-test and ovs-vlan-test, the RPM fails to build because of missing
> > > files
> > > > that no longer match the glob.
> > > >
> > > > These two lines need to be removed from the build:
> > > >
> > > > 471 %{_mandir}/man8/ovs-test.8*
> > > > 472 %{_mandir}/man8/ovs-vlan-test.8*
> > > >
> > > >
> > > > I'll submit a patch shortly.
> > >
> > > OK, I'm confused then.  There was no removal or renaming of the
> > > installed man8 pages, only of the source files.  So, when I run "make
> > > install DESTDIR=$PWD/inst", I get a file
> > > inst/usr/share/man/man8/ovs-test.8 installed, and that should be the
> > > same as before.
> > >
> > > Any idea what's going on?
> > >
> > 
> > Huh, well then I'm very confused as well :)
> > 
> > I just did a test, and things seem to build fine when I remove those two
> > lines. Otherwise, the RPM will fail to build with the following:
> > 
> > Processing files:
> > openvswitch-test-2.7.90.14191.git3570f7e4-1.el7.centos.noarch
> > error: File not found by glob:
> > /builddir/build/BUILDROOT/openvswitch-2.7.90.14191.git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-test.8*
> > error: File not found by glob:
> > /builddir/build/BUILDROOT/openvswitch-2.7.90.14191.git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-vlan-test.8*
> > 
> > 
> > RPM build errors:
> > File not found by glob:
> > /builddir/build/BUILDROOT/openvswitch-2.7.90.14191.git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-test.8*
> > File not found by glob:
> > /builddir/build/BUILDROOT/openvswitch-2.7.90.14191.git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-vlan-test.8*
> > 

I ran into this a little while ago. The problem in my case was that sphinx-build
was not installed, so these man pages were not being built (seems odd that this
would not cause a build failure...)

Doing "yum install python-sphinx" let me build the rpms successfully once
again. Seems we should add "BuildRequires: python-sphinx" to the spec file.

Regards,

   Lance
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] MAINTAINERS: Update committer documentation refs.

[Ben Pfaff]

On Tue, Apr 25, 2017 at 11:20:43AM -0700, Joe Stringer wrote:
> These references have moved since the MAINTAINERS file was introduced.
> Update them.
> 
> Signed-off-by: Joe Stringer 

Thanks!

Acked-by: Ben Pfaff 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] MAINTAINERS: Update committer documentation refs.

[Joe Stringer]

These references have moved since the MAINTAINERS file was introduced.
Update them.

Signed-off-by: Joe Stringer 
---
 MAINTAINERS.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS.rst b/MAINTAINERS.rst
index 28831ab4a242..60fa1f55d486 100644
--- a/MAINTAINERS.rst
+++ b/MAINTAINERS.rst
@@ -29,10 +29,10 @@ Open vSwitch committers are the people who have been 
granted access to push
 changes to to the Open vSwitch git repository.
 
 The responsibilities of an Open vSwitch committer are documented
-`here `__.
+`here `__.
 
 The process for adding or removing committers is documented
-`here `__.
+`here `__.
 
 This is the current list of Open vSwitch committers:
 
-- 
2.11.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.

[Chandran, Sugesh]

Regards
_Sugesh

> -Original Message-
> From: Kevin Traynor [mailto:ktray...@redhat.com]
> Sent: Tuesday, April 25, 2017 6:40 PM
> To: Chandran, Sugesh ; d...@openvswitch.org
> Subject: Re: [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.
> 
> On 04/10/2017 04:31 PM, Chandran, Sugesh wrote:
> > Thank you for the patch,
> > Please see the comment below.
> >
> > Regards
> > _Sugesh
> >
> >> -Original Message-
> >> From: Kevin Traynor [mailto:ktray...@redhat.com]
> >> Sent: Tuesday, April 4, 2017 6:35 AM
> >> To: d...@openvswitch.org
> >> Cc: Kevin Traynor ; Chandran, Sugesh
> >> 
> >> Subject: [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.
> >>
> >> Rx checksum offload is enabled by default where available, with
> >> reconfiguration through OVSDB options:rx-checksum-offload.
> >> However, setting rx-checksum-offload does not result in a
> >> reconfiguration of the NIC.
> >>
> >> Fix that by checking if the requested port config features (e.g. rx
> >> checksum
> >> offload) are currently applied on the NIC and if not, perform a
> >> reconfiguration.
> >>
> >> Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading
> >> feature on DPDK physical ports.")
> >> Cc: Sugesh Chandran 
> >> Signed-off-by: Kevin Traynor 
> >> ---
> >>  lib/netdev-dpdk.c | 14 +-
> >>  1 file changed, 9 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> >> ddc651b..d5a9800
> >> 100644
> >> --- a/lib/netdev-dpdk.c
> >> +++ b/lib/netdev-dpdk.c
> >> @@ -374,4 +374,5 @@ struct netdev_dpdk {
> >>  int requested_rxq_size;
> >>  int requested_txq_size;
> >> +uint32_t requested_hwol;
> >>
> >>  /* Number of rx/tx descriptors for physical devices */ @@ -647,5
> >> +648,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int
> >> n_rxq, int
> >> n_txq)
> >>  conf.rxmode.max_rx_pkt_len = 0;
> >>  }
> >> -conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
> >> +conf.rxmode.hw_ip_checksum = (dev->requested_hwol &
> >>NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> >>  /* A device may report more queues than it makes available (this
> >> has @@
> >> -701,4 +702,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
> *dev,
> >> int n_rxq, int n_txq)
> >>  dev->up.n_rxq = n_rxq;
> >>  dev->up.n_txq = n_txq;
> >> +dev->hw_ol_features = dev->requested_hwol;
> >>
> >>  return 0;
> >> @@ -718,5 +720,5 @@ dpdk_eth_checksum_offload_configure(struct
> >> netdev_dpdk *dev)
> >>   DEV_RX_OFFLOAD_IPV4_CKSUM;
> >>  rte_eth_dev_info_get(dev->port_id, );
> >> -rx_csum_ol_flag = (dev->hw_ol_features &
> >> NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> >> +rx_csum_ol_flag = (dev->requested_hwol &
> >> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> >>
> >>  if (rx_csum_ol_flag &&
> >> @@ -725,5 +727,5 @@ dpdk_eth_checksum_offload_configure(struct
> >> netdev_dpdk *dev)
> >>  VLOG_WARN_ONCE("Rx checksum offload is not supported on
> >> device %d",
> >> dev->port_id);
> >> -dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
> >> +dev->requested_hwol &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
> >>  return;
> >>  }
> >> @@ -871,4 +873,5 @@ common_construct(struct netdev *netdev,
> unsigned
> >> int port_no,
> >>  /* Initilize the hardware offload flags to 0 */
> >>  dev->hw_ol_features = 0;
> >> +dev->requested_hwol = 0;
> >>
> >>  dev->flags = NETDEV_UP | NETDEV_PROMISC; @@ -1259,5 +1262,5
> @@
> >> netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
> >>  != 0;
> >>  if (temp_flag != rx_chksm_ofld) {
> >> -dev->hw_ol_features ^= NETDEV_RX_CHECKSUM_OFFLOAD;
> >> +dev->requested_hwol ^= NETDEV_RX_CHECKSUM_OFFLOAD;
> >>  dpdk_eth_checksum_offload_configure(dev);
> >>  }
> >> @@ -3124,5 +3127,6 @@ netdev_dpdk_reconfigure(struct netdev
> *netdev)
> >>  && dev->rxq_size == dev->requested_rxq_size
> >>  && dev->txq_size == dev->requested_txq_size
> >> -&& dev->socket_id == dev->requested_socket_id) {
> >> +&& dev->socket_id == dev->requested_socket_id
> >> +&& dev->hw_ol_features == dev->requested_hwol) {
> >>  /* Reconfiguration is unnecessary */
> > [Sugesh] The patch fixes the issue however I believe these checks has to
> be moved to the corresponding functions than here. May be that looks
> cleaner.  Any configuration change that happen to a netdev validated at the
> corresponding functions than in a common reconfigure. That can avoid the
> need of 2 struct element for every param.
> > I verified the patch and now it reloads the NIC configuration when there is
> a change.
> 
> Hi Sugesh, Not sure I totally follow you, I think this fixes the issue 

Re: [ovs-dev] [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.

[Kevin Traynor]

On 04/10/2017 04:31 PM, Chandran, Sugesh wrote:
> Thank you for the patch,
> Please see the comment below.
> 
> Regards
> _Sugesh
> 
>> -Original Message-
>> From: Kevin Traynor [mailto:ktray...@redhat.com]
>> Sent: Tuesday, April 4, 2017 6:35 AM
>> To: d...@openvswitch.org
>> Cc: Kevin Traynor ; Chandran, Sugesh
>> 
>> Subject: [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.
>>
>> Rx checksum offload is enabled by default where available, with
>> reconfiguration through OVSDB options:rx-checksum-offload.
>> However, setting rx-checksum-offload does not result in a reconfiguration of
>> the NIC.
>>
>> Fix that by checking if the requested port config features (e.g. rx checksum
>> offload) are currently applied on the NIC and if not, perform a
>> reconfiguration.
>>
>> Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading feature
>> on DPDK physical ports.")
>> Cc: Sugesh Chandran 
>> Signed-off-by: Kevin Traynor 
>> ---
>>  lib/netdev-dpdk.c | 14 +-
>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ddc651b..d5a9800
>> 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -374,4 +374,5 @@ struct netdev_dpdk {
>>  int requested_rxq_size;
>>  int requested_txq_size;
>> +uint32_t requested_hwol;
>>
>>  /* Number of rx/tx descriptors for physical devices */ @@ -647,5 +648,5
>> @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int
>> n_txq)
>>  conf.rxmode.max_rx_pkt_len = 0;
>>  }
>> -conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>> +conf.rxmode.hw_ip_checksum = (dev->requested_hwol &
>>NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>>  /* A device may report more queues than it makes available (this has @@
>> -701,4 +702,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev,
>> int n_rxq, int n_txq)
>>  dev->up.n_rxq = n_rxq;
>>  dev->up.n_txq = n_txq;
>> +dev->hw_ol_features = dev->requested_hwol;
>>
>>  return 0;
>> @@ -718,5 +720,5 @@ dpdk_eth_checksum_offload_configure(struct
>> netdev_dpdk *dev)
>>   DEV_RX_OFFLOAD_IPV4_CKSUM;
>>  rte_eth_dev_info_get(dev->port_id, );
>> -rx_csum_ol_flag = (dev->hw_ol_features &
>> NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>> +rx_csum_ol_flag = (dev->requested_hwol &
>> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>>
>>  if (rx_csum_ol_flag &&
>> @@ -725,5 +727,5 @@ dpdk_eth_checksum_offload_configure(struct
>> netdev_dpdk *dev)
>>  VLOG_WARN_ONCE("Rx checksum offload is not supported on device
>> %d",
>> dev->port_id);
>> -dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
>> +dev->requested_hwol &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
>>  return;
>>  }
>> @@ -871,4 +873,5 @@ common_construct(struct netdev *netdev, unsigned
>> int port_no,
>>  /* Initilize the hardware offload flags to 0 */
>>  dev->hw_ol_features = 0;
>> +dev->requested_hwol = 0;
>>
>>  dev->flags = NETDEV_UP | NETDEV_PROMISC; @@ -1259,5 +1262,5 @@
>> netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
>>  != 0;
>>  if (temp_flag != rx_chksm_ofld) {
>> -dev->hw_ol_features ^= NETDEV_RX_CHECKSUM_OFFLOAD;
>> +dev->requested_hwol ^= NETDEV_RX_CHECKSUM_OFFLOAD;
>>  dpdk_eth_checksum_offload_configure(dev);
>>  }
>> @@ -3124,5 +3127,6 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
>>  && dev->rxq_size == dev->requested_rxq_size
>>  && dev->txq_size == dev->requested_txq_size
>> -&& dev->socket_id == dev->requested_socket_id) {
>> +&& dev->socket_id == dev->requested_socket_id
>> +&& dev->hw_ol_features == dev->requested_hwol) {
>>  /* Reconfiguration is unnecessary */
> [Sugesh] The patch fixes the issue however I believe these checks has to be 
> moved to the corresponding functions than here. May be that looks cleaner.  
> Any configuration change that happen to a netdev validated at the 
> corresponding functions than in a common reconfigure. That can avoid the need 
> of 2 struct element for every param.
> I verified the patch and now it reloads the NIC configuration when there is a 
> change.

Hi Sugesh, Not sure I totally follow you, I think this fixes the issue
close to the original patch and the current reconfigure approach. Are
you suggesting that the reconfigure code should be re-written to remove
the requested_*'s before this bug fix? or a different fix for this
specific issue? Maybe you could add comment at the LOC you think should
change so I understand.

thanks,
Kevin.

> 
> 
> 
>>
>> --
>> 1.8.3.1
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] compat: Fix build error in kernel 4.10.0

[Greg Rose]

On Tue, 2017-04-25 at 23:26 +0800, Guoshuai Li wrote:
> In kernel 4.10.0, the function "nf_defrag_ipv6_enable" need
> parameters "struct net *", so we need call it for each namespace init
> to load netfilter fragment kmod.
> 
> Reported-by: Raymond Burkholder 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331411.html
> Signed-off-by: Guoshuai Li 

Compile tested and looks OK to me.

Reviewed-by: Greg Rose 

> ---
>  datapath/linux/compat/ip_fragment.c| 14 ++
>  datapath/linux/compat/nf_conntrack_reasm.c | 14 ++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/datapath/linux/compat/ip_fragment.c 
> b/datapath/linux/compat/ip_fragment.c
> index b0f5d0e..efa86fc 100644
> --- a/datapath/linux/compat/ip_fragment.c
> +++ b/datapath/linux/compat/ip_fragment.c
> @@ -729,18 +729,32 @@ int rpl_ip_defrag(struct net *net, struct sk_buff *skb, 
> u32 user)
>   return -ENOMEM;
>  }
>  
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,10,0)
> +static int __net_init ipv4_frags_init_net(struct net *net)
> +{
> + nf_defrag_ipv4_enable(net);
> +
> + return 0;
> +}
> +#endif
> +
>  static void __net_exit ipv4_frags_exit_net(struct net *net)
>  {
>   inet_frags_exit_net(>ipv4.frags, _frags);
>  }
>  
>  static struct pernet_operations ip4_frags_ops = {
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,10,0)
> + .init = ipv4_frags_init_net,
> +#endif
>   .exit = ipv4_frags_exit_net,
>  };
>  
>  int __init rpl_ipfrag_init(void)
>  {
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,10,0)
>   nf_defrag_ipv4_enable();
> +#endif
>   register_pernet_subsys(_frags_ops);
>   ip4_frags.hashfn = ip4_hashfn;
>   ip4_frags.constructor = ip4_frag_init;
> diff --git a/datapath/linux/compat/nf_conntrack_reasm.c 
> b/datapath/linux/compat/nf_conntrack_reasm.c
> index 0bc4d9e..cb6da6c 100644
> --- a/datapath/linux/compat/nf_conntrack_reasm.c
> +++ b/datapath/linux/compat/nf_conntrack_reasm.c
> @@ -558,12 +558,24 @@ out_unlock:
>   return ret;
>  }
>  
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,10,0)
> +static int nf_ct_net_init(struct net *net)
> +{
> + nf_defrag_ipv6_enable(net);
> +
> + return 0;
> +}
> +#endif
> +
>  static void nf_ct_net_exit(struct net *net)
>  {
>   inet_frags_exit_net(>nf_frag.frags, _frags);
>  }
>  
>  static struct pernet_operations nf_ct_net_ops = {
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,10,0)
> + .init = nf_ct_net_init,
> +#endif
>   .exit = nf_ct_net_exit,
>  };
>  
> @@ -571,7 +583,9 @@ int rpl_nf_ct_frag6_init(void)
>  {
>   int ret = 0;
>  
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,10,0)
>   nf_defrag_ipv6_enable();
> +#endif
>   nf_frags.hashfn = nf_hashfn;
>   nf_frags.constructor = ip6_frag_init;
>   nf_frags.destructor = NULL;



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] nf_defrag_ipv6_enable problem in datapath/linux/compat/nf_conntrack_reasm.c

[Greg Rose]

On Wed, 2017-04-26 at 00:33 +0800, Guoshuai Li wrote:
> 
> > Sure, here it is:
> > 
> > make -C /lib/modules/4.11.0-rc7+/build
> > M=/home/gvrose/prj/ovs/datapath/linux modules
> > make[4]: Entering directory `/home/gvrose/prj/net-next'
> >   CC [M]  /home/gvrose/prj/ovs/datapath/linux/actions.o
> >   CC [M]  /home/gvrose/prj/ovs/datapath/linux/conntrack.o
> >   CC [M]  /home/gvrose/prj/ovs/datapath/linux/datapath.o
> >   CC [M]  /home/gvrose/prj/ovs/datapath/linux/flow.o
> >   CC [M]  /home/gvrose/prj/ovs/datapath/linux/dp_notify.o
> >   CC [M]  /home/gvrose/prj/ovs/datapath/linux/flow_netlink.o
> > /home/gvrose/prj/ovs/datapath/linux/datapath.c: In function
> > ‘ovs_flow_cmd_dump’:
> > /home/gvrose/prj/ovs/datapath/linux/datapath.c:1368:8: error: too few
> > arguments to function ‘genlmsg_parse’
> > OVS_FLOW_ATTR_MAX, flow_policy);
> Master branch code is:
> err = genlmsg_parse(cb->nlh, _flow_genl_family, a,
> OVS_FLOW_ATTR_MAX, flow_policy, NULL);
> 

In fact I was out of synch with upstream master.  Now the compile works
fine.

I'll add my Reviewed-by: to your original patch.  I think since I'm
still sort of new to OVS we should get another ACK on it from one of the
more experienced developers.

Thanks for your work!

- Greg


> 
> Please Check in genetlink.h:
> 
> static inline int rpl_genlmsg_parse(const struct nlmsghdr *nlh,
> const struct genl_family *family,
> struct nlattr *tb[], int maxtype,
> const struct nla_policy *policy,
> struct netlink_ext_ack *extack)
> or 
> 
> static inline int rpl_genlmsg_parse(const struct nlmsghdr *nlh,
> const struct genl_family *family,
> struct nlattr *tb[], int maxtype,
> const struct nla_policy *policy)
> 
> > ^
> > In file included
> > from /home/gvrose/prj/ovs/datapath/linux/compat/include/net/genetlink.h:7:0,
> >  from /home/gvrose/prj/ovs/datapath/linux/datapath.c:51:
> > ./include/net/genetlink.h:179:19: note: declared here
> >  static inline int genlmsg_parse(const struct nlmsghdr *nlh,
> >^
> > /home/gvrose/prj/ovs/datapath/linux/flow_netlink.c: In function
> > ‘validate_userspace’:
> > /home/gvrose/prj/ovs/datapath/linux/flow_netlink.c:2435:6: error: too
> > few arguments to function ‘nla_parse_nested’
> >   attr, userspace_policy);
> Are you in Branch2.7 ? The Code from master branch is not the same as
> yours code: 
> 
> error = nla_parse_nested(a, OVS_USERSPACE_ATTR_MAX, attr,
>  userspace_policy, NULL);
> 
> My patch is Base on Master branch.
> 
> >   ^
> > In file included
> > from /home/gvrose/prj/ovs/datapath/linux/compat/include/net/netlink.h:5:0,
> > 
> > from /home/gvrose/prj/ovs/datapath/linux/compat/include/linux/netlink.h:13,
> >  from ./include/uapi/linux/neighbour.h:5,
> >  from ./include/linux/netdevice.h:50,
> > 
> > from /home/gvrose/prj/ovs/datapath/linux/compat/include/linux/netdevice.h:4,
> > 
> > from /home/gvrose/prj/ovs/datapath/linux/flow_netlink.c:22:
> > ./include/net/netlink.h:754:19: note: declared here
> >  static inline int nla_parse_nested(struct nlattr *tb[], int maxtype,
> >^
> > 
> 



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] compat: Fix build error in kernel 4.10.0

[Raymond Burkholder]

Ok, thanx!  That gave me a successful build.  A two minute turnaround on a
patch request!

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Guoshuai Li
> Sent: Tuesday, April 25, 2017 12:27
> To: ovs-dev@openvswitch.org
> Subject: [ovs-dev] [PATCH] compat: Fix build error in kernel 4.10.0
> 
> In kernel 4.10.0, the function "nf_defrag_ipv6_enable" need parameters
> "struct net *", so we need call it for each namespace init to load
netfilter
> fragment kmod.
> 
> Reported-by: Raymond Burkholder 
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-
> April/331411.html
> Signed-off-by: Guoshuai Li 
> ---
>  datapath/linux/compat/ip_fragment.c| 14 ++
>  datapath/linux/compat/nf_conntrack_reasm.c | 14 ++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/datapath/linux/compat/ip_fragment.c
> b/datapath/linux/compat/ip_fragment.c
> index b0f5d0e..efa86fc 100644
> --- a/datapath/linux/compat/ip_fragment.c
> +++ b/datapath/linux/compat/ip_fragment.c
> @@ -729,18 +729,32 @@ int rpl_ip_defrag(struct net *net, struct sk_buff
> *skb, u32 user)
>   return -ENOMEM;
>  }
> 
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,10,0) static int __net_init
> +ipv4_frags_init_net(struct net *net) {
> + nf_defrag_ipv4_enable(net);
> +
> + return 0;
> +}
> +#endif
> +
>  static void __net_exit ipv4_frags_exit_net(struct net *net)  {
>   inet_frags_exit_net(>ipv4.frags, _frags);  }
> 
>  static struct pernet_operations ip4_frags_ops = {
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,10,0)
> + .init = ipv4_frags_init_net,
> +#endif
>   .exit = ipv4_frags_exit_net,
>  };
> 
>  int __init rpl_ipfrag_init(void)
>  {
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,10,0)
>   nf_defrag_ipv4_enable();
> +#endif
>   register_pernet_subsys(_frags_ops);
>   ip4_frags.hashfn = ip4_hashfn;
>   ip4_frags.constructor = ip4_frag_init; diff --git
> a/datapath/linux/compat/nf_conntrack_reasm.c
> b/datapath/linux/compat/nf_conntrack_reasm.c
> index 0bc4d9e..cb6da6c 100644
> --- a/datapath/linux/compat/nf_conntrack_reasm.c
> +++ b/datapath/linux/compat/nf_conntrack_reasm.c
> @@ -558,12 +558,24 @@ out_unlock:
>   return ret;
>  }
> 
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,10,0) static int
> +nf_ct_net_init(struct net *net) {
> + nf_defrag_ipv6_enable(net);
> +
> + return 0;
> +}
> +#endif
> +
>  static void nf_ct_net_exit(struct net *net)  {
>   inet_frags_exit_net(>nf_frag.frags, _frags);  }
> 
>  static struct pernet_operations nf_ct_net_ops = {
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,10,0)
> + .init = nf_ct_net_init,
> +#endif
>   .exit = nf_ct_net_exit,
>  };
> 
> @@ -571,7 +583,9 @@ int rpl_nf_ct_frag6_init(void)  {
>   int ret = 0;
> 
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,10,0)
>   nf_defrag_ipv6_enable();
> +#endif
>   nf_frags.hashfn = nf_hashfn;
>   nf_frags.constructor = ip6_frag_init;
>   nf_frags.destructor = NULL;
> --
> 2.10.1.windows.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> --
> This message has been scanned for viruses and dangerous content by
> MailScanner, and is believed to be clean.



-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] nf_defrag_ipv6_enable problem in datapath/linux/compat/nf_conntrack_reasm.c

[Guoshuai Li]

Sure, here it is:

make -C /lib/modules/4.11.0-rc7+/build
M=/home/gvrose/prj/ovs/datapath/linux modules
make[4]: Entering directory `/home/gvrose/prj/net-next'
   CC [M]  /home/gvrose/prj/ovs/datapath/linux/actions.o
   CC [M]  /home/gvrose/prj/ovs/datapath/linux/conntrack.o
   CC [M]  /home/gvrose/prj/ovs/datapath/linux/datapath.o
   CC [M]  /home/gvrose/prj/ovs/datapath/linux/flow.o
   CC [M]  /home/gvrose/prj/ovs/datapath/linux/dp_notify.o
   CC [M]  /home/gvrose/prj/ovs/datapath/linux/flow_netlink.o
/home/gvrose/prj/ovs/datapath/linux/datapath.c: In function
‘ovs_flow_cmd_dump’:
/home/gvrose/prj/ovs/datapath/linux/datapath.c:1368:8: error: too few
arguments to function ‘genlmsg_parse’
 OVS_FLOW_ATTR_MAX, flow_policy);

Master branch code is:
err = genlmsg_parse(cb->nlh, _flow_genl_family, a,
OVS_FLOW_ATTR_MAX, flow_policy, NULL);


Please Check in *genetlink.h*:

static inline int rpl_genlmsg_parse(const struct nlmsghdr *nlh,
const struct genl_family *family,
struct nlattr *tb[], int maxtype,
const struct nla_policy *policy,
struct netlink_ext_ack *extack)
or

static inline int rpl_genlmsg_parse(const struct nlmsghdr *nlh,
const struct genl_family *family,
struct nlattr *tb[], int maxtype,
const struct nla_policy *policy)


 ^
In file included
from /home/gvrose/prj/ovs/datapath/linux/compat/include/net/genetlink.h:7:0,
  from /home/gvrose/prj/ovs/datapath/linux/datapath.c:51:
./include/net/genetlink.h:179:19: note: declared here
  static inline int genlmsg_parse(const struct nlmsghdr *nlh,
^
/home/gvrose/prj/ovs/datapath/linux/flow_netlink.c: In function
‘validate_userspace’:
/home/gvrose/prj/ovs/datapath/linux/flow_netlink.c:2435:6: error: too
few arguments to function ‘nla_parse_nested’
   attr, userspace_policy);
Are you in Branch2.7 ? The Code from master branch is not the same as 
yours code:


error = nla_parse_nested(a, OVS_USERSPACE_ATTR_MAX, attr,
 userspace_policy, NULL);

My patch is Base on Master branch.


   ^
In file included
from /home/gvrose/prj/ovs/datapath/linux/compat/include/net/netlink.h:5:0,

from /home/gvrose/prj/ovs/datapath/linux/compat/include/linux/netlink.h:13,
  from ./include/uapi/linux/neighbour.h:5,
  from ./include/linux/netdevice.h:50,

from /home/gvrose/prj/ovs/datapath/linux/compat/include/linux/netdevice.h:4,

from /home/gvrose/prj/ovs/datapath/linux/flow_netlink.c:22:
./include/net/netlink.h:754:19: note: declared here
  static inline int nla_parse_nested(struct nlattr *tb[], int maxtype,
^



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v4 3/7] userspace: Switching of L3 packets in L2 pipeline

[Zoltán Balogh]

From: Jan Scheurich 

Ports have a new layer3 attribute if they send/receive L3 packets.

The packet_type included in structs dp_packet and flow is considered in
ofproto-dpif. The classical L2 match fields (dl_src, dl_dst, dl_type, and
vlan_tci, vlan_vid, vlan_pcp) now have Ethernet as pre-requisite.

A dummy ethernet header is pushed to L3 packets received from L3 ports
before the the pipeline processing starts. The ethernet header is popped
before sending a packet to a L3 port.

For datapath ports that can receive L2 or L3 packets, the packet_type
becomes part of the flow key for datapath flows and is handled
appropriately in dpif-netdev.

In the 'else' branch in flow_put_on_pmd() function, the additional check
flow_equal(, _flow->flow) was removed, as a) the dpcls
lookup is sufficient to uniquely identify a flow and b) it caused false
negatives because the flow in netdev->flow may not properly masked.

In dpif_netdev_flow_put() we now use the same method for constructing the
netdev_flow_key as the one used when adding the flow to the dplcs to make sure
these always match. The function netdev_flow_key_from_flow() used so far was
not only inefficient but sometimes caused mismatches and subsequent flow
update failures.

Signed-off-by: Lorand Jakab 
Signed-off-by: Simon Horman 
Signed-off-by: Jiri Benc 
Signed-off-by: Yi Yang 
Signed-off-by: Jan Scheurich 
Co-authored-by: Zoltan Balogh 
---
 build-aux/extract-ofp-fields  |   1 +
 datapath/linux/compat/include/linux/openvswitch.h |   2 +
 include/openvswitch/match.h   |   1 +
 include/openvswitch/meta-flow.h   |  15 +-
 lib/dpif-netdev.c |  46 +++---
 lib/dpif-netlink.c|   2 +-
 lib/dpif.c|   2 +-
 lib/match.c   |  25 +++
 lib/meta-flow.c   |   2 +
 lib/netdev-vport.c|   8 +-
 lib/netdev.h  |   1 +
 lib/odp-execute.c |   2 +
 lib/odp-util.c| 185 ++
 lib/odp-util.h|   6 +-
 lib/packets.h |   1 -
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-upcall.c |   4 +-
 ofproto/ofproto-dpif-xlate.c  |  55 ++-
 ofproto/ofproto-dpif.c|   6 +-
 ofproto/tunnel.c  |   3 +
 tests/tunnel-push-pop-ipv6.at |  10 +-
 tests/tunnel-push-pop.at  |  12 +-
 tests/tunnel.at   |  28 ++--
 23 files changed, 305 insertions(+), 113 deletions(-)

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index af7c69b..d5b8a82 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -39,6 +39,7 @@ FORMATTING = {"decimal":("MFS_DECIMAL",  1,   
8),
   "TCP flags":  ("MFS_TCP_FLAGS",2,   2)}
 
 PREREQS = {"none": "MFP_NONE",
+   "Ethernet": "MFP_ETHERNET",
"ARP": "MFP_ARP",
"VLAN VID": "MFP_VLAN_VID",
"IPv4": "MFP_IPV4",
diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 3a8fba1..4c7aff4 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -363,6 +363,8 @@ enum ovs_key_attr {
/* Only used within kernel data path. */
OVS_KEY_ATTR_TUNNEL_INFO,  /* struct ovs_tunnel_info */
 #endif
+
+   OVS_KEY_ATTR_PACKET_TYPE,  /* be32 packet type */
__OVS_KEY_ATTR_MAX
 };
 
diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
index 06fa04c..ce06919 100644
--- a/include/openvswitch/match.h
+++ b/include/openvswitch/match.h
@@ -115,6 +115,7 @@ void match_set_ct_ipv6_dst(struct match *, const struct 
in6_addr *);
 void match_set_ct_ipv6_dst_masked(struct match *, const struct in6_addr *,
   const struct in6_addr *);
 
+void match_set_packet_type(struct match *, ovs_be32 packet_type);
 void match_set_skb_priority(struct match *, uint32_t skb_priority);
 void match_set_dl_type(struct match *, ovs_be16);
 void match_set_dl_src(struct match *, const struct eth_addr );
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index 11852d2..c284ec6 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -985,7 +985,7 @@ enum OVS_PACKED_ENUM mf_field_id {
  * Type: MAC.
  * Maskable: bitwise.
  * 

[ovs-dev] [PATCH v4 7/7] userspace: add vxlan gpe support to vport

[Zoltán Balogh]

From: Georg Schmuecking 

This patch is based on the "datapath: enable vxlangpe creation in compat mode"
from Yi Yang. It introduces an extension option "gpe" to the vxlan port in the
netdev-dpdk datapath. Furthermore it introduces a new protocol header file
vxlangpe.h in which the vxlan gpe protocoll is described. In the vxlan specific
methods the different packet are introduced and handled.

Added VXLAN GPE tunnel push test.

Signed-off-by: Yi Yang 
Signed-off-by: Georg Schmuecking 
---
 datapath/linux/compat/include/linux/openvswitch.h |  1 +
 include/openvswitch/automake.mk   |  4 +-
 include/openvswitch/vxlangpe.h| 76 +++
 lib/netdev-native-tnl.c   | 61 --
 lib/netdev-vport.c| 18 +-
 tests/tunnel-push-pop.at  | 10 +++
 6 files changed, 160 insertions(+), 10 deletions(-)
 create mode 100755 include/openvswitch/vxlangpe.h

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 4c7aff4..4ae8bd9 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -291,6 +291,7 @@ enum ovs_vport_attr {
 enum {
OVS_VXLAN_EXT_UNSPEC,
OVS_VXLAN_EXT_GBP,  /* Flag or __u32 */
+   OVS_VXLAN_EXT_GPE = 8,  /* Flag or __u32 */
__OVS_VXLAN_EXT_MAX,
 };
 
diff --git a/include/openvswitch/automake.mk b/include/openvswitch/automake.mk
index c0e276f..c125f1e 100644
--- a/include/openvswitch/automake.mk
+++ b/include/openvswitch/automake.mk
@@ -29,5 +29,5 @@ openvswitchinclude_HEADERS = \
include/openvswitch/uuid.h \
include/openvswitch/version.h \
include/openvswitch/vconn.h \
-   include/openvswitch/vlog.h
-
+   include/openvswitch/vlog.h \
+   include/openvswitch/vxlangpe.h
diff --git a/include/openvswitch/vxlangpe.h b/include/openvswitch/vxlangpe.h
new file mode 100755
index 000..4c18d90
--- /dev/null
+++ b/include/openvswitch/vxlangpe.h
@@ -0,0 +1,76 @@
+#ifndef __OPENVSWITCH_VXLANGPE_H
+#define __OPENVSWITCH_VXLANGPE_H 1
+
+#include "openvswitch/types.h"
+
+#define u8 uint8_t
+#define u32 uint8_t
+#define __be32 ovs_be32
+
+/*
+ * VXLAN Generic Protocol Extension (VXLAN_F_GPE):
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |R|R|Ver|I|P|R|O|   Reserved|Next Protocol  |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |VXLAN Network Identifier (VNI) |   Reserved|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *
+ * Ver = Version. Indicates VXLAN GPE protocol version.
+ *
+ * P = Next Protocol Bit. The P bit is set to indicate that the
+ * Next Protocol field is present.
+ *
+ * O = OAM Flag Bit. The O bit is set to indicate that the packet
+ * is an OAM packet.
+ *
+ * Next Protocol = This 8 bit field indicates the protocol header
+ * immediately following the VXLAN GPE header.
+ *
+ * https://tools.ietf.org/html/draft-ietf-nvo3-vxlan-gpe-01
+ */
+
+struct vxlanhdr_gpe {
+#ifdef WORDS_BIGENDIAN
+   u8  reserved_flags2:2,
+   version:2,
+   instance_applied:1,
+   np_applied:1,
+   reserved_flags1:1,
+   oam_flag:1;
+#else
+   u8  oam_flag:1,
+   reserved_flags1:1,
+   np_applied:1,
+   instance_applied:1,
+   version:2,
+   reserved_flags2:2;
+#endif
+   u8  reserved_flags3;
+   u8  reserved_flags4;
+   u8  next_protocol;
+   __be32  vx_vni;
+};
+
+/* VXLAN-GPE header flags. */
+#define VXLAN_HF_VER   ((1U <<29) | (1U <<28))
+#define VXLAN_HF_NP(1U <<26)
+#define VXLAN_HF_OAM   (1U <<24)
+
+#define VXLAN_GPE_USED_BITS (VXLAN_HF_VER | VXLAN_HF_NP | VXLAN_HF_OAM | \
+0xff)
+
+/* VXLAN-GPE header Next Protocol. */
+#define VXLAN_GPE_NP_IPV4  0x01
+#define VXLAN_GPE_NP_IPV6  0x02
+#define VXLAN_GPE_NP_ETHERNET  0x03
+#define VXLAN_GPE_NP_NSH   0x04
+
+struct vxlan_metadata {
+   u32 gbp;
+   u32 gpe;
+};
+
+#define VXLAN_F_GPE0x4000
+#define VXLAN_HF_GPE 0x0400
+
+#endif /* __OPENVSWITCH_VXLANGPE_H */
diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 4590e25..2d947c2 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -44,6 +44,7 @@
 #include "unaligned.h"
 #include "unixctl.h"
 #include "openvswitch/vlog.h"
+#include "openvswitch/vxlangpe.h"
 
 VLOG_DEFINE_THIS_MODULE(native_tnl);
 static struct vlog_rate_limit err_rl = VLOG_RATE_LIMIT_INIT(60, 5);
@@ -497,6 +498,8 @@ netdev_vxlan_pop_header(struct dp_packet *packet)
 struct flow_tnl *tnl = >tunnel;
 struct vxlanhdr *vxh;
 unsigned 

[ovs-dev] [PATCH v4 4/7] userspace: L3 tunnel support for GRE and LISP

[Zoltán Balogh]

From: Jan Scheurich 

Add a boolean "layer3" configuration option for tunnel vports.
The layer3 option defaults to false for all ports except LISP.
GRE ports accept both true and false for "layer3".

A tunnel vport configured with layer3=true receives L3 packets.
which are then converted to Ethernet packets by pushing a dummy
Ethernet heder at the ingress of the OpenFlow pipeline. The
Ethernet header of a packet is stripped before sending to a
layer3 tunnel vport.

Presently a single GRE vport cannot carry both L2 and L3 packets.
But it is possible to create two GRE vports representing the same
GRE tunel, one with layer3=false, the other with layer3=true.
L2 packet from the tunnel are received on the first vport, L3
packets on the second. The controller must send packets to the
layer3 GRE vport to tunnel them without their Ethernet header.

Units tests have been added to check the L3 tunnel handling.

LISP tunnels are not yet supported by the netdev userspace datapath.

Signed-off-by: Simon Horman 
Signed-off-by: Jiri Benc 
Signed-off-by: Yi Yang 
Signed-off-by: Jan Scheurich 
Co-authored-by: Zoltan Balogh 
---
 lib/netdev-native-tnl.c   | 26 +-
 lib/netdev-vport.c| 14 --
 ofproto/tunnel.c  | 14 +++---
 tests/tunnel-push-pop-ipv6.at | 16 +++-
 tests/tunnel-push-pop.at  | 33 -
 vswitchd/vswitch.xml  | 13 +
 6 files changed, 96 insertions(+), 20 deletions(-)

diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 2798324..4590e25 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -154,6 +154,10 @@ netdev_tnl_push_ip_header(struct dp_packet *packet,
 *ip_tot_size = dp_packet_size(packet) - sizeof (struct eth_header);
 
 memcpy(eth, header, size);
+/* The encapsulated packet has type Ethernet. Adjust dp_packet. */
+packet->packet_type = htonl(PT_ETH);
+dp_packet_reset_offsets(packet);
+packet->l3_ofs = sizeof (struct eth_header);
 
 if (netdev_tnl_is_header_ipv6(header)) {
 ip6 = netdev_tnl_ipv6_hdr(eth);
@@ -345,6 +349,7 @@ parse_gre_header(struct dp_packet *packet,
 ovs_16aligned_be32 *options;
 int hlen;
 unsigned int ulen;
+uint16_t greh_protocol;
 
 greh = netdev_tnl_ip_extract_tnl_md(packet, tnl, );
 if (!greh) {
@@ -355,10 +360,6 @@ parse_gre_header(struct dp_packet *packet,
 return -EINVAL;
 }
 
-if (greh->protocol != htons(ETH_TYPE_TEB)) {
-return -EINVAL;
-}
-
 hlen = ulen + gre_header_len(greh->flags);
 if (hlen > dp_packet_size(packet)) {
 return -EINVAL;
@@ -388,6 +389,15 @@ parse_gre_header(struct dp_packet *packet,
 options++;
 }
 
+/* Set the new packet type depending on the GRE protocol field. */
+greh_protocol = ntohs(greh->protocol);
+if (greh_protocol == ETH_TYPE_TEB) {
+packet->packet_type = htonl(PT_ETH);
+} else {
+/* Allow all GRE protocol values as Ethertypes */
+packet->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE, greh_protocol);
+}
+
 return hlen;
 }
 
@@ -451,7 +461,11 @@ netdev_gre_build_header(const struct netdev *netdev,
 
 greh = netdev_tnl_ip_build_header(data, params, IPPROTO_GRE);
 
-greh->protocol = htons(ETH_TYPE_TEB);
+if (tnl_cfg->is_layer3) {
+greh->protocol = params->flow->dl_type;
+} else {
+greh->protocol = htons(ETH_TYPE_TEB);
+}
 greh->flags = 0;
 
 options = (ovs_16aligned_be32 *) (greh + 1);
@@ -504,6 +518,7 @@ netdev_vxlan_pop_header(struct dp_packet *packet)
 tnl->tun_id = htonll(ntohl(get_16aligned_be32(>vx_vni)) >> 8);
 tnl->flags |= FLOW_TNL_F_KEY;
 
+packet->packet_type = htonl(PT_ETH);
 dp_packet_reset_packet(packet, hlen + VXLAN_HLEN);
 
 return packet;
@@ -583,6 +598,7 @@ netdev_geneve_pop_header(struct dp_packet *packet)
 tnl->metadata.present.len = opts_len;
 tnl->flags |= FLOW_TNL_F_UDPIF;
 
+packet->packet_type = htonl(PT_ETH);
 dp_packet_reset_packet(packet, hlen);
 
 return packet;
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 84b9be3..ba69461 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -410,7 +410,7 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args, char **errp)
 const char *name = netdev_get_name(dev_);
 const char *type = netdev_get_type(dev_);
 struct ds errors = DS_EMPTY_INITIALIZER;
-bool needs_dst_port, has_csum;
+bool needs_dst_port, has_csum, optional_layer3;
 uint16_t dst_proto = 0, src_proto = 0;
 struct netdev_tunnel_config tnl_cfg;
 struct smap_node *node;
@@ -418,6 +418,7 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args, char **errp)
 
 has_csum = strstr(type, "gre") || 

[ovs-dev] [PATCH v4 1/7] userspace: Add packet_type in dp_packet and flow

[Zoltán Balogh]

From: Jan Scheurich 

This commit adds a packet_type attribute to the structs dp_packet and flow
to explicitly carry the type of the packet as prepration for the
introduction of the so-called packet type-aware pipeline (PTAP) in OVS.

The packet_type is a big-endian 32 bit integer with the encoding as
specified in OpenFlow verion 1.5.

The upper 16 bits contain the packet type name space. Pre-defined values
are defined in openflow-common.h:

enum ofp_header_type_namespaces {
OFPHTN_ONF = 0, /* ONF namespace. */
OFPHTN_ETHERTYPE = 1,   /* ns_type is an Ethertype. */
OFPHTN_IP_PROTO = 2,/* ns_type is a IP protocol number. */
OFPHTN_UDP_TCP_PORT = 3,/* ns_type is a TCP or UDP port. */
OFPHTN_IPV4_OPTION = 4, /* ns_type is an IPv4 option number. */
};

The lower 16 bits specify the actual type in the context of the name space.

Only name spaces 0 and 1 will be supported for now.

For name space OFPHTN_ONF the relevant packet type is 0 (Ethernet).
This is the default packet_type in OVS and the only one supported so far.
Packets of type (OFPHTN_ONF, 0) are called Ethernet packets.

In name space OFPHTN_ETHERTYPE the type is the Ethertype of the packet.
A packet of type (OFPHTN_ETHERTYPE, ) is a standard L2 packet
whith the Ethernet header (and any VLAN tags) removed to expose the L3
(or L2.5) payload of the packet. These will simply be called L3 packets.

The Ethernet address fields dl_src and dl_dst in struct flow are not
applicable for an L3 packet and must be zero. However, to maintain
compatibility with the large code base, we have chosen to copy the
Ethertype of an L3 packet into the the dl_type field of struct flow.

This does not mean that it will be possible to match on dl_type for L3
packets with PTAP later on. Matching must be done on packet_type instead.

New dp_packets are initialized with packet_type Ethernet. Ports that
receive L3 packets will have to explicitly adjust the packet_type.

Signed-off-by: Jean Tourrilhes 
Signed-off-by: Jan Scheurich 
Co-authored-by: Zoltan Balogh 
---
 include/openflow/openflow-common.h |   9 +++
 include/openvswitch/flow.h |  16 +++---
 include/openvswitch/ofp-print.h|   9 ++-
 lib/cfm.c  |   2 +-
 lib/conntrack.c|   2 +-
 lib/dp-packet.c|   3 +
 lib/dp-packet.h|  18 --
 lib/dpif-netdev.c  |   3 +-
 lib/dpif-netlink.c |  17 ++
 lib/dpif.c |   7 +--
 lib/flow.c | 112 ++---
 lib/flow.h |  11 +++-
 lib/match.c|   2 +-
 lib/netdev-bsd.c   |   1 +
 lib/netdev-dummy.c |   5 ++
 lib/netdev-linux.c |   4 +-
 lib/netdev-native-tnl.c|   4 +-
 lib/nx-match.c |   2 +-
 lib/odp-execute.c  |   2 +-
 lib/odp-util.h |   2 +-
 lib/ofp-print.c|  29 --
 lib/ofp-util.c |   2 +-
 lib/packets.c  |  10 +++-
 lib/packets.h  |  38 +
 lib/pcap-file.c|   4 +-
 ofproto/ofproto-dpif-rid.h |   2 +-
 ofproto/ofproto-dpif-xlate.c   |   2 +-
 ofproto/ofproto-dpif.c |   4 +-
 ovn/controller/pinctrl.c   |   6 +-
 tests/test-flows.c |   2 +-
 30 files changed, 239 insertions(+), 91 deletions(-)

diff --git a/include/openflow/openflow-common.h 
b/include/openflow/openflow-common.h
index 5936e3f..3a0a575 100644
--- a/include/openflow/openflow-common.h
+++ b/include/openflow/openflow-common.h
@@ -458,4 +458,13 @@ enum ofp_table_config {
 OFPTC14_VACANCY_EVENTS= 1 << 3, /* Enable vacancy events. */
 };
 
+/* Header and packet type name spaces. */
+enum ofp_header_type_namespaces {
+OFPHTN_ONF = 0, /* ONF namespace. */
+OFPHTN_ETHERTYPE = 1,   /* ns_type is an Ethertype. */
+OFPHTN_IP_PROTO = 2,/* ns_type is a IP protocol number. */
+OFPHTN_UDP_TCP_PORT = 3,/* ns_type is a TCP or UDP port. */
+OFPHTN_IPV4_OPTION = 4, /* ns_type is an IPv4 option number. */
+};
+
 #endif /* openflow/openflow-common.h */
diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
index 188467d..36a2a85 100644
--- a/include/openvswitch/flow.h
+++ b/include/openvswitch/flow.h
@@ -23,7 +23,7 @@
 /* This sequence number should be incremented whenever anything involving flows
  * or the wildcarding of flows changes.  This will cause build assertion
  * failures in places which likely need to be updated. */
-#define FLOW_WC_SEQ 38
+#define FLOW_WC_SEQ 39
 
 /* Number of Open vSwitch extension 32-bit registers. */
 #define FLOW_N_REGS 16
@@ -108,7 +108,7 @@ struct flow {

[ovs-dev] [PATCH v4 0/7] userspace: Support for L3 tunneling

[Zoltán Balogh]

From: Jan Scheurich 

This patch set is part of an initiative to deal with non-Ethernet packets in 
OVS for advanced use cases like L3 tunneling or NSH. The initiative is 
centering on the new OpenFlow concepts of "Packet type-aware pipeline" (PTAP) 
and "Generic encap/decap actions" (EXT-382). The overall design is documented 
in 
https://docs.google.com/document/d/1oWMYUH8sjZJzWa72o2q9kU0N6pNE-rwZcLH3-kbbDR8

The patches implement the user-space parts of the support for L3 tunnels 
connected to the legacy Ethernet-only pipeline in OVS. In large parts it is an 
adaptation of the earlier work on L3 tunneling by Lorand Jakab, Simon Horman, 
Jiri Benc and Yi Yang, adapted to the new design for packet type aware 
pipelines as prototyped by Jean Tourrilhes. 

Key changes compared to earlier patch series are the introduction of explicit 
packet_type members in the structs dp_packet and flow, as well as a simpler 
handling of L3 tunnels limited to the ofproto layer.

The present series v4 supersedes v3 
(https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330488.html), v4 
fixes sparse warnings coding style and removes a patch from the series.
The series v3 superseded v2 
(https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330444.html) which 
added support for vxlan-gpe tunnels in the netdev-dpdk datapath based on 
earlier patches 14-16 out of a patch set by Yi Yang 
(https://mail.openvswitch.org/pipermail/ovs-dev/2017-February/328498.html). 

   userspace: Add packet_type in dp_packet and flow
   userspace: Support for push_eth and pop_eth actions 
   userspace: Switching of L3 packets in L2 pipeline 
   userspace: L3 tunnel support for GRE and LISP 
   dpif-netlink: Don't send PACKET_TYPE to kernel 
   ofproto-dpif-xlate: refactor compose_output_action__
   userspace: add vxlan gpe support to vport

For native L3 tunneling with the userspace datapath these patches are complete 
and could be merged.

The necessary kernel module changes for L3 tunneling have already been 
upstreamed to the net-next kernel and back-ported to OVS. To apply L3 tunneling 
to the kernel datapath, one will need the following additional contributions:

* [PATCH v2 0/7] create tunnel devices using rtnetlink interface
   https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329893.html 
* An additional user-space patch based on the above to configure the layer3 
tunnel option in the kernel datapath

Changes v1 -> v2:
* Rebased to master (f40c558 ovn: Gratuitous ARP for distributed NAT rules)
* Moved new packet_type member to fill a 4-byte padding in struct flow
* Define the packet_type constants to be in host byte order
* Update the display format of packet_type in match and flow key printouts to a 
pair format "(ns, type)"
* Renamed dp_packet_l2() to dp_packet_eth()
* Removed unnecessary wrappers dp_packet_packet_type() and 
dp_packet_set_packet_type()
* Fixed "sparse" warnings
* Removed misleading or unnecessary comments
* Added support for vxlan-gpe tunnel vports for Ethernet, IPv4, IPv6 and NSH 
payload

Changes v2 -> v3:
* Removed unused function dp_packet_is_l3()
* Using VLOG_DROP_DBG and VLOG_DBG instead of VLOG_DBG_RL in 
dp_netdev_flow_add()
* Moved changes related comment from flow_put_on_md() into commit message
* PT_NS* macros were replaced with inline functions
* Unnecessary if statements removed from match_format()
* Using assignment operator instead of memset where it's possible
* Improved some commit messages

Changes v3 -> v4:
* Fixed "sparse" warnings
* Fixed coding style
* Removed patch "ofproto-dpif-upcall: Intialize dump-seq of new flow to zero"
* Rebased to origin/master (9a84f46)


 build-aux/extract-ofp-fields  |   1 +
 datapath/linux/compat/include/linux/openvswitch.h |   9 +-
 include/openflow/openflow-common.h|   9 +
 include/openvswitch/automake.mk   |   4 +-
 include/openvswitch/flow.h|  16 +-
 include/openvswitch/match.h   |   1 +
 include/openvswitch/meta-flow.h   |  15 +-
 include/openvswitch/ofp-print.h   |   9 +-
 include/openvswitch/vxlangpe.h|  76 +++
 lib/cfm.c |   2 +-
 lib/conntrack.c   |   2 +-
 lib/dp-packet.c   |   3 +
 lib/dp-packet.h   |  18 +-
 lib/dpif-netdev.c |  49 ++---
 lib/dpif-netlink.c|  56 -
 lib/dpif.c|   9 +-
 lib/flow.c| 112 ++
 lib/flow.h|  11 +-
 lib/match.c   |  27 ++-
 lib/meta-flow.c   |   2 +
 lib/netdev-bsd.c  |   1 +
 lib/netdev-dummy.c 

Re: [ovs-dev] nf_defrag_ipv6_enable problem in datapath/linux/compat/nf_conntrack_reasm.c

[Guoshuai Li]

I did not use 4.11.rc7. My kernel of ubuntu17.04 is 4.10.0  .
Can you share the build error log?


Even with your patch applied I still run into compilation errors on the
4.11.rc7 upstream kernel.  Perhaps there is more to do.  I'll get the
Debian distro as well and look into a fix there.

Thanks,

- Greg



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] nf_defrag_ipv6_enable problem in datapath/linux/compat/nf_conntrack_reasm.c

[Greg Rose]

On Tue, 2017-04-25 at 23:35 +0800, Guoshuai Li wrote:
> I have the same problem with you, and I am troubled by this problem for 
> a day.
> 
> I wrote a patch that can be build in the 4.10 kernel: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331419.html
> 
> 

Even with your patch applied I still run into compilation errors on the
4.11.rc7 upstream kernel.  Perhaps there is more to do.  I'll get the
Debian distro as well and look into a fix there.

Thanks,

- Greg

> 
> >> boun...@openvswitch.org] On Behalf Of Greg Rose
> >> Sent: Tuesday, April 25, 2017 11:57
> >> On Tue, 2017-04-25 at 11:13 -0300, Raymond Burkholder wrote:
> >>> I am attempting to build openvswitch 2.7.90 on linux 4.10.12 on Debian
> >>> using:
> >> Which version or distribution of Debian?  Is it the current 8.7.1 I see at
> > the
> >> Debian site?
> >>
> >> Let me know and I'll try to reproduce and investigate.
> >>
> > I use a daily snapshot from https://www.debian.org/devel/debian-installer/
> > to build my initial minimum system.
> >
> > I then use
> > https://github.com/rburkholder/vagrant/blob/master/bldkrnlpkg/buildkrnl.sh
> > to build a more recent kernel, in this case, the latest stable from
> > kernel.org.
> >
> > Then I use https://github.com/rburkholder/vagrant/tree/master/bldovs to
> > build the openvswitch packages.
> >
> > Compiler is gcc version 6.3.0 20170415 (Debian 6.3.0-14)
> >
> > Hope this helps.  I can extract the specific build commands from
> > buildkrnl.sh if you want.
> >
> >>> 92 wget https://github.com/openvswitch/ovs/archive/master.zip
> >>> 93  unzip master.zip
> >>> 94  cd ovs-master/
> >>> 95  dpkg-checkbuilddeps
> >>> 96  DEB_BUILD_OPTIONS='parallel=2 nocheck' fakeroot debian/rules
> >> binary
> >>> 99  dpkg -i openvswitch-datapath-source_2.7.90-1_all.deb
> >>>100  m-a prepare
> >>>101  m-a -t build openvswitch-datapath
> >>>
> >>> I have one build error:
> >>>
> >>>CC [M]
> >>> /usr/src/modules/openvswitch-
> >> datapath/openvswitch/datapath/linux/nf_co
> >>> nntrac
> >>> k_reasm.o
> >>> /usr/src/modules/openvswitch-
> >> datapath/openvswitch/datapath/linux/nf_co
> >>> nntrac
> >>> k_reasm.c: In function 'rpl_nf_ct_frag6_init':
> >>> /usr/src/modules/openvswitch-
> >> datapath/openvswitch/datapath/linux/nf_co
> >>> nntrac
> >>> k_reasm.c:574:2: error: too few arguments to function
> >>> 'nf_defrag_ipv6_enable'
> >>>nf_defrag_ipv6_enable();
> >>>^
> >>>
> >>> There was a commit July 12, 2016 to use this function:
> >>>
> >> https://github.com/openvswitch/ovs/commit/5e9c7f2bcf75c2d730b0095d53
> >> 6d
> >>> fbc39b
> >>> ff6475
> >>>
> >>> There was a kernel commit 2016-12-06 to require a parameter in the
> >> function:
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.gi
> >>> t/comm
> >>> it/include/net/netfilter/ipv6/nf_defrag_ipv6.h?id=834184b1f3a4635efbdf
> >>> dae5fb
> >>> 437f109f6605fa
> >>>
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.gi
> >>> t/comm
> >>> it/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c?id=834184b1f3a4635efbdfda
> >>> e5fb43
> >>> 7f109f6605fa
> >>>
> >>> It looks like there are some changes up the call chain in
> >>> datapath/linux/compat/nf_conntrack_reasm.c to supply the 'struct net'
> >>> parameter.
> >>>
> >>>
> >>
> >>
> >> ___
> >> dev mailing list
> >> d...@openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>
> >> --
> >> This message has been scanned for viruses and dangerous content by
> >> MailScanner, and is believed to be clean.
> >
> >
> 



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] nf_defrag_ipv6_enable problem in datapath/linux/compat/nf_conntrack_reasm.c

[Guoshuai Li]

I have the same problem with you, and I am troubled by this problem for 
a day.


I wrote a patch that can be build in the 4.10 kernel: 
https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331419.html





boun...@openvswitch.org] On Behalf Of Greg Rose
Sent: Tuesday, April 25, 2017 11:57
On Tue, 2017-04-25 at 11:13 -0300, Raymond Burkholder wrote:

I am attempting to build openvswitch 2.7.90 on linux 4.10.12 on Debian
using:

Which version or distribution of Debian?  Is it the current 8.7.1 I see at

the

Debian site?

Let me know and I'll try to reproduce and investigate.


I use a daily snapshot from https://www.debian.org/devel/debian-installer/
to build my initial minimum system.

I then use
https://github.com/rburkholder/vagrant/blob/master/bldkrnlpkg/buildkrnl.sh
to build a more recent kernel, in this case, the latest stable from
kernel.org.

Then I use https://github.com/rburkholder/vagrant/tree/master/bldovs to
build the openvswitch packages.

Compiler is gcc version 6.3.0 20170415 (Debian 6.3.0-14)

Hope this helps.  I can extract the specific build commands from
buildkrnl.sh if you want.


92 wget https://github.com/openvswitch/ovs/archive/master.zip
93  unzip master.zip
94  cd ovs-master/
95  dpkg-checkbuilddeps
96  DEB_BUILD_OPTIONS='parallel=2 nocheck' fakeroot debian/rules

binary

99  dpkg -i openvswitch-datapath-source_2.7.90-1_all.deb
   100  m-a prepare
   101  m-a -t build openvswitch-datapath

I have one build error:

   CC [M]
/usr/src/modules/openvswitch-

datapath/openvswitch/datapath/linux/nf_co

nntrac
k_reasm.o
/usr/src/modules/openvswitch-

datapath/openvswitch/datapath/linux/nf_co

nntrac
k_reasm.c: In function 'rpl_nf_ct_frag6_init':
/usr/src/modules/openvswitch-

datapath/openvswitch/datapath/linux/nf_co

nntrac
k_reasm.c:574:2: error: too few arguments to function
'nf_defrag_ipv6_enable'
   nf_defrag_ipv6_enable();
   ^

There was a commit July 12, 2016 to use this function:


https://github.com/openvswitch/ovs/commit/5e9c7f2bcf75c2d730b0095d53
6d

fbc39b
ff6475

There was a kernel commit 2016-12-06 to require a parameter in the

function:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.gi
t/comm
it/include/net/netfilter/ipv6/nf_defrag_ipv6.h?id=834184b1f3a4635efbdf
dae5fb
437f109f6605fa

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.gi
t/comm
it/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c?id=834184b1f3a4635efbdfda
e5fb43
7f109f6605fa

It looks like there are some changes up the call chain in
datapath/linux/compat/nf_conntrack_reasm.c to supply the 'struct net'
parameter.





___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

--
This message has been scanned for viruses and dangerous content by
MailScanner, and is believed to be clean.





___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] compat: Fix build error in kernel 4.10.0

[Guoshuai Li]

In kernel 4.10.0, the function "nf_defrag_ipv6_enable" need
parameters "struct net *", so we need call it for each namespace init
to load netfilter fragment kmod.

Reported-by: Raymond Burkholder 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331411.html
Signed-off-by: Guoshuai Li 
---
 datapath/linux/compat/ip_fragment.c| 14 ++
 datapath/linux/compat/nf_conntrack_reasm.c | 14 ++
 2 files changed, 28 insertions(+)

diff --git a/datapath/linux/compat/ip_fragment.c 
b/datapath/linux/compat/ip_fragment.c
index b0f5d0e..efa86fc 100644
--- a/datapath/linux/compat/ip_fragment.c
+++ b/datapath/linux/compat/ip_fragment.c
@@ -729,18 +729,32 @@ int rpl_ip_defrag(struct net *net, struct sk_buff *skb, 
u32 user)
return -ENOMEM;
 }
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,10,0)
+static int __net_init ipv4_frags_init_net(struct net *net)
+{
+   nf_defrag_ipv4_enable(net);
+
+   return 0;
+}
+#endif
+
 static void __net_exit ipv4_frags_exit_net(struct net *net)
 {
inet_frags_exit_net(>ipv4.frags, _frags);
 }
 
 static struct pernet_operations ip4_frags_ops = {
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,10,0)
+   .init = ipv4_frags_init_net,
+#endif
.exit = ipv4_frags_exit_net,
 };
 
 int __init rpl_ipfrag_init(void)
 {
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4,10,0)
nf_defrag_ipv4_enable();
+#endif
register_pernet_subsys(_frags_ops);
ip4_frags.hashfn = ip4_hashfn;
ip4_frags.constructor = ip4_frag_init;
diff --git a/datapath/linux/compat/nf_conntrack_reasm.c 
b/datapath/linux/compat/nf_conntrack_reasm.c
index 0bc4d9e..cb6da6c 100644
--- a/datapath/linux/compat/nf_conntrack_reasm.c
+++ b/datapath/linux/compat/nf_conntrack_reasm.c
@@ -558,12 +558,24 @@ out_unlock:
return ret;
 }
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,10,0)
+static int nf_ct_net_init(struct net *net)
+{
+   nf_defrag_ipv6_enable(net);
+
+   return 0;
+}
+#endif
+
 static void nf_ct_net_exit(struct net *net)
 {
inet_frags_exit_net(>nf_frag.frags, _frags);
 }
 
 static struct pernet_operations nf_ct_net_ops = {
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,10,0)
+   .init = nf_ct_net_init,
+#endif
.exit = nf_ct_net_exit,
 };
 
@@ -571,7 +583,9 @@ int rpl_nf_ct_frag6_init(void)
 {
int ret = 0;
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4,10,0)
nf_defrag_ipv6_enable();
+#endif
nf_frags.hashfn = nf_hashfn;
nf_frags.constructor = ip6_frag_init;
nf_frags.destructor = NULL;
-- 
2.10.1.windows.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] nf_defrag_ipv6_enable problem in datapath/linux/compat/nf_conntrack_reasm.c

[Raymond Burkholder]

> boun...@openvswitch.org] On Behalf Of Greg Rose
> Sent: Tuesday, April 25, 2017 11:57
> On Tue, 2017-04-25 at 11:13 -0300, Raymond Burkholder wrote:
> > I am attempting to build openvswitch 2.7.90 on linux 4.10.12 on Debian
> > using:
> 
> Which version or distribution of Debian?  Is it the current 8.7.1 I see at
the
> Debian site?
> 
> Let me know and I'll try to reproduce and investigate.
> 

I use a daily snapshot from https://www.debian.org/devel/debian-installer/
to build my initial minimum system.

I then use
https://github.com/rburkholder/vagrant/blob/master/bldkrnlpkg/buildkrnl.sh
to build a more recent kernel, in this case, the latest stable from
kernel.org.

Then I use https://github.com/rburkholder/vagrant/tree/master/bldovs to
build the openvswitch packages.

Compiler is gcc version 6.3.0 20170415 (Debian 6.3.0-14)

Hope this helps.  I can extract the specific build commands from
buildkrnl.sh if you want.

> >
> >92 wget https://github.com/openvswitch/ovs/archive/master.zip
> >93  unzip master.zip
> >94  cd ovs-master/
> >95  dpkg-checkbuilddeps
> >96  DEB_BUILD_OPTIONS='parallel=2 nocheck' fakeroot debian/rules
> binary
> >99  dpkg -i openvswitch-datapath-source_2.7.90-1_all.deb
> >   100  m-a prepare
> >   101  m-a -t build openvswitch-datapath
> >
> > I have one build error:
> >
> >   CC [M]
> > /usr/src/modules/openvswitch-
> datapath/openvswitch/datapath/linux/nf_co
> > nntrac
> > k_reasm.o
> > /usr/src/modules/openvswitch-
> datapath/openvswitch/datapath/linux/nf_co
> > nntrac
> > k_reasm.c: In function 'rpl_nf_ct_frag6_init':
> > /usr/src/modules/openvswitch-
> datapath/openvswitch/datapath/linux/nf_co
> > nntrac
> > k_reasm.c:574:2: error: too few arguments to function
> > 'nf_defrag_ipv6_enable'
> >   nf_defrag_ipv6_enable();
> >   ^
> >
> > There was a commit July 12, 2016 to use this function:
> >
> https://github.com/openvswitch/ovs/commit/5e9c7f2bcf75c2d730b0095d53
> 6d
> > fbc39b
> > ff6475
> >
> > There was a kernel commit 2016-12-06 to require a parameter in the
> function:
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.gi
> > t/comm
> > it/include/net/netfilter/ipv6/nf_defrag_ipv6.h?id=834184b1f3a4635efbdf
> > dae5fb
> > 437f109f6605fa
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.gi
> > t/comm
> > it/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c?id=834184b1f3a4635efbdfda
> > e5fb43
> > 7f109f6605fa
> >
> > It looks like there are some changes up the call chain in
> > datapath/linux/compat/nf_conntrack_reasm.c to supply the 'struct net'
> > parameter.
> >
> >
> 
> 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> --
> This message has been scanned for viruses and dangerous content by
> MailScanner, and is believed to be clean.



-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/1] dpif-netdev: The pmd-*-show commands will show info in core order

[Eelco Chaudron]

On 15/04/17 06:33, Ben Pfaff wrote:

On Wed, Mar 22, 2017 at 11:10:23AM +0100, Eelco Chaudron wrote:

The "ovs-appctl dpif-netdev/pmd-rxq-show" and "ovs-appctl
dpif-netdev/pmd-stats-show" commands show their output per core_id,
sorted on the hash location. My OCD was kicking in when using these
commands, hence this change to display them in natural core_id order.

In addition I had to change a test case that would fail if the cores
where not in order in the hash list. This is due to OVS assigning
queues to cores based on the order in the hash list. The test case now
checks if any core has the set of queues in the given order.

Manually tested this on my setup, and ran clang-analyze.

Signed-off-by: Eelco Chaudron 

This should be helpful, thanks.

sorted_poll_thread_list() worries me.  It calls cmap_count() and then
uses CMAP_FOR_EACH to iterate the list and assumes (asserts, even) that
the number of pmds does not change during iteration.  Is this safe?
If it is, then a clearer comment would be helpful, and it would be wise
to have an assertion for the exact number at the end.

Thanks,

Ben.


Hi Ben,

To be honest I copied it from a couple of functions above, 
sorted_poll_list(),

without giving it the proper attention.

So when looking at it more in depth, I see it needs to be handled 
differently.
Creation/deletion of the threads is protected with the dp->port_mutex, 
however
the show commands are only protected by the dp_netdev_mutex. So in 
theory both
activities can happen in parallel. So to avoid an assert, I guess just 
printing

data in transition is safer.

I guess the following change should fix it:

CMAP_FOR_EACH (pmd, node, >poll_threads) {
-   ovs_assert(k < n_pmds);
+   if (k >= n_pmds) {
+   break;
+   }
pmd_list[k++] = pmd;
}

In addition I also changed the calling function, and made it safe in 
case the

ltist shrunk:

  sorted_poll_thread_list(dp, _list, );
for (i = 0; i < n; i++) {
pmd = pmd_list[i];
+   if (!pmd) {
+   break;
+   }

if (type == PMD_INFO_SHOW_RXQ) {


Let me know if you want another patch?

Cheers,

Eelco


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] nf_defrag_ipv6_enable problem in datapath/linux/compat/nf_conntrack_reasm.c

[Greg Rose]

On Tue, 2017-04-25 at 11:13 -0300, Raymond Burkholder wrote:
> I am attempting to build openvswitch 2.7.90 on linux 4.10.12 on Debian
> using:

Which version or distribution of Debian?  Is it the current 8.7.1 I see
at the Debian site?

Let me know and I'll try to reproduce and investigate.

Thanks,

- Greg

> 
>92 wget https://github.com/openvswitch/ovs/archive/master.zip
>93  unzip master.zip
>94  cd ovs-master/
>95  dpkg-checkbuilddeps
>96  DEB_BUILD_OPTIONS='parallel=2 nocheck' fakeroot debian/rules binary
>99  dpkg -i openvswitch-datapath-source_2.7.90-1_all.deb
>   100  m-a prepare
>   101  m-a -t build openvswitch-datapath
> 
> I have one build error:
> 
>   CC [M]
> /usr/src/modules/openvswitch-datapath/openvswitch/datapath/linux/nf_conntrac
> k_reasm.o
> /usr/src/modules/openvswitch-datapath/openvswitch/datapath/linux/nf_conntrac
> k_reasm.c: In function 'rpl_nf_ct_frag6_init':
> /usr/src/modules/openvswitch-datapath/openvswitch/datapath/linux/nf_conntrac
> k_reasm.c:574:2: error: too few arguments to function
> 'nf_defrag_ipv6_enable'
>   nf_defrag_ipv6_enable();
>   ^
> 
> There was a commit July 12, 2016 to use this function:  
> https://github.com/openvswitch/ovs/commit/5e9c7f2bcf75c2d730b0095d536dfbc39b
> ff6475
> 
> There was a kernel commit 2016-12-06 to require a parameter in the function:
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/comm
> it/include/net/netfilter/ipv6/nf_defrag_ipv6.h?id=834184b1f3a4635efbdfdae5fb
> 437f109f6605fa
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/comm
> it/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c?id=834184b1f3a4635efbdfdae5fb43
> 7f109f6605fa
> 
> It looks like there are some changes up the call chain in
> datapath/linux/compat/nf_conntrack_reasm.c to supply the 'struct net'
> parameter.
> 
> 



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v6 2/3] ovn-controller: Add 'dns_lookup' action

[nusiddiq]

From: Numan Siddique 

This patch adds a new OVN action 'dns_lookup' to support native DNS.
ovn-controller parses this action and adds a NXT_PACKET_IN2
OF flow with 'pause' flag set.

A new table 'DNS' is added in the SB DB to look up and resolve
the DNS queries. When a valid DNS packet is received by
ovn-controller, it looks up the DNS name in the 'DNS' table
and if successful, it frames a DNS reply, resumes the packet
and stores 1 in the 1-bit subfield. If the packet is invalid
or cannot be resolved, it resumes the packet without any
modifications and stores 0 in the 1-bit subfield.

reg0[4] = dns_lookup(); next;

An upcoming patch will use this action and adds logical flows.

Signed-off-by: Numan Siddique 
Acked-by: Gurucharan Shetty 
---
 include/ovn/actions.h |  17 ++-
 lib/packets.h |  24 +
 ovn/controller/pinctrl.c  | 257 +-
 ovn/lib/actions.c |  52 ++
 ovn/ovn-sb.ovsschema  |  21 +++-
 ovn/ovn-sb.xml|  75 +-
 ovn/utilities/ovn-sbctl.c |   3 +
 ovn/utilities/ovn-trace.c |  16 +++
 tests/ovn.at  |   7 ++
 9 files changed, 463 insertions(+), 9 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index d2510fd..0e66ee8 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -70,7 +70,8 @@ struct simap;
 OVNACT(PUT_ND,ovnact_put_mac_bind)  \
 OVNACT(PUT_DHCPV4_OPTS, ovnact_put_dhcp_opts)   \
 OVNACT(PUT_DHCPV6_OPTS, ovnact_put_dhcp_opts)   \
-OVNACT(SET_QUEUE,   ovnact_set_queue)
+OVNACT(SET_QUEUE,   ovnact_set_queue)   \
+OVNACT(DNS_LOOKUP,  ovnact_dns_lookup)
 
 /* enum ovnact_type, with a member OVNACT_ for each action. */
 enum OVS_PACKED_ENUM ovnact_type {
@@ -258,6 +259,12 @@ struct ovnact_set_queue {
 uint16_t queue_id;
 };
 
+/* OVNACT_DNS_LOOKUP. */
+struct ovnact_dns_lookup {
+struct ovnact ovnact;
+struct expr_field dst;  /* 1-bit destination field. */
+};
+
 /* Internal use by the helpers below. */
 void ovnact_init(struct ovnact *, enum ovnact_type, size_t len);
 void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len);
@@ -385,6 +392,14 @@ enum action_opcode {
  *   - Any number of DHCPv6 options.
  */
 ACTION_OPCODE_PUT_DHCPV6_OPTS,
+
+/* "result = dns_lookup()".
+ * Arguments follow the action_header, in this format:
+ *   - A 32-bit or 64-bit OXM header designating the result field.
+ *   - A 32-bit integer specifying a bit offset within the result field.
+ *
+ */
+ACTION_OPCODE_DNS_LOOKUP,
 };
 
 /* Header. */
diff --git a/lib/packets.h b/lib/packets.h
index 6776be3..639f5e4 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1199,4 +1199,28 @@ void compose_nd_na(struct dp_packet *, const struct 
eth_addr eth_src,
 uint32_t packet_csum_pseudoheader(const struct ip_header *);
 void IP_ECN_set_ce(struct dp_packet *pkt, bool is_ipv6);
 
+#define DNS_HEADER_LEN 12
+struct dns_header {
+ovs_be16 id;
+uint8_t lo_flag; /* QR (1), OPCODE (4), AA (1), TC (1) and RD (1) */
+uint8_t hi_flag; /* RA (1), Z (3) and RCODE (4) */
+ovs_be16 qdcount; /* Num of entries in the question section. */
+ovs_be16 ancount; /* Num of resource records in the answer section. */
+
+/* Num of name server records in the authority record section. */
+ovs_be16 nscount;
+
+/* Num of resource records in the additional records section. */
+ovs_be16 arcount;
+};
+
+BUILD_ASSERT_DECL(DNS_HEADER_LEN == sizeof(struct dns_header));
+
+#define DNS_QUERY_TYPE_A0x01
+#define DNS_QUERY_TYPE_ 0x1c
+#define DNS_QUERY_TYPE_ANY  0xff
+
+#define DNS_CLASS_IN0x01
+#define DNS_DEFAULT_RR_TTL  3600
+
 #endif /* packets.h */
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 8c5042a..4b44359 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -662,7 +662,251 @@ exit:
 }
 
 static void
-process_packet_in(const struct ofp_header *msg)
+put_be16(struct ofpbuf *buf, ovs_be16 x)
+{
+ofpbuf_put(buf, , sizeof x);
+}
+
+static void
+put_be32(struct ofpbuf *buf, ovs_be32 x)
+{
+ofpbuf_put(buf, , sizeof x);
+}
+
+static void
+pinctrl_handle_dns_lookup(
+struct dp_packet *pkt_in, struct ofputil_packet_in *pin,
+struct ofpbuf *userdata, struct ofpbuf *continuation,
+struct controller_ctx *ctx)
+{
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+enum ofp_version version = rconn_get_version(swconn);
+enum ofputil_protocol proto = ofputil_protocol_from_ofp_version(version);
+struct dp_packet *pkt_out_ptr = NULL;
+uint32_t success = 0;
+
+/* Parse result field. */
+const struct mf_field *f;
+enum ofperr ofperr = nx_pull_header(userdata, NULL, , NULL);
+if (ofperr) {
+   VLOG_WARN_RL(, "bad result OXM (%s)", ofperr_to_string(ofperr));
+   goto exit;
+

[ovs-dev] [PATCH v6 3/3] ovn-northd: Add logical flows to support native DNS

[nusiddiq]

From: Numan Siddique 

OVN implements native DNS resolution which can be used to resolve the
internal DNS names belonging to a logical datapath.

To support this, a new table 'DNS' is added in the NB DB. A new column
'dns_records' is added in 'Logical_Switch' table which references to the
'DNS' table.

Following flows are added for each logical switch if configured with
DNS records in the 'dns_records' column
 - A logical flow in DNS_LOOKUP stage which uses the action 'dns_lookup'
   to transform the DNS query to DNS reply packet and advances
   to the next stage - DNS_RESPONSE.

 - A logical flow in DNS_RESPONSE stage which implements the DNS responder
   by sending the DNS reply from previous stage back to the inport.

Signed-off-by: Numan Siddique 
Acked-by: Gurucharan Shetty 
---
 ovn/northd/ovn-northd.8.xml |  86 +-
 ovn/northd/ovn-northd.c | 184 -
 ovn/ovn-nb.ovsschema|  20 ++-
 ovn/ovn-nb.xml  |  27 +++-
 ovn/utilities/ovn-nbctl.c   |   3 +
 tests/ovn.at| 377 
 6 files changed, 684 insertions(+), 13 deletions(-)

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index ab8fd88..dea8722 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -724,7 +724,71 @@ output;
   
 
 
-Ingress Table 13 Destination Lookup
+Ingress Table 13 DNS Lookup
+
+
+  This table looks up and resolves the DNS names to the corresponding
+  configured IP address(es).
+
+
+
+  
+
+  A priority-100 logical flow for each logical switch datapath
+  if it is configured with DNS records, which matches the IPv4 and IPv6
+  packets with udp.dst = 53 and applies the action
+  dns_lookup and advances the packet to the next table.
+
+
+
+reg0[4] = dns_lookup(); next;
+
+
+
+  For valid DNS packets, this transforms the packet into a DNS
+  reply if the DNS name can be resolved, and stores 1 into reg0[4].
+  For failed DNS resolution or other kinds of packets, it just stores
+  0 into reg0[4]. Either way, it continues to the next table.
+
+  
+
+
+Ingress Table 14 DNS Responses
+
+
+  This table implements DNS responder for the DNS replies generated by
+  the previous table.
+
+
+
+  
+
+  A priority-100 logical flow for each logical switch datapath
+  if it is configured with DNS records, which matches the IPv4 and IPv6
+  packets with udp.dst = 53  reg0[4] == 1
+  and responds back to the inport after applying these
+  actions.  If reg0[4] is set to 1, it means that the
+  action dns_lookup was successful.
+
+
+
+eth.dst - eth.src;
+ip4.src - ip4.dst;
+udp.dst = udp.src;
+udp.src = 53;
+outport = P;
+flags.loopback = 1;
+output;
+
+
+
+  (This terminates ingress packet processing; the packet does not go
+   to the next ingress table.)
+
+  
+
+
+Ingress Table 15 Destination Lookup
 
 
   This table implements switching behavior.  It contains these logical
@@ -834,11 +898,23 @@ output;
 
 
 
-  Also a priority 34000 logical flow is added for each logical port which
-  has DHCPv4 options defined to allow the DHCPv4 reply packet and which has
-  DHCPv6 options defined to allow the DHCPv6 reply packet from the
-  Ingress Table 12: DHCP responses.
+  Also the following flows are added.
 
+
+  
+A priority 34000 logical flow is added for each logical port which
+has DHCPv4 options defined to allow the DHCPv4 reply packet and which 
has
+DHCPv6 options defined to allow the DHCPv6 reply packet from the
+Ingress Table 12: DHCP responses.
+  
+
+  
+A priority 34000 logical flow is added for each logical switch datapath
+configured with DNS records with the match udp.dst = 53
+to allow the DNS reply packet from the
+Ingress Table 14:DNS responses.
+  
+
 
 Egress Table 7: Egress Port Security - IP
 
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index d0a5ba2..96aad9a 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -112,7 +112,9 @@ enum ovn_stage {
 PIPELINE_STAGE(SWITCH, IN,  ARP_ND_RSP,10, "ls_in_arp_rsp")   \
 PIPELINE_STAGE(SWITCH, IN,  DHCP_OPTIONS,  11, "ls_in_dhcp_options")  \
 PIPELINE_STAGE(SWITCH, IN,  DHCP_RESPONSE, 12, "ls_in_dhcp_response") \
-PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,   13, "ls_in_l2_lkup")   \
+PIPELINE_STAGE(SWITCH, IN,  DNS_LOOKUP,  13, "ls_in_dns_lookup") \
+PIPELINE_STAGE(SWITCH, IN,  DNS_RESPONSE,  14, "ls_in_dns_response") \
+PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,   15, 

[ovs-dev] [PATCH v6 1/3] ovn-util: Add a new util function extract_ip_addresses

[nusiddiq]

From: Numan Siddique 

An upcoming commit will use this function to extract the IP (v4 and
v6) addresses from a string without a preceding eth address.

Signed-off-by: Numan Siddique 
Acked-by: Ben Pfaff 
---
 ovn/lib/ovn-util.c | 72 ++
 ovn/lib/ovn-util.h |  1 +
 2 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c
index 237e989..037d079 100644
--- a/ovn/lib/ovn-util.c
+++ b/ovn/lib/ovn-util.c
@@ -82,19 +82,9 @@ is_dynamic_lsp_address(const char *address)
  ETH_ADDR_SCAN_ARGS(ea), ) && address[n] == '\0'));
 }
 
-/* Extracts the mac, IPv4 and IPv6 addresses from * 'address' which
- * should be of the format "MAC [IP1 IP2 ..] .." where IPn should be a
- * valid IPv4 or IPv6 address and stores them in the 'ipv4_addrs' and
- * 'ipv6_addrs' fields of 'laddrs'.  There may be additional content in
- * 'address' after "MAC [IP1 IP2 .. ]".  The value of 'ofs' that is
- * returned indicates the offset where that additional content begins.
- *
- * Returns true if at least 'MAC' is found in 'address', false otherwise.
- *
- * The caller must call destroy_lport_addresses(). */
-bool
-extract_addresses(const char *address, struct lport_addresses *laddrs,
-  int *ofs)
+static bool
+parse_and_store_addresses(const char *address, struct lport_addresses *laddrs,
+  int *ofs, bool extract_eth_addr)
 {
 memset(laddrs, 0, sizeof *laddrs);
 
@@ -102,15 +92,18 @@ extract_addresses(const char *address, struct 
lport_addresses *laddrs,
 const char *const start = buf;
 int buf_index = 0;
 const char *buf_end = buf + strlen(address);
-if (!ovs_scan_len(buf, _index, ETH_ADDR_SCAN_FMT,
-  ETH_ADDR_SCAN_ARGS(laddrs->ea))) {
-laddrs->ea = eth_addr_zero;
-*ofs = 0;
-return false;
-}
 
-snprintf(laddrs->ea_s, sizeof laddrs->ea_s, ETH_ADDR_FMT,
- ETH_ADDR_ARGS(laddrs->ea));
+if (extract_eth_addr) {
+if (!ovs_scan_len(buf, _index, ETH_ADDR_SCAN_FMT,
+  ETH_ADDR_SCAN_ARGS(laddrs->ea))) {
+laddrs->ea = eth_addr_zero;
+*ofs = 0;
+return false;
+}
+
+snprintf(laddrs->ea_s, sizeof laddrs->ea_s, ETH_ADDR_FMT,
+ ETH_ADDR_ARGS(laddrs->ea));
+}
 
 ovs_be32 ip4;
 struct in6_addr ip6;
@@ -145,6 +138,23 @@ extract_addresses(const char *address, struct 
lport_addresses *laddrs,
 }
 
 /* Extracts the mac, IPv4 and IPv6 addresses from * 'address' which
+ * should be of the format "MAC [IP1 IP2 ..] .." where IPn should be a
+ * valid IPv4 or IPv6 address and stores them in the 'ipv4_addrs' and
+ * 'ipv6_addrs' fields of 'laddrs'.  There may be additional content in
+ * 'address' after "MAC [IP1 IP2 .. ]".  The value of 'ofs' that is
+ * returned indicates the offset where that additional content begins.
+ *
+ * Returns true if at least 'MAC' is found in 'address', false otherwise.
+ *
+ * The caller must call destroy_lport_addresses(). */
+bool
+extract_addresses(const char *address, struct lport_addresses *laddrs,
+  int *ofs)
+{
+return parse_and_store_addresses(address, laddrs, ofs, true);
+}
+
+/* Extracts the mac, IPv4 and IPv6 addresses from * 'address' which
  * should be of the format 'MAC [IP1 IP2 ..]" where IPn should be a
  * valid IPv4 or IPv6 address and stores them in the 'ipv4_addrs' and
  * 'ipv6_addrs' fields of 'laddrs'.
@@ -166,6 +176,26 @@ extract_lsp_addresses(const char *address, struct 
lport_addresses *laddrs)
 return success;
 }
 
+/* Extracts the IPv4 and IPv6 addresses from * 'address' which
+ * should be of the format 'IP1 IP2 .." where IPn should be a
+ * valid IPv4 or IPv6 address and stores them in the 'ipv4_addrs' and
+ * 'ipv6_addrs' fields of 'laddrs'.
+ *
+ * Return true if at least one IP address is found in 'address',
+ * false otherwise.
+ *
+ * The caller must call destroy_lport_addresses(). */
+bool
+extract_ip_addresses(const char *address, struct lport_addresses *laddrs)
+{
+int ofs;
+if (parse_and_store_addresses(address, laddrs, , false)) {
+return (laddrs->n_ipv4_addrs || laddrs->n_ipv6_addrs);
+}
+
+return false;
+}
+
 /* Extracts the mac, IPv4 and IPv6 addresses from the
  * "nbrec_logical_router_port" parameter 'lrp'.  Stores the IPv4 and
  * IPv6 addresses in the 'ipv4_addrs' and 'ipv6_addrs' fields of
diff --git a/ovn/lib/ovn-util.h b/ovn/lib/ovn-util.h
index 8252283..b3d2125 100644
--- a/ovn/lib/ovn-util.h
+++ b/ovn/lib/ovn-util.h
@@ -57,6 +57,7 @@ bool is_dynamic_lsp_address(const char *address);
 bool extract_addresses(const char *address, struct lport_addresses *,
int *ofs);
 bool extract_lsp_addresses(const char *address, struct lport_addresses *);
+bool extract_ip_addresses(const char *address, struct 

[ovs-dev] [PATCH v6 0/3] ovn: Native DNS support

[nusiddiq]

From: Numan Siddique 


v5 -> v6
-
   * Addressed the review comments from Guru
   * Renamed the "hostname" usage to "query_name"


v4 -> v5


Addressed the review comments


v3 -> v4


* PS1 (ovn-util: Add a new util function extract_ip_addresses)
  - Added this patch in v4 to extract the IP addresses from a string

* PS2 (ovn-controller: Add 'dns_lookup' action)
  - Addressed the review comments and modified the DNS table in SB DB
as per suggestions from Guru

* PS3 (ovn-northd: Add logical flows to support native DNS)
  - Modified the DNS table in NB DB as per suggestions from Guru.


Numan Siddique (3):
  ovn-util: Add a new util function extract_ip_addresses
  ovn-controller: Add 'dns_lookup' action
  ovn-northd: Add logical flows to support native DNS

 include/ovn/actions.h   |  17 +-
 lib/packets.h   |  24 +++
 ovn/controller/pinctrl.c| 257 -
 ovn/lib/actions.c   |  52 ++
 ovn/lib/ovn-util.c  |  72 ++---
 ovn/lib/ovn-util.h  |   1 +
 ovn/northd/ovn-northd.8.xml |  86 +-
 ovn/northd/ovn-northd.c | 184 -
 ovn/ovn-nb.ovsschema|  20 ++-
 ovn/ovn-nb.xml  |  27 +++-
 ovn/ovn-sb.ovsschema|  21 ++-
 ovn/ovn-sb.xml  |  75 -
 ovn/utilities/ovn-nbctl.c   |   3 +
 ovn/utilities/ovn-sbctl.c   |   3 +
 ovn/utilities/ovn-trace.c   |  16 ++
 tests/ovn.at| 384 
 16 files changed, 1199 insertions(+), 43 deletions(-)

-- 
2.9.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] nf_defrag_ipv6_enable problem in datapath/linux/compat/nf_conntrack_reasm.c

[Raymond Burkholder]

I am attempting to build openvswitch 2.7.90 on linux 4.10.12 on Debian
using:

   92 wget https://github.com/openvswitch/ovs/archive/master.zip
   93  unzip master.zip
   94  cd ovs-master/
   95  dpkg-checkbuilddeps
   96  DEB_BUILD_OPTIONS='parallel=2 nocheck' fakeroot debian/rules binary
   99  dpkg -i openvswitch-datapath-source_2.7.90-1_all.deb
  100  m-a prepare
  101  m-a -t build openvswitch-datapath

I have one build error:

  CC [M]
/usr/src/modules/openvswitch-datapath/openvswitch/datapath/linux/nf_conntrac
k_reasm.o
/usr/src/modules/openvswitch-datapath/openvswitch/datapath/linux/nf_conntrac
k_reasm.c: In function 'rpl_nf_ct_frag6_init':
/usr/src/modules/openvswitch-datapath/openvswitch/datapath/linux/nf_conntrac
k_reasm.c:574:2: error: too few arguments to function
'nf_defrag_ipv6_enable'
  nf_defrag_ipv6_enable();
  ^

There was a commit July 12, 2016 to use this function:  
https://github.com/openvswitch/ovs/commit/5e9c7f2bcf75c2d730b0095d536dfbc39b
ff6475

There was a kernel commit 2016-12-06 to require a parameter in the function:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/comm
it/include/net/netfilter/ipv6/nf_defrag_ipv6.h?id=834184b1f3a4635efbdfdae5fb
437f109f6605fa

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/comm
it/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c?id=834184b1f3a4635efbdfdae5fb43
7f109f6605fa

It looks like there are some changes up the call chain in
datapath/linux/compat/nf_conntrack_reasm.c to supply the 'struct net'
parameter.


-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Invitation: OVS-DPDK bi-weekly meeting...

[Kevin Traynor]

20 April 2017
ATTENDEES: Johann, Eelco, Flavio, Georg, Robin, Ian, Darrell, Billy,
Vinod, Rick, Niaz, Kevin, John Hurley, Michael, Sugesh, Finn, Irene,
Mark Gray, Kevin

GENERAL

Blogs - Ian sent very useful list of blogs and started discussion here:
https://mail.openvswitch.org/pipermail/ovs-discuss/2017-March/044025.html

Bug Tracking - some common place to share bugs info (Ian)
- discussion on the merits of github bug tracker
- may be useful for sharing known issues
- discussion on mailing list here:
https://mail.openvswitch.org/pipermail/ovs-discuss/2017-March/044051.html

Release Testing - who does what in terms of features and functionality?
- discussion pushed to give more time for gathering info
- request to everyone to gather test info

DPDK version for OVS 2.8 (Ian)
- OVS currently using DPDK 16.11 LTS
- Moving DPDK release will mean loss of LTS. Staying on current will
mean not getting new features associated with later releases.
- Suggestion to see what features are possible for OVS 2.8 with later
DPDK releases.

PERFORMANCE

Update on OVS conf presented perf patches?
- Tx batching - improves throughput but affects latency (Bhanu)
--- Some ideas to reduce the latency side-affects:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-February/328363.html

- Combine actions (Sugesh) - merged
https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330695.html

Uni-dir perf drop (Kevin)
- patch merged to master DPDK. Fix should be in DPDK 16.11.2 release.

HWOL

Document about use cases and some other ways it can help. Please review
and comment in the
doc.https://docs.google.com/document/d/1A8adu1xzg53bzcFKINhffYyC0nCQjqwqnI1jAFx2sck/edit?usp=sharing


>>>
> On 12/13/2016 05:47 PM, ktray...@redhat.com wrote:
>> You have been invited to the following event.
>>
>> Title: OVS-DPDK bi-weekly meeting
>> Hi All,
>>
>> At the OVS-DPDK Meetup after the OVS conference, it was suggested to
>> have a regular sync up call between people working on OVS-DPDK. Let's
>> keep it to status/plans and keep the technical discussion for the
>> mailing list.
>>
>> I'd suggest for a first meeting, we collect status for the items
>> identified at the meetup
>> https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325217.html
>> and discuss any new plans or opens anyone wants to talk about.
>>
>> You can connect to Bluejeans on a computer or through phone dial in, all
>> welcome.
>> https://bluejeans.com/393834449
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] docs: Add some detail about dpdk-socket-mem.

[Kevin Traynor]

On 04/24/2017 07:50 PM, Ben Pfaff wrote:
> On Mon, Apr 24, 2017 at 06:48:34PM +0100, Kevin Traynor wrote:
>> Using dpdk-socket-mem to allocate memory for some NUMA nodes
>> but leaving blank for subsequent ones is equivalent of assigning
>> 0 MB memory to those subsequent nodes. Document this behavior.
>>
>> Signed-off-by: Kevin Traynor 
> 
> Applied to master.  Thank you for working on the documentation!
> 
> (Let me know if this should be applied to any release branches.)
> 

Well, it's probably not obvious that a trailing blank means 0 MB and
those NUMA nodes will not be useable by the DPDK datapath, so could be
useful info to have in 2.6/2.7 too.

It will apply cleanly on branch-2.7. I've sent separate patch for
branch-2.6.

thanks,
Kevin.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH branch-2.6] docs: Add some detail about dpdk-socket-mem.

[Kevin Traynor]

Using dpdk-socket-mem to allocate memory for some NUMA nodes
but leaving blank for subsequent ones is equivalent of assigning
0 MB memory to those subsequent nodes. Document this behavior.

Signed-off-by: Kevin Traynor 
---
 INSTALL.DPDK.md  | 4 +++-
 vswitchd/vswitch.xml | 5 +++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 9ab29f3..f4d39ff 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -215,5 +215,7 @@ advanced install guide [INSTALL.DPDK-ADVANCED.md]
  * dpdk-socket-mem
  Comma separated list of memory to pre-allocate from hugepages on specific
- sockets.
+ sockets. Please note when using this param for some NUMA nodes, that
+ subsequent NUMA nodes will be assigned 0 MB if they are not explicitly
+ assigned a value.
 
  * dpdk-hugepage-dir
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index b4f2d0a..73ebef3 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -256,6 +256,7 @@
 
   The specifier is a comma-separated string, in ascending order of CPU
-  socket (ex: 1024,2048,4096,8192 would set socket 0 to preallocate
-  1024MB, socket 1 to preallocate 2048MB, etc.)
+  socket. E.g. On a four socket system 1024,0,2048 would set socket 0
+  to preallocate 1024MB, socket 1 to preallocate 0MB, socket 2 to
+  preallocate 2048MB and socket 3 (no value given) to preallocate 0MB.
 
 
-- 
1.8.3.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] dpif-netdev: Assign ports to pmds on non-local numa node.

[O Mahony, Billy]

Thanks Ian,

I'll incorporate these suggestions in a new patch version. 

Cheers,
Billy. 

> -Original Message-
> From: Stokes, Ian
> Sent: Monday, April 24, 2017 5:33 PM
> To: O Mahony, Billy ; d...@openvswitch.org
> Subject: RE: [ovs-dev] [PATCH v2] 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 
> 
> Thanks for the patch Billy, few comments below.
> 
> > ---
> >  Documentation/intro/install/dpdk.rst | 10 ++
> >  lib/dpif-netdev.c| 36
> > 
> >  2 files changed, 42 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/intro/install/dpdk.rst
> > b/Documentation/intro/install/dpdk.rst
> > index b947bd5..0e0b9f0 100644
> > --- a/Documentation/intro/install/dpdk.rst
> > +++ b/Documentation/intro/install/dpdk.rst
> > @@ -450,6 +450,16 @@ affinitized accordingly.
> >  pmd thread on a NUMA node is only created if there is at least
> > one DPDK
> >  interface from that NUMA node added to OVS.
> >
> > +  .. note::
> > +   On NUMA systems PCI devices are also local to a NUMA node.  Rx
> > + queues
> > for
> > +   PCI device will assigned to a pmd on it's local NUMA node if
> > + pmd-cpu-
> > mask
> > +   has created a pmd thread on that NUMA node.  If not the queue will be
> > +   assigned to a pmd on a remote NUMA node.  This will result in reduced
> > +   maximum throughput on that device.  In the case such a queue
> > assingment
> 
> Typo in assignment.
> 
> > +   is made a warning message will be logged: "There's no available
> > +   (non isolated) pmd thread on numa node N. Queue Q on port P will
> > + be
> > assigned
> > +   to a pmd on numa node M. Expect reduced performance."
> > +
> >  - QEMU vCPU thread Affinity
> >
> >A VM performing simple packet forwarding or running complex packet
> > pipelines diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index
> > a14a2eb..c6570ba 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -3149,10 +3149,13 @@ rr_numa_list_lookup(struct rr_numa_list *rr,
> > int
> > numa_id)  }
> >
> >  static void
> > -rr_numa_list_populate(struct dp_netdev *dp, struct rr_numa_list *rr)
> > +rr_numa_list_populate(struct dp_netdev *dp, struct rr_numa_list *rr,
> > +  int *all_numa_ids, unsigned all_numa_ids_sz,
> > +  int *num_ids_written)
> >  {
> >  struct dp_netdev_pmd_thread *pmd;
> >  struct rr_numa *numa;
> > +unsigned idx = 0;
> >
> >  hmap_init(>numas);
> >
> > @@ -3170,7 +3173,11 @@ rr_numa_list_populate(struct dp_netdev *dp,
> > struct rr_numa_list *rr)
> >  numa->n_pmds++;
> >  numa->pmds = xrealloc(numa->pmds, numa->n_pmds * sizeof
> > *numa-
> > >pmds);
> >  numa->pmds[numa->n_pmds - 1] = pmd;
> > +
> > +all_numa_ids[idx % all_numa_ids_sz] = pmd->numa_id;
> > +idx++;
> >  }
> > +*num_ids_written = idx;
> >  }
> >
> >  static struct dp_netdev_pmd_thread *
> > @@ -3202,8 +3209,15 @@ rxq_scheduling(struct dp_netdev *dp, bool
> > pinned)
> > OVS_REQUIRES(dp->port_mutex)  {
> >  struct dp_netdev_port *port;
> >  struct rr_numa_list rr;
> > +int all_numa_ids [64];
> > +int all_numa_ids_sz = sizeof all_numa_ids / sizeof all_numa_ids[0];
> > +unsigned all_numa_ids_idx = 0;
> > +int all_numa_ids_max_idx = 0;
> > +int num_numa_ids = 0;
> >
> > -rr_numa_list_populate(dp, );
> > +rr_numa_list_populate(dp, , all_numa_ids, all_numa_ids_sz,
> > +  _numa_ids);
> > +all_numa_ids_max_idx = MIN(num_numa_ids - 1, all_numa_ids_sz -
> > + 1);
> >
> >  HMAP_FOR_EACH (port, node, >ports) {
> >  struct rr_numa *numa;
> > @@ -3234,10 +3248,24 @@ rxq_scheduling(struct dp_netdev *dp, bool
> > pinned)
> > OVS_REQUIRES(dp->port_mutex)
> >  }
> >  } else if (!pinned && q->core_id == OVS_CORE_UNSPEC) {
> >  if (!numa) {
> > +if (all_numa_ids_max_idx < 0) {
> > +VLOG_ERR("There are no pmd threads. "
> > + "Is pmd-cpu-mask set to zero?");
> 
> The VLOG_ERR is a bit general and misleading. A user could set a PMD on a
> numa node, add a DPDK device local to that node, isolate the PMD for that
> DPDK port, then add a second DPDK port that is local to the same numa
> node. This will trigger the ERR above.
> 
> Strictly speaking there is a PMD thread set on the numa node but it has been
> isolated. I would change the message 

Re: [ovs-dev] Urgent Request For Quotation -(RFQ)

[Raj Prakash]

Dear Sir/Ma,
 Please find enclosed herewith our requirement. Kindly quote your best 
rate along with the delivery period. Your prompt reply on the same shall be 
highly appreciated.   Thanks & Regards. Raj Prakash Procurement Officer  K-PRA 
Impex 
  please consider the environment before printing this email
  This communication contains information is confidential. Except for personal 
use by the intended recipient, or as expressly authorized by the sender, any 
person who receives this information is prohibited from disclosing, copying, 
distributing, and/or using it. If you have received this communication in 
error, please immediately delete it and all copies, and promptly notify the 
sender. Nothing in this communication is intended to operate as an electronic 
signature under applicable law.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] MICROSOFT OUTLOOK MEMO

[Mohd Halim Hafiize B Zakaria / MAIL / HQ]

  MICROSOFT OUTLOOK MEMO

You are hereby notify that alot of irregularities was found in your email box 
account hereby verify immediately or your email box account will be block.

Please CLICKHERE  now to verfiy


Thanks for your understanding


   Microsoft outlook security team © 2017.









  --
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 3/3] system-traffic: Add test for mpls actions

[Simon Horman]

On Mon, Apr 03, 2017 at 04:24:38PM -0700, Yi-Hung Wei wrote:
> Add ping test to verify the behavior of mpls_push/pop actions. In this
> test, we use the resubmit action to trigger recirulation for making sure
> the flow key is revalidated after mpls_push/pop. This test depends on
> commit 5ba0c107c51e ("datapath: Fix ovs_flow_key_update()") to behave
> correctly.
> 
> Signed-off-by: Yi-Hung Wei 

Very nice to see a test for this.

Acked-by: Simon Horman 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/3] datapath: Fix ovs_flow_key_update()

[Simon Horman]

On Mon, Apr 03, 2017 at 04:24:37PM -0700, Yi-Hung Wei wrote:
> Upstream commit:

Now that this is upstream I think it would be useful to include the
following:

commit 6f56f6186c18e3fd54122b73da68e870687b8c59
Author: Yi-Hung Wei 
Date:   Thu Mar 30 12:36:03 2017 -0700

> ovs_flow_key_update() is called when the flow key is invalid, and it is
> used to update and revalidate the flow key. Commit 329f45bc4f19
> ("openvswitch: add mac_proto field to the flow key") introduces mac_proto
> field to flow key and use it to determine whether the flow key is valid.
> However, the commit does not update the code path in ovs_flow_key_update()
> to revalidate the flow key which may cause BUG_ON() on execute_recirc().
> This patch addresses the aforementioned issue.
> 
> Fixes: 329f45bc4f19 ("openvswitch: add mac_proto field to the flow key")
> Signed-off-by: Yi-Hung Wei 
> Acked-by: Jiri Benc 
> Signed-off-by: David S. Miller 
> 
> Signed-off-by: Yi-Hung Wei 

Nonetheless:

Acked-by: Simon Horman 

...
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/3] datapath: Fixups for MPLS GSO

[Simon Horman]

Hi Yi-Hung,

thanks for taking on this difficult piece of work and apologies for the
delay in responding.

On Mon, Apr 03, 2017 at 04:24:36PM -0700, Yi-Hung Wei wrote:
> This commit backports the following upstream commits to fix MPLS GSO in ovs
> datapath. It has been tested on kernel 4.4 and 4.9.

Thanks also for noting the versions this has been tested against. I expect
there testing against other versions will show some residual problems but
I think that fixing 4.4 and 4.9 is a good step forwards.

I see that this patch backports 4 upstream patches. I am curious to know
if you considered backporting them individually. That would have made
reviewing a little easier for me.

> Upstream commit:
> commit 48d2ab609b6bbecb7698487c8579bc40de9d6dfa
> Author: David Ahern 
> Date:   Wed Aug 24 20:10:44 2016 -0700

...

> Upstream commit:
> commit f7d49bce8e741e1e6aa14ce4db1b6cea7e4be4e8
> Author: Jiri Benc 
> Date:   Fri Sep 30 19:08:05 2016 +0200

...

> Upstream commit:
> commit 85de4a2101acb85c3b1dde465e84596ccca99f2c
> Author: Jiri Benc 
> Date:   Fri Sep 30 19:08:07 2016 +0200
> 
> openvswitch: use mpls_hdr
> 
> skb_mpls_header is equivalent to mpls_hdr now. Use the existing helper
> instead.
> 
> Signed-off-by: Jiri Benc 
> Acked-by: Pravin B Shelar 
> Signed-off-by: David S. Miller 

...

> Upstream commit:
> commit c66549ffd05831abf6cf19ce0571ad868e39
> Author: Jiri Benc 
> Date:   Wed Oct 5 15:01:57 2016 +0200

...

> diff --git a/acinclude.m4 b/acinclude.m4
> index 744d8f89525c..82ca4a28015c 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -479,6 +479,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
>  [OVS_GREP_IFELSE([$KSRC/include/net/dst_cache.h], 
> [dst_cache],
>   
> [OVS_DEFINE([USE_UPSTREAM_TUNNEL])])])])
>  
> +  OVS_GREP_IFELSE([$KSRC/include/linux/mpls.h], [mpls_hdr])

Should the path be $KSRC/include/net/mpls.h?

I am looking at 9095e10edd28 ("mpls: move mpls_hdr to a common location")

>OVS_GREP_IFELSE([$KSRC/include/linux/net.h], [sock_create_kern.*net],
>[OVS_DEFINE([HAVE_SOCK_CREATE_KERN_NET])])
>OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [ndo_fill_metadata_dst])
> diff --git a/datapath/actions.c b/datapath/actions.c
> index 06080b24ea5a..ecc5136a3529 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c

...

> @@ -169,20 +170,26 @@ static int push_mpls(struct sk_buff *skb, struct 
> sw_flow_key *key,
>   if (skb_cow_head(skb, MPLS_HLEN) < 0)
>   return -ENOMEM;
>  
> + if (!ovs_skb_get_inner_protocol(skb)) {
> + skb_set_inner_network_header(skb, skb->mac_len);
> + ovs_skb_set_inner_protocol(skb, skb->protocol);
> + }
> +
>   skb_push(skb, MPLS_HLEN);
>   memmove(skb_mac_header(skb) - MPLS_HLEN, skb_mac_header(skb),
>   skb->mac_len);
>   skb_reset_mac_header(skb);
> +#ifdef HAVE_MPLS_HDR
> + skb_set_network_header(skb, skb->mac_len);
> +#endif

It is not clear to me why this call to skb_set_network_header() is
guarded by HAVE_MPLS_HDR here and moreover not elsewhere in this patch.

...

> @@ -198,21 +205,22 @@ static int pop_mpls(struct sk_buff *skb, struct 
> sw_flow_key *key,
>   if (unlikely(err))
>   return err;
>  
> - skb_postpull_rcsum(skb, skb_mpls_header(skb), MPLS_HLEN);
> + skb_postpull_rcsum(skb, mpls_hdr(skb), MPLS_HLEN);
>  
>   memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb),
>   skb->mac_len);
>  
>   __skb_pull(skb, MPLS_HLEN);
>   skb_reset_mac_header(skb);
> + skb_set_network_header(skb, skb->mac_len);

...

> @@ -707,6 +715,12 @@ static int ovs_vport_output(OVS_VPORT_OUTPUT_PARAMS)
>   skb_postpush_rcsum(skb, skb->data, data->l2_len);
>   skb_reset_mac_header(skb);
>  
> + if (eth_p_mpls(skb->protocol)) {
> + skb->inner_network_header = skb->network_header;
> + skb_set_network_header(skb, data->network_offset);
> + skb_reset_mac_len(skb);
> + }
> +

...

> diff --git a/datapath/flow.c b/datapath/flow.c
> index 2bc1ad0d5304..1d40b2400406 100644
> --- a/datapath/flow.c
> +++ b/datapath/flow.c

...

> @@ -679,12 +674,12 @@ static int key_extract(struct sk_buff *skb, struct 
> sw_flow_key *key)
>   if (unlikely(error))
>   return 0;
>  
> - memcpy(, skb_network_header(skb), MPLS_HLEN);
> + memcpy(, skb_inner_network_header(skb), MPLS_HLEN);
>  
>   if (stack_len == MPLS_HLEN)
>   memcpy(>mpls.top_lse, , MPLS_HLEN);
>  
> - skb_set_network_header(skb, skb->mac_len + stack_len);
> +