[ovs-dev] [PATCH] tunnel: cleanup geneve md before forwarding to another port.

[William Tu]

When there is a flow rule which forwards a packet from geneve
port to another tunnel port, ex: gre, the tun_metadata carried
from the geneve port needs to be clean up.  Otherwise, the output
tunnel port will incorrectly have geneve option being set.

For example, without the patch, the datapath action output to gre
port (1) shows:
  set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64,
geneve({class=0x,type=0,len=4,0x123}),flags(df|key))),1
Where the geneve(...) should not exist.

When using kernel's tunnel port, this triggers an error saying:
"Multiple metadata blocks provided", when there is a rule forwarding
the geneve packet to vxlan/erspan tunnel port.  A userspace test case
using geneve and gre also demonstrates the issue.

Reported-by: Xiaoyan Jin 
Signed-off-by: William Tu 
Cc: Greg Rose 
---
 ofproto/tunnel.c |  6 ++
 tests/tunnel.at  | 18 ++
 2 files changed, 24 insertions(+)

diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index e0214ced69e2..00104091c991 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -403,6 +403,7 @@ tnl_port_send(const struct ofport_dpif *ofport, struct flow 
*flow,
 struct tnl_port *tnl_port;
 char *pre_flow_str = NULL;
 odp_port_t out_port;
+const char *type;
 
 fat_rwlock_rdlock(&rwlock);
 tnl_port = tnl_find_ofport(ofport);
@@ -474,6 +475,11 @@ tnl_port_send(const struct ofport_dpif *ofport, struct 
flow *flow,
 wc->masks.pkt_mark = UINT32_MAX;
 }
 
+type = netdev_get_type(tnl_port->netdev);
+if (strcmp("geneve", type)) {
+memset(&flow->tunnel.metadata, 0, sizeof(flow->tunnel.metadata));
+}
+
 if (pre_flow_str) {
 char *post_flow_str = flow_to_string(flow, NULL);
 char *tnl_str = tnl_port_to_string(tnl_port);
diff --git a/tests/tunnel.at b/tests/tunnel.at
index 3c217b344f9b..34e1e883a68f 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -708,5 +708,23 @@ AT_CHECK([tail -2 stdout], [0],
 Datapath actions: set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64,flags(df|key))),1
 ])
 
+AT_CHECK([ovs-ofctl add-tlv-map br0 
"{class=0x,type=0,len=4}->tun_metadata0"])
+AT_CHECK([ovs-ofctl del-flows br0])
+
+AT_DATA([flows2.txt], [dnl
+priority=1,in_port=1,tun_metadata0=0x123, actions=3
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows2.txt])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0x,type=0,len=4,0x123}),flags(csum|key)),in_port(6081),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'],
 [0], [stdout])
+AT_CHECK([tail -2 stdout], [0],
+  [Megaflow: 
recirc_id=0,eth,ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=-df+csum+key,tun_metadata0=0x123,in_port=1,nw_ecn=0,nw_frag=no
+Datapath actions: set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64,flags(df|key))),1
+])
+
+dnl with the fix, the actions still have geneve options:
+dnl 
set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64,geneve({class=0x,type=0,len=4,0x123}),flags(df|key))),1
+dnl which is not correct
+
 OVS_VSWITCHD_STOP
 AT_CLEANUP
-- 
2.7.4

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

Re: [ovs-dev] [PATCH] Avoid crash in OvS while transmitting fragmented packets over tunnel.

[Rohith Basavaraja]

Hi Darell,

I reproduce the issue by making VM to transmit fragmented packets and in OvS if 
we have corresponding rule
to transmit the received fragmented packet (from VM) over tunnel then I always 
hit the crash.

Thanks
Rohith

On 11/05/18, 2:16 AM, "Darrell Ball"  wrote:

Thanks Rohith
I see this patch was applied, but I have one question inline.


On 4/20/18, 1:48 AM, "ovs-dev-boun...@openvswitch.org on behalf of Rohith 
Basavaraja"  wrote:

Currently when fragmented packets are to be transmitted in to tunnel,
base_flow->nw_frag which was initially non-zero at reception is not
reset to zero when the base_flow and flow are rewritten
as part of the emulated tnl_push action in the ofproto-dpif-xlate
module.

Because of this when fragmented packets are transmitted out of tunnel,
we hit crash caused by the following assert.

lib/odp-util.c:5654: assertion flow->nw_proto == base_flow->nw_proto &&
flow->nw_frag == base_flow->nw_frag failed in commit_set_ipv4_action()

Can you describe how you hit this assertion?
I have some testing in and around this code, but have not hit this yet, so 
I was curious?

With the following change propagate_tunnel_data_to_flow__
is modified to reset *nw_frag* to zero. 


Also, that currently we don't
fragment tunnelled packets, we should reset *nw_frag* to zero in
propagate_tunnel_data_to_flow__.

Signed-off-by: Jan Scheurich 
From: Rohith Basavaraja 
CC: Jan Scheurich 

---
 ofproto/ofproto-dpif-xlate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 94e3ddb..e9ed037 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3310,6 +3310,7 @@ propagate_tunnel_data_to_flow__(struct flow 
*dst_flow,
 dst_flow->ipv6_dst = src_flow->tunnel.ipv6_dst;
 dst_flow->ipv6_src = src_flow->tunnel.ipv6_src;
 
+dst_flow->nw_frag = 0; /* Tunnel packets are unfragmented. */


 dst_flow->nw_tos = src_flow->tunnel.ip_tos;
 dst_flow->nw_ttl = src_flow->tunnel.ip_ttl;
 dst_flow->tp_dst = src_flow->tunnel.tp_dst;
-- 
1.9.1




___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=6ZMZ7J9yNmX0ZQRMQyUfQ8fZrhemcMFiUqpnVD_jN9w&s=lYu98hfGnEvKr7YcK50fxDDY9-d0mA3W0yYtpSeeIQo&e=






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

Re: [ovs-dev] [PATCH v2 1/5] ovs-kmod-ctl: introduce a kernel module load script

[Ansis Atteka]

On Fri, 4 May 2018 at 11:28, Aaron Conole  wrote:

> Currently, Open vSwitch on linux embeds the logic of loading and unloading
> kernel modules into the ovs-ctl and ovs-lib script files.  This works, but
> it means that there is no way to leverage extended filesystem attributes
> to grant fine grain permissions relating to module loading.

> The split out utility 'ovs-kmod-ctl' will be used in an upcoming commit
> for RHEL-based distributions to have a separate transition domain that
> will allow module loading to be given to a separate selinux domain from
> the openvswitch_t domain.

One thing I have been thinking about recently is how we could containerize
Open vSwitch (not sure if that is even possible in feasible way).

The idea would be that there would be, for example, Ubuntu based container
running OVS user space daemons installed from our deb packages. And the
"container host" would have Open vSwitch kernel module installed from our
dkms or kmod rpm packages. (or vice versa).

Such design, I think, inevitably would require sometihing like ovs-kmod-ctl
utility distributed with the dkms or kmod kernel module package.

This is something that does not requre action w.r.t. your series, but I
would be interested to hear your opinion if you have thought how to make
that happen...

> Acked-By: Timothy Redaelli 
> Signed-off-by: Aaron Conole 
> ---
>   debian/openvswitch-switch.install  |   1 +
>   debian/openvswitch-switch.manpages |   1 +
>   rhel/openvswitch-fedora.spec.in|   2 +
>   rhel/openvswitch.spec.in   |   2 +
>   utilities/.gitignore   |   1 +
>   utilities/automake.mk  |   5 +
>   utilities/ovs-ctl.in   |  32 +-
>   utilities/ovs-kmod-ctl.8   | 109 ++
>   utilities/ovs-kmod-ctl.in  | 228
+
>   utilities/ovs-lib.in   |  12 +-
>   10 files changed, 356 insertions(+), 37 deletions(-)
>   create mode 100644 utilities/ovs-kmod-ctl.8
>   create mode 100644 utilities/ovs-kmod-ctl.in

> diff --git a/debian/openvswitch-switch.install
b/debian/openvswitch-switch.install
> index bfb391fe8..6a6e9a543 100644
> --- a/debian/openvswitch-switch.install
> +++ b/debian/openvswitch-switch.install
> @@ -12,5 +12,6 @@ usr/sbin/ovs-vswitchd
>   usr/sbin/ovsdb-server
>   usr/share/openvswitch/scripts/ovs-check-dead-ifs
>   usr/share/openvswitch/scripts/ovs-ctl
> +usr/share/openvswitch/scripts/ovs-kmod-ctl
>   usr/share/openvswitch/scripts/ovs-save
>   usr/share/openvswitch/vswitch.ovsschema
> diff --git a/debian/openvswitch-switch.manpages
b/debian/openvswitch-switch.manpages
> index c85cbfd30..1161cfda7 100644
> --- a/debian/openvswitch-switch.manpages
> +++ b/debian/openvswitch-switch.manpages
> @@ -3,6 +3,7 @@ ovsdb/ovsdb-server.5
>   utilities/ovs-ctl.8
>   utilities/ovs-dpctl-top.8
>   utilities/ovs-dpctl.8
> +utilities/ovs-kmod-ctl.8
>   utilities/ovs-pcap.1
>   utilities/ovs-tcpdump.8
>   utilities/ovs-tcpundump.1
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/
openvswitch-fedora.spec.in
> index 3e5cf21e0..bf4526de2 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -545,6 +545,7 @@ fi
>   %{_datadir}/openvswitch/scripts/ovs-save
>   %{_datadir}/openvswitch/scripts/ovs-vtep
>   %{_datadir}/openvswitch/scripts/ovs-ctl
> +%{_datadir}/openvswitch/scripts/ovs-kmod-ctl
>   %{_datadir}/openvswitch/scripts/ovs-systemd-reload
>   %config %{_datadir}/openvswitch/vswitch.ovsschema
>   %config %{_datadir}/openvswitch/vtep.ovsschema
> @@ -578,6 +579,7 @@ fi
>   %{_mandir}/man8/ovs-ctl.8*
>   %{_mandir}/man8/ovs-dpctl.8*
>   %{_mandir}/man8/ovs-dpctl-top.8*
> +%{_mandir}/man8/ovs-kmod-ctl.8*
>   %{_mandir}/man8/ovs-ofctl.8*
>   %{_mandir}/man8/ovs-pki.8*
>   %{_mandir}/man8/ovs-vsctl.8*
> diff --git a/rhel/openvswitch.spec.in b/rhel/openvswitch.spec.in
> index 2c5f0409a..883d25607 100644
> --- a/rhel/openvswitch.spec.in
> +++ b/rhel/openvswitch.spec.in
> @@ -237,6 +237,7 @@ exit 0
>   /usr/share/man/man8/ovs-ctl.8.gz
>   /usr/share/man/man8/ovs-dpctl.8.gz
>   /usr/share/man/man8/ovs-dpctl-top.8.gz
> +/usr/share/man/man8/ovs-kmod-ctl.8.gz
>   /usr/share/man/man8/ovs-ofctl.8.gz
>   /usr/share/man/man8/ovs-parse-backtrace.8.gz
>   /usr/share/man/man8/ovs-pki.8.gz
> @@ -250,6 +251,7 @@ exit 0
>   /usr/share/openvswitch/scripts/ovs-bugtool-*
>   /usr/share/openvswitch/scripts/ovs-check-dead-ifs
>   /usr/share/openvswitch/scripts/ovs-ctl
> +/usr/share/openvswitch/scripts/ovs-kmod-ctl
>   /usr/share/openvswitch/scripts/ovs-lib
>   /usr/share/openvswitch/scripts/ovs-save
>   /usr/share/openvswitch/scripts/ovs-vtep
> diff --git a/utilities/.gitignore b/utilities/.gitignore
> index 34c58f20f..eb2a69bf3 100644
> --- a/utilities/.gitignore
> +++ b/utilities/.gitignore
> @@ -13,6 +13,7 @@
>   /ovs-dpctl.8
>   /ovs-dpctl-top
>   /ovs-dpctl-top.8
> +/ovs-kmod-ctl
>   /ovs-l3ping
>   /ovs-l3ping.8
>   /ovs-lib
> diff --git a/utilities/automake.mk b/utilities/au

Re: [ovs-dev] [PATCH v2] ovn-nbctl: Support ACL commands on port groups.

[Ben Pfaff]

On Wed, May 09, 2018 at 11:32:04PM -0700, Han Zhou wrote:
> Add support for using ovn-nbctl to add/delete/list ACLs on port
> groups.
> 
> A new option --type is also supported for these commands to
> explicitely specify, when needed, whether the operation is on a
> port-group or a logical switch. E.g.
> 
> ovn-nbctl --type=port-group acl-add port_group1 to-lport 1000 \
> 'outport == @port_group1 && ip4.src == $port_group1_ip4' \
>  allow-related
> 
> Signed-off-by: Han Zhou 

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

Re: [ovs-dev] [PATCH v2] Add mirror/ovs-tcpdump support for slow protocols' Rx path.

[Ben Pfaff]

On Thu, May 10, 2018 at 06:56:22AM +, Manohar Krishnappa Chidambaraswamy 
wrote:
> Ben,
> 
> Here is the v2 patch based on your suggestion. Please let me know if you have 
> any other comments.

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

Re: [ovs-dev] [PATCH v1] Fix crash when processing malformed Bundle Add message in OVS

[Ben Pfaff]

Thanks.  I applied this to master and branch-2.9.

On Thu, May 10, 2018 at 09:13:41AM +, Anju Thomas wrote:
> Hi Ben,
> 
> Yes. This patch certainly looks more cleaner and better to me .
> 
> Thanks
> Anju
>  -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org] 
> Sent: Thursday, May 10, 2018 2:33 AM
> To: Anju Thomas 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v1] Fix crash when processing malformed Bundle 
> Add message in OVS
> 
> On Mon, May 07, 2018 at 10:58:06PM +0530, Anju Thomas wrote:
> > When an OpenFlow Bundle Add message is received, a bundle entry is 
> > created and the OpenFlow message embedded in the bundle add message is 
> > processed.  If any error is encountered while processing the embedded 
> > message, the bundle entry is freed. The bundle entry free function 
> > assumes that the entry has been populated with a properly formatted 
> > OpenFlow message and performs some message specific cleanup actions .
> > This assumption does not hold true in the error case and OVS crashes 
> > when performing the cleanup.
> > 
> > The fix is in case of errors, simply free the bundle entry without 
> > attempting to perform any embedded message cleanup
> > 
> > Signed-off-by: Anju Thomas 
> 
> Thanks for the fix.
> 
> It's not really nice to have multiple levels of initialized-ness.  What do 
> you think of this fix instead?
> 
> Thanks,
> 
> Ben.
> 
> --8<--cut here-->8--
> 
> From: Anju Thomas 
> Date: Mon, 7 May 2018 22:58:06 +0530
> Subject: [PATCH] Fix crash when processing malformed Bundle Add message in OVS
> 
> When an OpenFlow Bundle Add message is received, a bundle entry is created 
> and the OpenFlow message embedded in the bundle add message is processed.  If 
> any error is encountered while processing the embedded message, the bundle 
> entry is freed. The bundle entry free function assumes that the entry has 
> been populated with a properly formatted OpenFlow message and performs some 
> message specific cleanup actions .
> This assumption does not hold true in the error case and OVS crashes when 
> performing the cleanup.
> 
> The fix is in case of errors, simply free the bundle entry without attempting 
> to perform any embedded message cleanup
> 
> Signed-off-by: Anju Thomas 
> Signed-off-by: Ben Pfaff 
> ---
>  ofproto/bundles.h | 13 -
>  ofproto/ofproto.c | 21 +
>  2 files changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/ofproto/bundles.h b/ofproto/bundles.h index 
> 806e853d6883..1164fddd8702 100644
> --- a/ofproto/bundles.h
> +++ b/ofproto/bundles.h
> @@ -58,8 +58,6 @@ struct ofp_bundle {
>  struct ovs_list   msg_list;  /* List of 'struct bundle_message's */
>  };
>  
> -static inline struct ofp_bundle_entry *ofp_bundle_entry_alloc(
> -enum ofptype type, const struct ofp_header *oh);
>  static inline void ofp_bundle_entry_free(struct ofp_bundle_entry *);
>  
>  enum ofperr ofp_bundle_open(struct ofconn *, uint32_t id, uint16_t flags, @@ 
> -72,17 +70,6 @@ enum ofperr ofp_bundle_add_message(struct ofconn *, uint32_t 
> id,
>  
>  void ofp_bundle_remove__(struct ofconn *, struct ofp_bundle *);
>  
> 
> -static inline struct ofp_bundle_entry * -ofp_bundle_entry_alloc(enum ofptype 
> type, const struct ofp_header *oh) -{
> -struct ofp_bundle_entry *entry = xmalloc(sizeof *entry);
> -
> -entry->type = type;
> -entry->msg = xmemdup(oh, ntohs(oh->length));
> -
> -return entry;
> -}
> -
>  static inline void
>  ofp_bundle_entry_free(struct ofp_bundle_entry *entry)  { diff --git 
> a/ofproto/ofproto.c b/ofproto/ofproto.c index 36f4c0b16d48..565a45db1507 
> 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -7899,7 +7899,6 @@ handle_bundle_add(struct ofconn *ofconn, const struct 
> ofp_header *oh)
>  struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
>  enum ofperr error;
>  struct ofputil_bundle_add_msg badd;
> -struct ofp_bundle_entry *bmsg;
>  enum ofptype type;
>  
>  error = reject_slave_controller(ofconn); @@ -7912,7 +7911,8 @@ 
> handle_bundle_add(struct ofconn *ofconn, const struct ofp_header *oh)
>  return error;
>  }
>  
> -bmsg = ofp_bundle_entry_alloc(type, badd.msg);
> +/* Allocate bundle entry and decode the embedded message. */
> +struct ofp_bundle_entry *bmsg = xmalloc(sizeof *bmsg);
>  
>  struct ofpbuf ofpacts;
>  uint64_t ofpacts_stub[1024 / 8];
> @@ -7951,18 +7951,23 @@ handle_bundle_add(struct ofconn *ofconn, const struct 
> ofp_header *oh)
>  } else {
>  OVS_NOT_REACHED();
>  }
> -
>  ofpbuf_uninit(&ofpacts);
> -
> -if (!error) {
> -error = ofp_bundle_add_message(ofconn, badd.bundle_id, badd.flags,
> -   bmsg, oh);
> +if (error) {
> +free(bmsg);
> +return error;
>  }
>  
> +/* Now that the embedded message has been successfully decoded, finish up
> +

Re: [ovs-dev] [PATCH v3] ovn: Set proper Neighbour Adv flag when replying for NS request for router IP

[Ben Pfaff]

On Thu, May 10, 2018 at 11:18:23PM +0530, nusid...@redhat.com wrote:
> From: Numan Siddique 
> 
> Presently when a VM's IPv6 stack sends a Neighbor Solicitation request for its
> router IP, (mostly when the ND cache entry for the router is in STALE state)
> ovn-controller responds with a Neighbor Adv packet (using the action nd_na).
> But it doesn't set 'ND_RSO_ROUTER' in the RSO flags (please see RFC4861 page 
> 23).
> Because of which, the VM deletes the default route. The default route gets 
> added
> again when the next RA is received (but would again gets deleted if its sends
> NS request). And this results in disruption of IPv6 traffic.
> 
> This patch addresses this issue by adding a new action 'nd_na_router' which is
> same as 'nd_na' but it sets the 'ND_RSO_ROUTER' in the RSO flags. ovn-northd
> uses this action. A new action is added instead of modifying the existing 
> 'nd_na'
> action. This is because
>   - We cannot set the RSO flags in the "nd_na { ..actions .. }"
>   - It would be ugly to have something like nd_na { router_flags, ...actions 
> .. }
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1567735
> Signed-off-by: Numan Siddique 
> Acked-by: Mark Michelson 

Please document the new action in ovn/ovn-sb.xml.

Otherwise this looks good to me, thank you!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] tests: Fix typo in test name.

[Ben Pfaff]

Thanks, applied to master.

On Thu, May 10, 2018 at 08:46:58PM +0300, Alin Gabriel Serdean wrote:
> Acked-by: Alin Gabriel Serdean mailto:aserd...@ovn.org>>
> 
> > On 10 May 2018, at 20:26, Ben Pfaff  wrote:
> > 
> > On Fri, Apr 13, 2018 at 10:38:53AM -0700, Ben Pfaff wrote:
> >> Signed-off-by: Ben Pfaff 
> > 
> > This still needs a review.
> > ___
> > 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] faq: Start an OVN FAQ by giving a rationale for how it uses tunnels.

[Ben Pfaff]

On Thu, May 10, 2018 at 01:55:31PM -0400, Aaron Conole wrote:
> Ben Pfaff  writes:
> 
> > On Mon, Apr 16, 2018 at 12:16:24PM -0700, Ben Pfaff wrote:
> >> Signed-off-by: Ben Pfaff 
> >
> > This still needs a review.
> 
> Looks good, but I found one part slightly confusing.
> 
> >> +These compromises wouldn't suit every site, since some might want to
> >> +allocate more bits to the datapath, some to the egress port.
> 
> I'm having trouble with the clarity in the final clause of this
> paragraph.  Maybe reprase as:
> 
> +These compromises wouldn't suit every site, since some deployments
> +may need to allocate more bits to the datapath or egress port
> +identifiers.
> 
> Otherwise,
> Acked-by: Aaron Conole 

Thanks!  I applied your suggestion and then pushed this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] checkpatch: Fix filename matching.

[Ben Pfaff]

On Thu, May 10, 2018 at 11:12:39PM +0530, Numan Siddique wrote:
> On Wed, May 9, 2018 at 11:56 PM, Ben Pfaff  wrote:
> 
> > The .match() method only matches at the beginning of a string but the
> > blacklists here need to match anywhere in a string.
> >
> > Signed-off-by: Ben Pfaff 
> >
> 
> Acked-by: Numan Siddique 
> 
> Thanks Ben. With this patch I don't see the warnings which I observed
> earlier with the IPv6 patch.

Thanks Numan and Aaron, I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 7/7] ofproto-dpif-xlate: Improve tracing through groups.

[Ben Pfaff]

This makes it clear which buckets from a group are executed and why.

The update to nsh.at provides an example.

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto-dpif-trace.c |  2 ++
 ofproto/ofproto-dpif-trace.h |  1 +
 ofproto/ofproto-dpif-xlate.c | 48 ++--
 tests/nsh.at | 41 +++--
 4 files changed, 71 insertions(+), 21 deletions(-)

diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index 7211eac8e1a5..396a06ba0a6c 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -49,6 +49,7 @@ oftrace_node_type_is_terminal(enum oftrace_node_type type)
 case OFT_DETAIL:
 case OFT_WARN:
 case OFT_ERROR:
+case OFT_BUCKET:
 return true;
 
 case OFT_BRIDGE:
@@ -167,6 +168,7 @@ oftrace_node_print_details(struct ds *output,
 ds_put_char(output, '\n');
 break;
 case OFT_TABLE:
+case OFT_BUCKET:
 case OFT_THAW:
 case OFT_ACTION:
 ds_put_format(output, "%s\n", sub->text);
diff --git a/ofproto/ofproto-dpif-trace.h b/ofproto/ofproto-dpif-trace.h
index ea6cb3fc01ac..63dbb50bad57 100644
--- a/ofproto/ofproto-dpif-trace.h
+++ b/ofproto/ofproto-dpif-trace.h
@@ -39,6 +39,7 @@ enum oftrace_node_type {
 /* Nodes that may have children (nonterminal nodes). */
 OFT_BRIDGE, /* Packet travel through an OpenFlow switch. */
 OFT_TABLE,  /* Packet travel through a flow table. */
+OFT_BUCKET, /* Executing a bucket in an OpenFlow group. */
 OFT_THAW,   /* Thawing a frozen state. */
 
 /* Nodes that never have children (terminal nodes). */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 5c436b8a3440..a6657b387c46 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1829,6 +1829,30 @@ bucket_is_alive(const struct xlate_ctx *ctx,
&& group_is_alive(ctx, bucket->watch_group, depth + 1)));
 }
 
+static void
+xlate_report_bucket_not_live(const struct xlate_ctx *ctx,
+ const struct ofputil_bucket *bucket)
+{
+if (OVS_UNLIKELY(ctx->xin->trace)) {
+struct ds s = DS_EMPTY_INITIALIZER;
+if (bucket->watch_port != OFPP_ANY) {
+ds_put_cstr(&s, "port ");
+ofputil_format_port(bucket->watch_port, NULL, &s);
+}
+if (bucket->watch_group != OFPG_ANY) {
+if (s.length) {
+ds_put_cstr(&s, " and ");
+}
+ds_put_format(&s, "port %"PRIu32, bucket->watch_group);
+}
+
+xlate_report(ctx, OFT_DETAIL, "bucket %"PRIu32": not live due to %s",
+ bucket->bucket_id, ds_cstr(&s));
+
+ds_destroy(&s);
+}
+}
+
 static struct ofputil_bucket *
 group_first_live_bucket(const struct xlate_ctx *ctx,
 const struct group_dpif *group, int depth)
@@ -1838,6 +1862,7 @@ group_first_live_bucket(const struct xlate_ctx *ctx,
 if (bucket_is_alive(ctx, bucket, depth)) {
 return bucket;
 }
+xlate_report_bucket_not_live(ctx, bucket);
 }
 
 return NULL;
@@ -1860,6 +1885,10 @@ group_best_live_bucket(const struct xlate_ctx *ctx,
 best_bucket = bucket;
 best_score = score;
 }
+xlate_report(ctx, OFT_DETAIL, "bucket %"PRIu32": score %"PRIu32,
+ bucket->bucket_id, score);
+} else {
+xlate_report_bucket_not_live(ctx, bucket);
 }
 }
 
@@ -4221,6 +4250,14 @@ static void
 xlate_group_bucket(struct xlate_ctx *ctx, struct ofputil_bucket *bucket,
bool is_last_action)
 {
+struct ovs_list *old_trace = ctx->xin->trace;
+if (OVS_UNLIKELY(ctx->xin->trace)) {
+char *s = xasprintf("bucket %"PRIu32, bucket->bucket_id);
+ctx->xin->trace = &oftrace_report(ctx->xin->trace, OFT_BUCKET,
+  s)->subs;
+free(s);
+}
+
 uint64_t action_list_stub[1024 / 8];
 struct ofpbuf action_list = OFPBUF_STUB_INITIALIZER(action_list_stub);
 struct ofpbuf action_set = ofpbuf_const_initializer(bucket->ofpacts,
@@ -4282,6 +4319,7 @@ xlate_group_bucket(struct xlate_ctx *ctx, struct 
ofputil_bucket *bucket,
 ctx->error = XLATE_OK;
 }
 
+ctx->xin->trace = old_trace;
 }
 
 static struct ofputil_bucket *
@@ -4414,11 +4452,17 @@ xlate_group_action__(struct xlate_ctx *ctx, struct 
group_dpif *group,
 } else {
 OVS_NOT_REACHED();
 }
+
 if (bucket) {
+xlate_report(ctx, OFT_DETAIL, "using bucket %"PRIu32,
+ bucket->bucket_id);
 xlate_group_bucket(ctx, bucket, is_last_action);
 xlate_group_stats(ctx, group, bucket);
-} else if (ctx->xin->xcache) {
-ofproto_group_unref(&group->

[ovs-dev] [PATCH 6/7] ofp-print: Handle statistics more systematically.

[Ben Pfaff]

ofp_to_string__() is supposed to call ofp_print_stats() for all kinds of
statistics, but it was only doing so haphazardly.  This commit makes it
systematic and in the process adds it to at least one case where it was
missing (and fixes up a test case).

Signed-off-by: Ben Pfaff 
---
 include/openvswitch/ofp-msgs.h |  2 ++
 lib/ofp-msgs.c | 16 +++-
 lib/ofp-print.c| 24 
 tests/ofp-print.at |  2 +-
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
index 33cc3f571d16..a5ee2d53ea95 100644
--- a/include/openvswitch/ofp-msgs.h
+++ b/include/openvswitch/ofp-msgs.h
@@ -766,6 +766,8 @@ const char *ofptype_get_name(enum ofptype);
 void ofpmsg_update_length(struct ofpbuf *);
 const void *ofpmsg_body(const struct ofp_header *);
 bool ofpmsg_is_stat_request(const struct ofp_header *);
+bool ofpmsg_is_stat_reply(const struct ofp_header *);
+bool ofpmsg_is_stat(const struct ofp_header *);
 
 /* Multipart messages (aka "statistics").
  *
diff --git a/lib/ofp-msgs.c b/lib/ofp-msgs.c
index dd8894a524eb..6517210c2cdf 100644
--- a/lib/ofp-msgs.c
+++ b/lib/ofp-msgs.c
@@ -894,12 +894,26 @@ ofpmsg_body(const struct ofp_header *oh)
 return (const uint8_t *) oh + ofphdrs_len(&hdrs);
 }
 
-/* Return if it's a stat/multipart (OFPST) request message. */
+/* Return if 'oh' is a stat/multipart (OFPST) request message. */
 bool
 ofpmsg_is_stat_request(const struct ofp_header *oh)
 {
 return ofp_is_stat_request(oh->version, oh->type);
 }
+
+/* Return if 'oh' is a stat/multipart (OFPST) reply message. */
+bool
+ofpmsg_is_stat_reply(const struct ofp_header *oh)
+{
+return ofp_is_stat_reply(oh->version, oh->type);
+}
+
+/* Return if 'oh' is a stat/multipart (OFPST) request or reply message. */
+bool
+ofpmsg_is_stat(const struct ofp_header *oh)
+{
+return ofp_is_stat(oh->version, oh->type);
+}
 
 static ovs_be16 *ofpmp_flags__(const struct ofp_header *);
 
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 6e30312aef7c..68e382471dbd 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1834,25 +1834,26 @@ ofp_to_string__(const struct ofp_header *oh,
 const struct ofputil_table_map *table_map, enum ofpraw raw,
 struct ds *string, int verbosity)
 {
+if (ofpmsg_is_stat(oh)) {
+ofp_print_stats(string, oh);
+}
+
 const void *msg = oh;
 enum ofptype type = ofptype_from_ofpraw(raw);
 switch (type) {
 case OFPTYPE_GROUP_STATS_REQUEST:
-ofp_print_stats(string, oh);
 return ofputil_group_stats_request_format(string, oh);
 
 case OFPTYPE_GROUP_STATS_REPLY:
 return ofputil_group_stats_format(string, oh);
 
 case OFPTYPE_GROUP_DESC_STATS_REQUEST:
-ofp_print_stats(string, oh);
 return ofputil_group_desc_request_format(string, oh);
 
 case OFPTYPE_GROUP_DESC_STATS_REPLY:
 return ofputil_group_desc_format(string, oh, port_map, table_map);
 
 case OFPTYPE_GROUP_FEATURES_STATS_REQUEST:
-ofp_print_stats(string, oh);
 break;
 
 case OFPTYPE_GROUP_FEATURES_STATS_REPLY:
@@ -1944,73 +1945,56 @@ ofp_to_string__(const struct ofp_header *oh,
 
 case OFPTYPE_METER_STATS_REQUEST:
 case OFPTYPE_METER_CONFIG_STATS_REQUEST:
-ofp_print_stats(string, oh);
 return ofp_print_meter_stats_request(string, oh);
 
 case OFPTYPE_METER_STATS_REPLY:
-ofp_print_stats(string, oh);
 return ofp_print_meter_stats_reply(string, oh);
 
 case OFPTYPE_METER_CONFIG_STATS_REPLY:
-ofp_print_stats(string, oh);
 return ofp_print_meter_config_reply(string, oh);
 
 case OFPTYPE_METER_FEATURES_STATS_REPLY:
-ofp_print_stats(string, oh);
 return ofp_print_meter_features_reply(string, oh);
 
 case OFPTYPE_DESC_STATS_REQUEST:
 case OFPTYPE_METER_FEATURES_STATS_REQUEST:
-ofp_print_stats(string, oh);
 break;
 
 case OFPTYPE_FLOW_STATS_REQUEST:
 case OFPTYPE_AGGREGATE_STATS_REQUEST:
-ofp_print_stats(string, oh);
 return ofp_print_flow_stats_request(string, oh, port_map, table_map);
 
 case OFPTYPE_TABLE_STATS_REQUEST:
-ofp_print_stats(string, oh);
 break;
 
 case OFPTYPE_PORT_STATS_REQUEST:
-ofp_print_stats(string, oh);
 return ofp_print_ofpst_port_request(string, oh, port_map);
 
 case OFPTYPE_QUEUE_STATS_REQUEST:
-ofp_print_stats(string, oh);
 return ofp_print_ofpst_queue_request(string, oh, port_map);
 
 case OFPTYPE_DESC_STATS_REPLY:
-ofp_print_stats(string, oh);
 return ofp_print_ofpst_desc_reply(string, oh);
 
 case OFPTYPE_FLOW_STATS_REPLY:
-ofp_print_stats(string, oh);
 return ofp_print_flow_stats_reply(string, oh, port_map, table_map);
 
 case OFPTYPE_QUEUE_STATS_REPLY:
-ofp_print_stats(string, oh);
 return ofp_print_ofpst_queue

[ovs-dev] [PATCH 4/7] Add OpenFlow extensions for group support in OpenFlow 1.0.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 include/openflow/openflow-1.5.h  |   3 +-
 include/openvswitch/ofp-errors.h |  54 +
 include/openvswitch/ofp-msgs.h   |  35 ---
 lib/nx-match.c   |  11 +---
 lib/ofp-group.c  | 120 +
 lib/ofp-print.c  |   2 +-
 manpages.mk  |   1 +
 tests/ofp-print.at   | 126 ++-
 tests/ofproto.at |  93 +++--
 9 files changed, 304 insertions(+), 141 deletions(-)

diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 73b76d88398f..5e839df5ee48 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -77,12 +77,11 @@ struct ofp15_bucket {
64-bit aligned. */
 ovs_be16 action_array_len;  /* Length of all actions in bytes. */
 ovs_be32 bucket_id; /* Bucket Id used to identify bucket*/
-/* Followed by exactly len - 8 bytes of group bucket properties. */
 /* Followed by:
  *   - Exactly 'action_array_len' bytes containing an array of
  * struct ofp_action_*.
  *   - Zero or more bytes of group bucket properties to fill out the
- * overall length in header.length. */
+ * overall length in 'len'. */
 };
 OFP_ASSERT(sizeof(struct ofp15_bucket) == 8);
 
diff --git a/include/openvswitch/ofp-errors.h b/include/openvswitch/ofp-errors.h
index 16feacb559b4..6e8e55ab4f4f 100644
--- a/include/openvswitch/ofp-errors.h
+++ b/include/openvswitch/ofp-errors.h
@@ -254,7 +254,7 @@ enum ofperr {
 /* OF1.0+(2,8).  Problem validating output queue. */
 OFPERR_OFPBAC_BAD_QUEUE,
 
-/* OF1.1+(2,9).  Invalid group id in output action. */
+/* NX1.0(2,9), OF1.1+(2,9).  Invalid group id in output action. */
 OFPERR_OFPBAC_BAD_OUT_GROUP,
 
 /* NX1.0(1,522), OF1.1+(2,10).  Action can't apply for this match or a
@@ -460,63 +460,65 @@ enum ofperr {
 /* ## OFPET_GROUP_MOD_FAILED ## */
 /* ## -- ## */
 
-/* OF1.1+(6,0).  Group not added because a group ADD attempted to replace
- * an already-present group. */
+/* NX1.0(6,0), OF1.1+(6,0).  Group not added because a group ADD attempted
+ * to replace an already-present group. */
 OFPERR_OFPGMFC_GROUP_EXISTS,
 
-/* OF1.1+(6,1).  Group not added because Group specified is invalid. */
+/* NX1.0(6,1), OF1.1+(6,1).  Group not added because Group specified is
+ * invalid. */
 OFPERR_OFPGMFC_INVALID_GROUP,
 
-/* OF1.1+(6,2).  Switch does not support unequal load sharing with select
- * groups. */
+/* NX1.0(6,2), OF1.1+(6,2).  Switch does not support unequal load sharing
+ * with select groups. */
 OFPERR_OFPGMFC_WEIGHT_UNSUPPORTED,
 
-/* OF1.1+(6,3).  The group table is full. */
+/* NX1.0(6,3), OF1.1+(6,3).  The group table is full. */
 OFPERR_OFPGMFC_OUT_OF_GROUPS,
 
-/* OF1.1+(6,4).  The maximum number of action buckets for a group has been
- * exceeded. */
+/* NX1.0(6,4), OF1.1+(6,4).  The maximum number of action buckets for a
+ * group has been exceeded. */
 OFPERR_OFPGMFC_OUT_OF_BUCKETS,
 
-/* OF1.1+(6,5).  Switch does not support groups that forward to groups. */
+/* NX1.0(6,5), OF1.1+(6,5).  Switch does not support groups that forward to
+ * groups. */
 OFPERR_OFPGMFC_CHAINING_UNSUPPORTED,
 
-/* OF1.1+(6,6).  This group cannot watch the watch_port or watch_group
- * specified. */
+/* NX1.0(6,6), OF1.1+(6,6).  This group cannot watch the watch_port or
+ * watch_group specified. */
 OFPERR_OFPGMFC_WATCH_UNSUPPORTED,
 
-/* OF1.1+(6,7).  Group entry would cause a loop. */
+/* NX1.0(6,7), OF1.1+(6,7).  Group entry would cause a loop. */
 OFPERR_OFPGMFC_LOOP,
 
-/* OF1.1+(6,8).  Group not modified because a group MODIFY attempted to
- * modify a non-existent group. */
+/* NX1.0(6,8), OF1.1+(6,8).  Group not modified because a group MODIFY
+ * attempted to modify a non-existent group. */
 OFPERR_OFPGMFC_UNKNOWN_GROUP,
 
-/* OF1.2+(6,9).  Group not deleted because another
-group is forwarding to it. */
+/* NX1.0(6,9), OF1.2+(6,9).  Group not deleted because another group is
+ * forwarding to it. */
 OFPERR_OFPGMFC_CHAINED_GROUP,
 
-/* OF1.2+(6,10).  Unsupported or unknown group type. */
+/* NX1.0(6,10), OF1.2+(6,10).  Unsupported or unknown group type. */
 OFPERR_OFPGMFC_BAD_TYPE,
 
-/* OF1.2+(6,11).  Unsupported or unknown command. */
+/* NX1.0(6,11), OF1.2+(6,11).  Unsupported or unknown command. */
 OFPERR_OFPGMFC_BAD_COMMAND,
 
-/* OF1.2+(6,12).  Error in bucket. */
+/* NX1.0(6,12), OF1.2+(6,12).  Error in bucket. */
 OFPERR_OFPGMFC_BAD_BUCKET,
 
-/* OF1.2+(6,13).  Error in watch port/group. */
+/* NX1.0(6,13), OF1.2+(6,13).  Error in watch

[ovs-dev] [PATCH 5/7] ofp-group: Move formatting code for groups into ofp-group.

[Ben Pfaff]

This does a better job of putting related code together.

Signed-off-by: Ben Pfaff 
---
 include/openvswitch/ofp-group.h |  19 ++-
 lib/ofp-group.c | 306 ++
 lib/ofp-print.c | 317 +---
 3 files changed, 332 insertions(+), 310 deletions(-)

diff --git a/include/openvswitch/ofp-group.h b/include/openvswitch/ofp-group.h
index 8d893a53fcb2..10790eb020bd 100644
--- a/include/openvswitch/ofp-group.h
+++ b/include/openvswitch/ofp-group.h
@@ -108,6 +108,14 @@ struct ofpbuf *ofputil_encode_group_mod(enum ofp_version 
ofp_version,
 enum ofperr ofputil_decode_group_mod(const struct ofp_header *,
  struct ofputil_group_mod *);
 
+void ofputil_group_mod_format__(struct ds *, enum ofp_version,
+const struct ofputil_group_mod *,
+const struct ofputil_port_map *,
+const struct ofputil_table_map *);
+enum ofperr ofputil_group_mod_format(struct ds *, const struct ofp_header *,
+ const struct ofputil_port_map *,
+ const struct ofputil_table_map *);
+
 char *parse_ofp_group_mod_file(const char *file_name,
const struct ofputil_port_map *,
const struct ofputil_table_map *, int command,
@@ -140,9 +148,12 @@ enum ofperr ofputil_decode_group_stats_request(
 const struct ofp_header *request, uint32_t *group_id);
 void ofputil_append_group_stats(struct ovs_list *replies,
 const struct ofputil_group_stats *);
+enum ofperr ofputil_group_stats_request_format(struct ds *,
+   const struct ofp_header *);
 
 int ofputil_decode_group_stats_reply(struct ofpbuf *,
  struct ofputil_group_stats *);
+enum ofperr ofputil_group_stats_format(struct ds *, const struct ofp_header *);
 
 /* Group features reply, independent of protocol.
  *
@@ -172,13 +183,19 @@ void ofputil_uninit_group_desc(struct ofputil_group_desc 
*gd);
 uint32_t ofputil_decode_group_desc_request(const struct ofp_header *);
 struct ofpbuf *ofputil_encode_group_desc_request(enum ofp_version,
  uint32_t group_id);
+enum ofperr ofputil_group_desc_request_format(struct ds *,
+  const struct ofp_header *);
 
 int ofputil_decode_group_desc_reply(struct ofputil_group_desc *,
 struct ofpbuf *, enum ofp_version);
-
 void ofputil_append_group_desc_reply(const struct ofputil_group_desc *,
  const struct ovs_list *buckets,
  struct ovs_list *replies);
+enum ofperr ofputil_group_desc_format(struct ds *, const struct ofp_header *,
+  const struct ofputil_port_map *,
+  const struct ofputil_table_map *);
+enum ofperr ofputil_group_features_format(struct ds *,
+  const struct ofp_header *);
 
 #ifdef __cplusplus
 }
diff --git a/lib/ofp-group.c b/lib/ofp-group.c
index a2755ee47fea..f5b0af8cec0f 100644
--- a/lib/ofp-group.c
+++ b/lib/ofp-group.c
@@ -25,6 +25,7 @@
 #include "openvswitch/ofp-msgs.h"
 #include "openvswitch/ofp-parse.h"
 #include "openvswitch/ofp-port.h"
+#include "openvswitch/ofp-print.h"
 #include "openvswitch/ofp-prop.h"
 #include "openvswitch/ofpbuf.h"
 #include "openvswitch/vlog.h"
@@ -291,6 +292,18 @@ ofputil_encode_group_desc_request(enum ofp_version 
ofp_version,
 return request;
 }
 
+
+enum ofperr
+ofputil_group_desc_request_format(struct ds *string,
+   const struct ofp_header *oh)
+{
+uint32_t group_id = ofputil_decode_group_desc_request(oh);
+ds_put_cstr(string, " group_id=");
+ofputil_format_group(group_id, string);
+
+return 0;
+}
+
 static void
 ofputil_group_bucket_counters_to_ofp11(const struct ofputil_group_stats *gs,
 struct ofp11_bucket_counter bucket_cnts[])
@@ -371,6 +384,7 @@ ofputil_append_group_stats(struct ovs_list *replies,
 OVS_NOT_REACHED();
 }
 }
+
 /* Returns an OpenFlow group features request for OpenFlow version
  * 'ofp_version'. */
 struct ofpbuf *
@@ -420,6 +434,45 @@ ofputil_decode_group_features_reply(const struct 
ofp_header *oh,
 }
 }
 
+static const char *
+group_type_to_string(enum ofp11_group_type type)
+{
+switch (type) {
+case OFPGT11_ALL: return "all";
+case OFPGT11_SELECT: return "select";
+case OFPGT11_INDIRECT: return "indirect";
+case OFPGT11_FF: return "fast failover";
+default: OVS_NOT_REACHED();
+}
+}
+
+enum ofperr
+ofputil_group_features_format(struct ds *string, const struct ofp_header *oh)
+{
+struct ofputil_

[ovs-dev] [PATCH 3/7] ofp-group: Require watch_port or watch_group when parsing ff groups.

[Ben Pfaff]

Fast failover buckets must have a watch_port or a watch_group (or both),
and ovs-vswitchd enforces this, but the bucket parsing code didn't check
it.  This meant that when it was omitted, the error messages were harder
to understand.

Signed-off-by: Ben Pfaff 
---
 lib/ofp-group.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/lib/ofp-group.c b/lib/ofp-group.c
index 31b04379448c..5b54faee963f 100644
--- a/lib/ofp-group.c
+++ b/lib/ofp-group.c
@@ -611,6 +611,10 @@ parse_bucket_str(struct ofputil_bucket *bucket, char *str_,
 if (!actions.length) {
 return xstrdup("bucket must specify actions");
 }
+if (group_type == OFPGT11_FF && !ofputil_bucket_has_liveness(bucket)) {
+return xstrdup("fast failover bucket requires watch_port or "
+   "watch_group");
+}
 ds_chomp(&actions, ',');
 
 ofpbuf_init(&ofpacts, 0);
-- 
2.16.1

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

[ovs-dev] [PATCH 2/7] ofproto-dpif-xlate: Simplify translation for groups.

[Ben Pfaff]

Translation of groups had a lot of redundant code.  This commit eliminates
most of it.  It should also make it harder to accidentally reintroduce
the reference leak fixed in a previous commit.

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto-dpif-xlate.c | 131 +++
 1 file changed, 46 insertions(+), 85 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 85fa8b3f7855..5c436b8a3440 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4281,57 +4281,26 @@ xlate_group_bucket(struct xlate_ctx *ctx, struct 
ofputil_bucket *bucket,
 /* reset the error and continue processing other buckets */
 ctx->error = XLATE_OK;
 }
-}
 
-static void
-xlate_all_group(struct xlate_ctx *ctx, struct group_dpif *group,
-bool is_last_action)
-{
-struct ofputil_bucket *bucket;
-LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {
-bool last = is_last_action && !bucket->list_node.next;
-xlate_group_bucket(ctx, bucket, last);
-}
-xlate_group_stats(ctx, group, NULL);
 }
 
-static void
-xlate_ff_group(struct xlate_ctx *ctx, struct group_dpif *group,
-   bool is_last_action)
+static struct ofputil_bucket *
+pick_ff_group(struct xlate_ctx *ctx, struct group_dpif *group)
 {
-struct ofputil_bucket *bucket;
-
-bucket = group_first_live_bucket(ctx, group, 0);
-if (bucket) {
-xlate_group_bucket(ctx, bucket, is_last_action);
-xlate_group_stats(ctx, group, bucket);
-} else if (ctx->xin->xcache) {
-ofproto_group_unref(&group->up);
-}
+return group_first_live_bucket(ctx, group, 0);
 }
 
-static void
-xlate_default_select_group(struct xlate_ctx *ctx, struct group_dpif *group,
-   bool is_last_action)
+static struct ofputil_bucket *
+pick_default_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
 {
-struct flow_wildcards *wc = ctx->wc;
-struct ofputil_bucket *bucket;
-uint32_t basis;
-
-basis = flow_hash_symmetric_l4(&ctx->xin->flow, 0);
-flow_mask_hash_fields(&ctx->xin->flow, wc, NX_HASH_FIELDS_SYMMETRIC_L4);
-bucket = group_best_live_bucket(ctx, group, basis);
-if (bucket) {
-xlate_group_bucket(ctx, bucket, is_last_action);
-xlate_group_stats(ctx, group, bucket);
-} else if (ctx->xin->xcache) {
-ofproto_group_unref(&group->up);
-}
+flow_mask_hash_fields(&ctx->xin->flow, ctx->wc,
+  NX_HASH_FIELDS_SYMMETRIC_L4);
+return group_best_live_bucket(ctx, group,
+  flow_hash_symmetric_l4(&ctx->xin->flow, 0));
 }
 
-static void
-xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group,
-   bool is_last_action)
+static struct ofputil_bucket *
+pick_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
 {
 const struct field_array *fields = &group->up.props.fields;
 const uint8_t *mask_values = fields->values;
@@ -4367,21 +4336,12 @@ xlate_hash_fields_select_group(struct xlate_ctx *ctx, 
struct group_dpif *group,
 mf_mask_field_masked(mf, &mask, ctx->wc);
 }
 
-struct ofputil_bucket *bucket = group_best_live_bucket(ctx, group, basis);
-if (bucket) {
-xlate_group_bucket(ctx, bucket, is_last_action);
-xlate_group_stats(ctx, group, bucket);
-} else if (ctx->xin->xcache) {
-ofproto_group_unref(&group->up);
-}
+return group_best_live_bucket(ctx, group, basis);
 }
 
-static void
-xlate_dp_hash_select_group(struct xlate_ctx *ctx, struct group_dpif *group,
-   bool is_last_action)
+static struct ofputil_bucket *
+pick_dp_hash_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
 {
-struct ofputil_bucket *bucket;
-
 /* dp_hash value 0 is special since it means that the dp_hash has not been
  * computed, as all computed dp_hash values are non-zero.  Therefore
  * compare to zero can be used to decide if the dp_hash value is valid
@@ -4390,9 +4350,7 @@ xlate_dp_hash_select_group(struct xlate_ctx *ctx, struct 
group_dpif *group,
 uint64_t param = group->up.props.selection_method_param;
 
 ctx_trigger_recirculate_with_hash(ctx, param >> 32, (uint32_t)param);
-if (ctx->xin->xcache) {
-ofproto_group_unref(&group->up);
-}
+return NULL;
 } else {
 uint32_t n_buckets = group->up.n_buckets;
 if (n_buckets) {
@@ -4403,23 +4361,15 @@ xlate_dp_hash_select_group(struct xlate_ctx *ctx, 
struct group_dpif *group,
 uint32_t basis = 0xc2b73583 * (ctx->xin->flow.dp_hash & mask);
 
 ctx->wc->masks.dp_hash |= mask;
-bucket = group_best_live_bucket(ctx, group, basis);
-if (bucket) {
-xlate_group_bucket(ctx, bucket, is_last_action);
-xlate_group_stats(ctx, group, bucket);
-

[ovs-dev] [PATCH 1/7] ofproto-dpif-xlate: Fix reference leak in xlate_dp_hash_select_group().

[Ben Pfaff]

xlate_group_action() takes a reference to the ofgroup and passes it
down to xlate_group_action__(), xlate_select_group(), and finally to
xlate_dp_hash_select_group(), which is supposed to consume it but fails
to do so.  This commit fixes the problem.

Found by inspection.

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto-dpif-xlate.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 794893a78c67..85fa8b3f7855 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4390,6 +4390,9 @@ xlate_dp_hash_select_group(struct xlate_ctx *ctx, struct 
group_dpif *group,
 uint64_t param = group->up.props.selection_method_param;
 
 ctx_trigger_recirculate_with_hash(ctx, param >> 32, (uint32_t)param);
+if (ctx->xin->xcache) {
+ofproto_group_unref(&group->up);
+}
 } else {
 uint32_t n_buckets = group->up.n_buckets;
 if (n_buckets) {
@@ -4404,6 +4407,8 @@ xlate_dp_hash_select_group(struct xlate_ctx *ctx, struct 
group_dpif *group,
 if (bucket) {
 xlate_group_bucket(ctx, bucket, is_last_action);
 xlate_group_stats(ctx, group, bucket);
+} else if (ctx->xin->xcache) {
+ofproto_group_unref(&group->up);
 }
 }
 }
-- 
2.16.1

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

Re: [ovs-dev] [PATCH v2] ovn: Avoid nb_cfg update notification flooding

[Han Zhou]

On Thu, Apr 12, 2018 at 10:19 AM, Ben Pfaff  wrote:
>
> On Wed, Apr 11, 2018 at 11:29:28PM -0700, Han Zhou wrote:
> > On Tue, Apr 10, 2018 at 6:21 PM, Han Zhou  wrote:
> > >
> > >
> > >
> > > On Tue, Apr 10, 2018 at 5:04 PM, Ben Pfaff  wrote:
> > > >
> > > > On Fri, Apr 06, 2018 at 02:40:21PM -0700, Han Zhou wrote:
> > > > > On Fri, Apr 6, 2018 at 1:54 PM, Ben Pfaff  wrote:
> > > > > >
> > > > > > Thanks for working on making OVN faster and scale better.
> > > > > >
> > > > > > I see what you mean about how nb_cfg can be a scale problem.
> > Really,
> > > > > > each hypervisor only cares about its own row, but each of them
is
> > being
> > > > > > notified about the current value in every row.  Ideally, we want
> > the HVs
> > > > > > to be able to say to ovsdb-server, "Give me all the data in the
> > Chassis
> > > > > > table, except don't bother with nb_cfg in any rows except my own
> > row."
> > > > > >
> > > > > > Actually, there's a way for ovn-controller to do that: the OVSDB
> > > > > > protocol definition of monitor_cond allows the client to specify
> > > > > > multiple sets of columns to monitor in a table, and each set of
> > columns
> > > > > > has an independently attached condition.  This can be used to
> > specify
> > > > > > that most of the columns are monitored unconditionally but
nb_cfg is
> > > > > > monitored only for a particular row.
> > > > > >
> > > > > > But there are still problems:
> > > > > >
> > > > > > 1. The implementation in ovsdb-server looks broken.  I don't
think
> > it
> > > > > >implements the spec.  It certainly isn't tested--none of the
> > existing
> > > > > >clients ever passes more than a single set of columns.  I
think
> > that
> > > > > >it will work if all the sets of columns use the same
condition,
> > but
> > > > > >that doesn't help with this case.  It will need to be fixed.
> > > > > >
> > > > > > 2. The client implementation in ovsdb-idl doesn't anticipate
> > difference
> > > > > >conditions for different columns.  It will need to be
enhanced to
> > > > > >support this use.
> > > > > >
> > > > > > In the short term this is going to be more work than just
creating
> > a new
> > > > > > table.  In the long term, though, it will allow us to be more
> > flexible
> > > > > > and more agile, since improvements similar to the one in this
patch
> > will
> > > > > > require only client changes, not database schema changes.
> > > > > >
> > > > > > Comments?
> > > > >
> > > > > Thanks for the valuable information! I didn't even know that
ovsdb is
> > > > > supposed to support multiple set columns with different monitor
> > conditions.
> > > > > It surely would be the right way to go if it is supported.
However,
> > as you
> > > > > mentioned the mechanism doesn't work yet and I am not sure how
much
> > work is
> > > > > needed to make it work, it seems reasonable to me to "workaround"
it
> > at
> > > > > least for short term until that is ready in the future. Then we
could
> > > > > simply resurrect the old column in the old table with the new
monitor
> > > > > conditions. What do you think?
> > > >
> > > > This kind of workaround would essentially persist forever.  We'd
never
> > > > be able to get rid of it--or, more to the point, we would never be
able
> > > > to justify the work.  We'd be stuck with something ugly.
> > > >
> > > > So: if you think the workaround is valuable, then one way to
justify it
> > > > would be to reformulate it in a way that isn't so ugly.  As
currently
> > > > written, when a person looks at the table structure, (s)he notices a
> > > > table that has a single column and is named for that single column.
> > > > That sticks out, clearly, as some kind of compromise, and the
> > > > documentation even calls it out as an optimization.  But there
might be
> > > > a logical, conceptual reason why it makes sense to have a separate
table
> > > > for some kinds of chassis-related data.  If there is, then that
would
> > > > suggest a name for the table, and for the class of data that should
> > > > should go in there, and it would make it possible to have something
that
> > > > is both faster and a reasonable design.
> > > >
> > > > I'm not saying that there is necessarily a logical, conceptual
reason
> > > > that one can come up here, but if there is then it would certainly
make
> > > > it much easier to support the patch.
> > >
> > > Hi Ben, I'm not sure if I got your point completely. A logical reason
for
> > this separate table might be for holding chassis private data, the data
> > that other chassis is not interested in. Of course the reason for
holding
> > such private data in a separate table is still this huge performance
> > difference as it is shown in the test result. So I think the table can
be
> > named as Chassis_Private_Data, and I can document in ovn-sb.xml more
about
> > the performance considerations, and the future improvement about the
> > monitor_cond. Would this address your point or did I totally miss

Re: [ovs-dev] [PATCH] Fix datapath compilation on RHEL >= 7.5

[Daniel Alvarez]

Yes, let’s go with that one then.
Thanks a lot!
Daniel 

> On 10 May 2018, at 23:15, Gregory Rose  wrote:
> 
>> On 5/10/2018 7:11 AM, Daniel Alvarez wrote:
>> On RHEL 7.5 we get compilation errors due to field ndo_change_mtu
>> missing. This patch checks the RHEL version and redefines it to
>> ndo_change_mtu_rh74.
>> 
>> Reported-by: Lucas Alvares 
>> Signed-off-by: Daniel Alvarez 
>> ---
>>  datapath/datapath.h | 5 +
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/datapath/datapath.h b/datapath/datapath.h
>> index 93c9ed505..d418bf381 100644
>> --- a/datapath/datapath.h
>> +++ b/datapath/datapath.h
>> @@ -25,6 +25,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  @@ -36,6 +37,10 @@
>>  #define DP_MAX_PORTS   USHRT_MAX
>>  #define DP_VPORT_HASH_BUCKETS  1024
>>  +#if RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7,5)
>> +#define ndo_change_mtu ndo_change_mtu_rh74
>> +#endif
>> +
>>  /**
>>   * struct dp_stats_percpu - per-cpu packet processing statistics for a given
>>   * datapath.
> 
> There are dueling patches for this.  I think the one posted by Yi-hung is a 
> bit more complete because it doesn't just check version numbers.
> 
> Thanks,
> 
> - Greg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Fix datapath compilation on RHEL >= 7.5

[Gregory Rose]

On 5/10/2018 7:11 AM, Daniel Alvarez wrote:

On RHEL 7.5 we get compilation errors due to field ndo_change_mtu
missing. This patch checks the RHEL version and redefines it to
ndo_change_mtu_rh74.

Reported-by: Lucas Alvares 
Signed-off-by: Daniel Alvarez 
---
  datapath/datapath.h | 5 +
  1 file changed, 5 insertions(+)

diff --git a/datapath/datapath.h b/datapath/datapath.h
index 93c9ed505..d418bf381 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -25,6 +25,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  
@@ -36,6 +37,10 @@

  #define DP_MAX_PORTS   USHRT_MAX
  #define DP_VPORT_HASH_BUCKETS  1024
  
+#if RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7,5)

+#define ndo_change_mtu ndo_change_mtu_rh74
+#endif
+
  /**
   * struct dp_stats_percpu - per-cpu packet processing statistics for a given
   * datapath.


There are dueling patches for this.  I think the one posted by Yi-hung 
is a bit more complete because it doesn't just check version numbers.


Thanks,

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

Re: [ovs-dev] [PATCH] datapath: compat: Fix build on RHEL 7.5

[Gregory Rose]

On 5/9/2018 6:21 PM, Yi-Hung Wei wrote:

1) OVS datapath compat modules breaks on RHEL 7.5, because it moves
ndo_change_mtu function pointer from 'struct net_device_ops' to
'struct net_device_ops_extended'.

2) RHEL 7.5 introduces the MTU range checking as mentioned in
6c0bf091 ("datapath: use core MTU range checking in core net infra").
However, the max_mtu field is defined in 'struct net_device_extended'
but not in 'struct net_device' as upstream kernel.

This patch defines a new symbol HAVE_RHEL7_MAX_MTU that determines
the previous 2 conditions, and fixes the backport issue.

Signed-off-by: Yi-Hung Wei 
---
  acinclude.m4   | 2 ++
  datapath/linux/compat/geneve.c | 4 
  datapath/linux/compat/ip_gre.c | 4 
  datapath/linux/compat/lisp.c   | 4 
  datapath/linux/compat/stt.c| 4 
  datapath/linux/compat/vxlan.c  | 8 
  datapath/vport-internal_dev.c  | 2 +-
  7 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 60186d33de88..a2444af33c22 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -576,6 +576,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
  
OVS_FIND_FIELD_IFELSE([$KSRC/include/linux/netdevice.h], [net_device],

  [max_mtu])
+  OVS_FIND_FIELD_IFELSE([$KSRC/include/linux/netdevice.h], 
[net_device_ops_extended],
+[ndo_change_mtu], [OVS_DEFINE([HAVE_RHEL7_MAX_MTU])])
  
OVS_GREP_IFELSE([$KSRC/include/linux/netfilter.h], [nf_hook_state])

OVS_GREP_IFELSE([$KSRC/include/linux/netfilter.h], [nf_register_net_hook])
diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c
index c08dced3feaf..eacffb2c5b2a 100644
--- a/datapath/linux/compat/geneve.c
+++ b/datapath/linux/compat/geneve.c
@@ -1271,7 +1271,11 @@ static const struct net_device_ops geneve_netdev_ops = {
.ndo_stop   = geneve_stop,
.ndo_start_xmit = geneve_dev_xmit,
.ndo_get_stats64= ip_tunnel_get_stats64,
+#ifdef HAVE_RHEL7_MAX_MTU
+   .extended.ndo_change_mtu = geneve_change_mtu,
+#else
.ndo_change_mtu = geneve_change_mtu,
+#endif
.ndo_validate_addr  = eth_validate_addr,
.ndo_set_mac_address= eth_mac_addr,
  #ifdef HAVE_NDO_FILL_METADATA_DST
diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c
index 4e325914d8be..2f297a548e30 100644
--- a/datapath/linux/compat/ip_gre.c
+++ b/datapath/linux/compat/ip_gre.c
@@ -491,7 +491,11 @@ static const struct net_device_ops gre_tap_netdev_ops = {
.ndo_start_xmit = gre_dev_xmit,
.ndo_set_mac_address= eth_mac_addr,
.ndo_validate_addr  = eth_validate_addr,
+#ifdef HAVE_RHEL7_MAX_MTU
+   .extended.ndo_change_mtu = ip_tunnel_change_mtu,
+#else
.ndo_change_mtu = ip_tunnel_change_mtu,
+#endif
  #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
.ndo_get_stats64= ip_tunnel_get_stats64,
  #endif
diff --git a/datapath/linux/compat/lisp.c b/datapath/linux/compat/lisp.c
index 34f82324a7ff..4882a636a33e 100644
--- a/datapath/linux/compat/lisp.c
+++ b/datapath/linux/compat/lisp.c
@@ -546,7 +546,11 @@ static const struct net_device_ops lisp_netdev_ops = {
.ndo_open   = lisp_open,
.ndo_stop   = lisp_stop,
.ndo_start_xmit = lisp_dev_xmit,
+#ifdef  HAVE_RHEL7_MAX_MTU
+   .extended.ndo_change_mtu = lisp_change_mtu,
+#else
.ndo_change_mtu = lisp_change_mtu,
+#endif
.ndo_validate_addr  = eth_validate_addr,
.ndo_set_mac_address= eth_mac_addr,
  #ifdef USE_UPSTREAM_TUNNEL
diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c
index 2189476e176e..ee1c7aa0a78d 100644
--- a/datapath/linux/compat/stt.c
+++ b/datapath/linux/compat/stt.c
@@ -1857,7 +1857,11 @@ static const struct net_device_ops stt_netdev_ops = {
.ndo_stop   = stt_stop,
.ndo_start_xmit = stt_dev_xmit,
.ndo_get_stats64= ip_tunnel_get_stats64,
+#ifdef  HAVE_RHEL7_MAX_MTU
+   .extended.ndo_change_mtu = stt_change_mtu,
+#else
.ndo_change_mtu = stt_change_mtu,
+#endif
.ndo_validate_addr  = eth_validate_addr,
.ndo_set_mac_address= eth_mac_addr,
  #ifdef USE_UPSTREAM_TUNNEL
diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c
index 54b953454bf4..40cb12b1329d 100644
--- a/datapath/linux/compat/vxlan.c
+++ b/datapath/linux/compat/vxlan.c
@@ -1481,7 +1481,11 @@ static const struct net_device_ops 
vxlan_netdev_ether_ops = {
.ndo_start_xmit = vxlan_dev_xmit,
.ndo_get_stats64= ip_tunnel_get_stats64,
.ndo_set_rx_mode= vxlan_set_multicast_list,
+#ifdef HAVE_RHEL7_MAX_MTU
+   .extended.ndo_change_mtu = vxlan_change_mtu,
+#else
.ndo_change_mtu = vxlan_change_mtu,
+#endif
.ndo_validate_addr  = eth_validate_addr,
.ndo_set_mac_ad

Re: [ovs-dev] [PATCH] compat: Fix upstream 4.4.119 kernel

[Gregory Rose]

On 5/8/2018 7:44 PM, Pravin Shelar wrote:

On Fri, Apr 20, 2018 at 11:13 AM, Greg Rose  wrote:

The Linux 4.4.119 kernel (and perhaps others) from kernel.org
backports some dst_cache code that breaks the openvswitch kernel
due to a duplicated name "dst_cache_destroy".  For most cases the
"USE_UPSTREAM_TUNNEL" covers this but in this case the dst_cache
feature needs to be separated out.

Add the necessary compatibility detection layer in acinclude.m4 and
then fixup the source files so that if the built-in kernel includes
dst_cache support then exclude our own compatibility code.

Signed-off-by: Greg Rose 

I pushed it to master and upto branch 2.6.

Thanks.


Thank you Pravin!

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

Re: [ovs-dev] [PATCH 1/1] netdev-dpdk: Fixed netdev_dpdk structure alignment

[Stokes, Ian]

> Thanks for this, Eelco.
> 
> (Sorry for the double email, missed the ML on the first one.)
> 
> On 25/04/2018 16:48, Eelco Chaudron wrote:
> > Currently, the code tells us we have 4 pad bytes left in cacheline0
> > while actually we are 8 bytes short:
> >
> 
> This was caused by commit 5e925cc ("netdev-dpdk: DPDK v17.11 upgrade"),
> where the dpdk_port_t typedef was changed from uint8_t to uint16_t,
> messing up the alignment.
> 

Thanks all, I merged this to DPDK_MERGE and backported for OVS 2.9.

Thanks
Ian

> > struct netdev_dpdk {
> > union {
> > OVS_CACHE_LINE_MARKER cacheline0;/*   1 */
> > struct {
> > dpdk_port_t port_id; /* 0 2 */
> > _Bool  attached; /* 2 1 */
> > struct eth_addr hwaddr;  /* 4 6 */
> > intmtu;  /*12 4 */
> > intsocket_id;/*16 4 */
> > intbuf_size; /*20 4 */
> > intmax_packet_len;   /*24 4 */
> > enum dpdk_dev_type type; /*28 4 */
> > enum netdev_flags flags; /*32 4 */
> > char * devargs;  /*40 8 */
> > struct dpdk_tx_queue * tx_q; /*48 8 */
> > struct rte_eth_link link;/*56 8 */
> > intlink_reset_cnt;   /*64 4 */
> > };   /*  72 */
> > uint8_tpad9[128];/* 128 */
> > };   /* 0   128 */
> > /* --- cacheline 2 boundary (128 bytes) --- */
> >
> > Re-located one member, link_reset_cnt, and now it's one cache line:
> >
> > struct netdev_dpdk {
> > union {
> > OVS_CACHE_LINE_MARKER cacheline0;/*   1 */
> > struct {
> > dpdk_port_t port_id; /* 0 2 */
> > _Bool  attached; /* 2 1 */
> > struct eth_addr hwaddr;  /* 4 6 */
> > intmtu;  /*12 4 */
> > intsocket_id;/*16 4 */
> > intbuf_size; /*20 4 */
> > intmax_packet_len;   /*24 4 */
> > enum dpdk_dev_type type; /*28 4 */
> > enum netdev_flags flags; /*32 4 */
> > intlink_reset_cnt;   /*36 4 */
> > char * devargs;  /*40 8 */
> > struct dpdk_tx_queue * tx_q; /*48 8 */
> > struct rte_eth_link link;/*56 8 */
> > };   /*  64 */
> > uint8_tpad9[64]; /*  64 */
> > };   /* 064 */
> > /* --- cacheline 1 boundary (64 bytes) --- */
> >
> > Signed-off-by: Eelco Chaudron 
> 
> Acked-by: Tiago Lam 
> 
> Tiago
> ___
> 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] Avoid crash in OvS while transmitting fragmented packets over tunnel.

[Darrell Ball]

Thanks Rohith
I see this patch was applied, but I have one question inline.


On 4/20/18, 1:48 AM, "ovs-dev-boun...@openvswitch.org on behalf of Rohith 
Basavaraja"  wrote:

Currently when fragmented packets are to be transmitted in to tunnel,
base_flow->nw_frag which was initially non-zero at reception is not
reset to zero when the base_flow and flow are rewritten
as part of the emulated tnl_push action in the ofproto-dpif-xlate
module.

Because of this when fragmented packets are transmitted out of tunnel,
we hit crash caused by the following assert.

lib/odp-util.c:5654: assertion flow->nw_proto == base_flow->nw_proto &&
flow->nw_frag == base_flow->nw_frag failed in commit_set_ipv4_action()

Can you describe how you hit this assertion?
I have some testing in and around this code, but have not hit this yet, so I 
was curious?

With the following change propagate_tunnel_data_to_flow__
is modified to reset *nw_frag* to zero. 


Also, that currently we don't
fragment tunnelled packets, we should reset *nw_frag* to zero in
propagate_tunnel_data_to_flow__.

Signed-off-by: Jan Scheurich 
From: Rohith Basavaraja 
CC: Jan Scheurich 

---
 ofproto/ofproto-dpif-xlate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 94e3ddb..e9ed037 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3310,6 +3310,7 @@ propagate_tunnel_data_to_flow__(struct flow *dst_flow,
 dst_flow->ipv6_dst = src_flow->tunnel.ipv6_dst;
 dst_flow->ipv6_src = src_flow->tunnel.ipv6_src;
 
+dst_flow->nw_frag = 0; /* Tunnel packets are unfragmented. */


 dst_flow->nw_tos = src_flow->tunnel.ip_tos;
 dst_flow->nw_ttl = src_flow->tunnel.ip_ttl;
 dst_flow->tp_dst = src_flow->tunnel.tp_dst;
-- 
1.9.1




___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=6ZMZ7J9yNmX0ZQRMQyUfQ8fZrhemcMFiUqpnVD_jN9w&s=lYu98hfGnEvKr7YcK50fxDDY9-d0mA3W0yYtpSeeIQo&e=




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

Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action

[Ben Pfaff]

On Thu, May 10, 2018 at 08:51:03AM +, Anju Thomas wrote:
> It looks like your commit message describes at least two other bugs in
> OVS, though.  First, if OVS crashes when it pushes tunnel headers,
> even if there's not enough headroom, that's really bad.  At worst, it
> should drop the packet.  Do you know where the crash occurs?  We
> should fix the problem, since it might recur in some other context.
> 
> [Anju] OVS was actually crashing in the dpdk datapath. The crash is a manual 
> assert in our case.
> The rootcause is that dp receives the actions after the upcall (say with >=3 
> tunnel pushes ) . Now as part of action processing , since it is a tunnel 
> push action , we try to find space in the dpdk mbuf packet headroom (which Is 
> 128 bytes). By the time we try to process the third tunnel push , there is no 
> headroom left since each tunnel header is of 50 bytes (50 *3 > 128 bytes 
> headroom). Hence it manually asserts .  This assert is to catch any 
> unexpected code flow . Do you think that in this case we should still go 
> ahead and  prevent the crash ? 

I don't understand why it's OK to crash in this case.  Why do you think
so?

> Second, it's a little shocking to hear that an encapsulation action without a 
> following output action causes a memory leak.  We also need to fix that.  Do 
> you have any more details?
> [Anju] Now as explained above, the crash happens because we run out of 
> headroom. But in case we have say 2 or less than 2 tunnel pushes we will have 
> a mem leak as packet is never freed because the tnl push is the dp last 
> action and there is no other output action or any other action like recirc 
> that may translate to output action in the end leading  to packet buffer not 
> being freed.
> Are you proposing that we have some sort of preventive fix in the dp to 
> handle an incorrect action list from the upcall handling? 

Yes.  It's unacceptable to leak memory because there's a packet
modification without an output action.  The kernel datapath never does
this, for example.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v9 0/7] OVS-DPDK flow offload with rte_flow

[Stokes, Ian]

> -Original Message-
> From: Shahaf Shuler [mailto:shah...@mellanox.com]
> Sent: Thursday, May 10, 2018 1:41 PM
> To: Stokes, Ian ; f...@napatech.com
> Cc: ovs-dev@openvswitch.org; simon.hor...@netronome.com; Flavio Leitner
> 
> Subject: RE: [PATCH v9 0/7] OVS-DPDK flow offload with rte_flow
> 
> Hi,
> 
> Friday, April 20, 2018 11:07 AM, Stokes, Ian:
> > Subject: RE: [PATCH v9 0/7] OVS-DPDK flow offload with rte_flow
> >
> > > > Hi,
> > > >
> > > > Here is a joint work from Mellanox and Napatech, to enable the
> > > > flow hw offload with the DPDK generic flow interface (rte_flow).
> > >
> > > Hi folks, I feel Mellanox/Netronome have reached the point where
> > > HWOL can be introduced to OVS DPDK pending performance review.
> >
> > Apologies , Mellanox an Napatech.
> 
> Any progress with this series?
> 

Hi Shahaf,

I'm finishing the pull request for this week now. This series is my priority in 
the next pull request.

The only outstanding item I need to do is a performance check before and after 
to ensure there is no negative impact on the non HWOL usecase. Other than that 
it should be ok.

Thanks
Ian 
> 
> >
> > Ian
> > >
> > > This has not been an easy path, but I will put one final request out
> > > there for comments.
> > >
> > > If there are none then pending on Travis compilation and final
> > > performance checks I'm thinking of introducing this to the code base.
> > >
> > > Keep in mind this feature is experimental, not only can it be
> > > changed but it can also be removed pending future review if not
> maintained.
> > >
> > > I invite all to give opinions here as I think this is an important
> > > feature.
> > >
> > > Sincere thanks to all involved.
> > >
> > > Regards
> > > Ian
> > >
> > >
> > > >
> > > > The basic idea is to associate the flow with a mark id (a unit32_t
> > > > number).
> > > > Later, we then get the flow directly from the mark id, which could
> > > > bypass some heavy CPU operations, including but not limiting to
> > > > mini flow extract, emc lookup, dpcls lookup, etc.
> > > >
> > > > The association is done with CMAP in patch 1. The CPU workload
> > > > bypassing is done in patch 2. The flow offload is done in patch 3,
> > > > which mainly does two things:
> > > >
> > > > - translate the ovs match to DPDK rte flow patterns
> > > > - bind those patterns with a RSS + MARK action.
> > > >
> > > > Patch 5 makes the offload work happen in another thread, for
> > > > leaving the datapath as light as possible.
> > > >
> > > > A PHY-PHY forwarding with 1000 mega flows (udp,tp_src=1000-1999)
> > > > and
> > > > 1 million streams (tp_src=1000-1999, tp_dst=2000-2999) show more
> > > > than 260% performance boost.
> > > >
> > > > Note that it's disabled by default, which can be enabled by:
> > > >
> > > > $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
> > > >
> > > > v9: - introduced IP packet sanity checks in a seperate commit
> > > > - moved structs and enums definition to the begining of the file
> > > > - fixed sparse compilation error (error is assumed to be
> > > > fixed,
> > > issues
> > > >   on Travis CI preventing from fully testing it.
> > > >
> > > > v8: - enhanced documentation with more details on supported
> protocols
> > > > - fixed VLOG to start with capital letter
> > > > - fixed compilation issues
> > > > - fixed coding style
> > > > - addressed the rest of Ian's comments
> > > >
> > > > v7: - fixed wrong hash for mark_to_flow that has been refactored in
> v6
> > > > - set the rss_conf for rss action to NULL, to workaround a
> > > > mlx5
> > > change
> > > >   in DPDK v17.11. Note that it will obey the rss settings
> > > > OVS-DPDK
> > > has
> > > >   set in the beginning. Thus, nothing should be effected.
> > > >
> > > > v6: - fixed a sparse warning
> > > > - added documentation
> > > > - used hash_int to compute mark to flow hash
> > > > - added more comments
> > > > - added lock for pot lookup
> > > > - rebased on top of the latest code
> > > >
> > > > v5: - fixed an issue that it took too long if we do flow add/remove
> > > >   repeatedly.
> > > > - removed an unused mutex lock
> > > > - turned most of the log level to DBG
> > > > - rebased on top of the latest code
> > > >
> > > > v4: - use RSS action instead of QUEUE action with MARK
> > > > - make it work with multiple queue (see patch 1)
> > > > - rebased on top of latest code
> > > >
> > > > v3: - The mark and id association is done with array instead of
> CMAP.
> > > > - Added a thread to do hw offload operations
> > > > - Removed macros completely
> > > > - dropped the patch to set FDIR_CONF, which is a workround some
> > > >   Intel NICs.
> > > > - Added a debug patch to show all flow patterns we have created.
> > > > - Misc fixes
> > > >
> > > > v2: - workaround the queue action issue
> > > > - fixed the tcp_flags being skipped issue, which also fixed the
> > > > 

Re: [ovs-dev] [PATCH] doc: Final cleanup of the DPDK documents

[Stokes, Ian]

> This concludes the cleanup by fixing some nits and adding some additional
> cross-references.
> 
> Signed-off-by: Stephen Finucane 
> ---
> This was initially sent as part of a larger series but excluded from a
> recent merge due to label issues. I wasn't able to reproduce these so I'm
> resending verbatim.

Hi Stephen,

Thanks for this. Bit of a strange one. I'm still seeing the complaint regarding 
the duplication of labels on my own system

Warning, treated as error:
ovs/Documentation/topics/dpdk/index.rst:None: WARNING: duplicate label bridges 
and ports, other instance in ovs/Documentation/howto/dpdk.rst

This could be specific to my board as you couldn't re-produce, however running 
the patch through travis build throws up a separate error on compilation 
regarding the use of caption in toctree.

Warning, treated as error:
Documentation/topics/dpdk/index.rst:1: ERROR: Error in "toctree" directive:
unknown option: "caption".
.. toctree::
   :maxdepth: 2
   :caption: Bridges and Ports
   /topics/dpdk/bridge
   /topics/dpdk/phy
   /topics/dpdk/vhost-user
   /topics/dpdk/ring
   /topics/dpdk/vdev

For completeness the link to the travis build is included below.

https://travis-ci.org/istokes/ovs/jobs/377412427

I wondering is this related to different sphinx versions across the test 
systems.

I'll investigate the error that appears on my board if you can look at the 
travis error above.

Thanks
Ian
> ---
>  Documentation/howto/dpdk.rst| 51 
> -
>  Documentation/topics/dpdk/index.rst |  6 +
>  2 files changed, 28 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index 380181db0..6344f1cb0 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -25,23 +25,26 @@
>  Using Open vSwitch with DPDK
>  
> 
> -This document describes how to use Open vSwitch with DPDK datapath.
> +This document describes how to use Open vSwitch with DPDK datapath. For
> +more detailed information, refer to the various :doc:`DPDK topic guides
> +`.
> 
>  .. important::
> 
> Using the DPDK datapath requires building OVS with DPDK support. Refer
> to
> :doc:`/intro/install/dpdk` for more information.
> 
> -Ports and Bridges
> --
> +Overview
> +
> 
> -ovs-vsctl can be used to set up bridges and other Open vSwitch features.
> -Bridges should be created with a ``datapath_type=netdev``::
> +:program:`ovs-vsctl` can be used to set up bridges and other Open
> +vSwitch features.  Bridges should be created with a
> ``datapath_type=netdev``::
> 
>  $ ovs-vsctl add-br br0 -- set bridge br0 datapath_type=netdev
> 
> -ovs-vsctl can also be used to add DPDK devices. ovs-vswitchd should print
> the -number of dpdk devices found in the log file::
> +:program:`ovs-vsctl` can also be used to add DPDK devices.
> +:program:`ovs-vswitchd` should print the number of ``dpdk`` devices
> +found in the log file::
> 
>  $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
>  options:dpdk-devargs=:01:00.0 @@ -59,14 +62,13 @@ is
> suggested::
> 
>  .. important::
> 
> -Hotplugging physical interfaces is not supported using the above
> syntax.
> -This is expected to change with the release of DPDK v18.05. For
> information
> -on hotplugging physical interfaces, you should instead refer to
> -:ref:`port-hotplug`.
> +Hotplugging physical interfaces is not supported for these devices.
> This
> +is expected to change with the release of DPDK v18.05. For
> information on
> +hotplugging physical interfaces, refer to :ref:`port-hotplug`.
> 
>  After the DPDK ports get added to switch, a polling thread continuously
> polls -DPDK devices and consumes 100% of the core, as can be checked from
> ``top`` and -``ps`` commands::
> +DPDK devices and consumes 100% of the core, as can be checked from
> +:command:`top` and :command:`ps` commands::
> 
>  $ top -H
>  $ ps -eLo pid,psr,comm | grep pmd
> @@ -79,7 +81,7 @@ set. For example::
>  -- set Interface p0 type=dpdk options:dpdk-devargs=:01:00.0 \
>  -- set Interface p1 type=dpdk options:dpdk-devargs=:01:00.1
> 
> -To stop ovs-vswitchd & delete bridge, run::
> +To stop :program:`ovs-vswitchd` and delete the bridge, run::
> 
>  $ ovs-appctl -t ovs-vswitchd exit
>  $ ovs-appctl -t ovsdb-server exit
> @@ -137,13 +139,16 @@ Add test flows to forward packets between DPDK port
> 0 and port 1::
> 
>  Transmit traffic into either port. You should see it returned via the
> other.
> 
> +More information on the ``dpdk`` ports can be found in
> :doc:`/topics/dpdk/phy`.
> +
>  .. _dpdk-vhost-loopback:
> 
>  PHY-VM-PHY (vHost Loopback)
>  ---
> 
>  Add a userspace bridge, two ``dpdk`` (PHY) ports, and two
> ``dpdkvhostuser``
> -ports::
> +ports. It is assumed that the physical ports are already bound to DPDK,
> +as de

Re: [ovs-dev] [PATCH] rhel: openvswitch-fedora.spec.in: Specify PYTHON and PYTHON3

[Ansis Atteka]

On Thu, 10 May 2018 at 08:21, Timothy Redaelli  wrote:

> Currently python2 and python3 binaries are searched by following the
> PATHs, but, on Fedora, the python2 package does not provides /bin/python2
> and so if the PATH contains /bin before /usr/bin (for example by using
> the ansible poc) then the resulting RPM file will require /bin/python2
> instead of /usr/bin/python2 and this breaks some tools (for example
> createrepo).

> This patch specify the full path of python2 interpreter and,
> if python3-openvswitch package is built, the full path of python3
> interpreter.

> Reported-by: Ansis Atteka 
> Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-May/346796.html
> Signed-off-by: Timothy Redaelli 
Thanks for the fix. Will push to master branch.

Acked-by: Ansis Atteka 

> ---
>   rhel/openvswitch-fedora.spec.in | 8 +++-
>   1 file changed, 7 insertions(+), 1 deletion(-)

> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/
openvswitch-fedora.spec.in
> index 6dbf20ce1..9462ce236 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -222,7 +222,13 @@ Docker network plugins for OVN.
>  --with-dpdk=$(dirname %{_datadir}/dpdk/*/.config) \
>   %endif
>  --enable-ssl \
> -   --with-pkidir=%{_sharedstatedir}/openvswitch/pki
> +   --with-pkidir=%{_sharedstatedir}/openvswitch/pki \
> +%if 0%{?fedora} > 22 || %{with build_python3}
> +   PYTHON3=%{__python3} \
> +   PYTHON=%{__python2}
> +%else
> +   PYTHON=%{__python}
> +%endif

>   build-aux/dpdkstrip.py \
>   %if %{with dpdk}
> --
> 2.17.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] checkpatch: Fix filename matching.

[Aaron Conole]

Ben Pfaff  writes:

> The .match() method only matches at the beginning of a string but the
> blacklists here need to match anywhere in a string.
>
> Signed-off-by: Ben Pfaff 
> ---

d'oh!  At least the debian/rules part should have matched :)

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

Re: [ovs-dev] [PATCH] faq: Start an OVN FAQ by giving a rationale for how it uses tunnels.

[Aaron Conole]

Ben Pfaff  writes:

> On Mon, Apr 16, 2018 at 12:16:24PM -0700, Ben Pfaff wrote:
>> Signed-off-by: Ben Pfaff 
>
> This still needs a review.

Looks good, but I found one part slightly confusing.

>> +These compromises wouldn't suit every site, since some might want to
>> +allocate more bits to the datapath, some to the egress port.

I'm having trouble with the clarity in the final clause of this
paragraph.  Maybe reprase as:

+These compromises wouldn't suit every site, since some deployments
+may need to allocate more bits to the datapath or egress port
+identifiers.

Otherwise,
Acked-by: Aaron Conole 

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

[ovs-dev] [PATCH v3] ovn: Set proper Neighbour Adv flag when replying for NS request for router IP

[nusiddiq]

From: Numan Siddique 

Presently when a VM's IPv6 stack sends a Neighbor Solicitation request for its
router IP, (mostly when the ND cache entry for the router is in STALE state)
ovn-controller responds with a Neighbor Adv packet (using the action nd_na).
But it doesn't set 'ND_RSO_ROUTER' in the RSO flags (please see RFC4861 page 
23).
Because of which, the VM deletes the default route. The default route gets added
again when the next RA is received (but would again gets deleted if its sends
NS request). And this results in disruption of IPv6 traffic.

This patch addresses this issue by adding a new action 'nd_na_router' which is
same as 'nd_na' but it sets the 'ND_RSO_ROUTER' in the RSO flags. ovn-northd
uses this action. A new action is added instead of modifying the existing 
'nd_na'
action. This is because
  - We cannot set the RSO flags in the "nd_na { ..actions .. }"
  - It would be ugly to have something like nd_na { router_flags, ...actions .. 
}

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1567735
Signed-off-by: Numan Siddique 
Acked-by: Mark Michelson 
---

v2 -> v3
---
Updated the commit subject

v1 -> v2
---
Updated the commit message with the RFC info.

 include/ovn/actions.h |  7 +++
 ovn/controller/pinctrl.c  | 23 +++
 ovn/lib/actions.c | 22 ++
 ovn/northd/ovn-northd.c   |  2 +-
 ovn/utilities/ovn-trace.c |  1 +
 tests/ovn.at  |  5 +
 6 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index fb8f51509..638465193 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -68,6 +68,7 @@ struct ovn_extend_table;
 OVNACT(ICMP6, ovnact_nest)\
 OVNACT(TCP_RESET, ovnact_nest)\
 OVNACT(ND_NA, ovnact_nest)\
+OVNACT(ND_NA_ROUTER,  ovnact_nest)\
 OVNACT(GET_ARP,   ovnact_get_mac_bind)\
 OVNACT(PUT_ARP,   ovnact_put_mac_bind)\
 OVNACT(GET_ND,ovnact_get_mac_bind)\
@@ -444,6 +445,12 @@ enum action_opcode {
  * The actions, in OpenFlow 1.3 format, follow the action_header.
  */
 ACTION_OPCODE_TCP_RESET,
+
+/* "nd_na_router { ...actions... }" with rso flag 'ND_RSO_ROUTER' set.
+*
+* The actions, in OpenFlow 1.3 format, follow the action_header.
+*/
+ACTION_OPCODE_ND_NA_ROUTER,
 };
 
 /* Header. */
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 6e6aa1caa..305f20649 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -81,7 +81,8 @@ static void send_garp_run(struct controller_ctx *ctx,
   struct sset *active_tunnels);
 static void pinctrl_handle_nd_na(const struct flow *ip_flow,
  const struct match *md,
- struct ofpbuf *userdata);
+ struct ofpbuf *userdata,
+ bool is_router);
 static void reload_metadata(struct ofpbuf *ofpacts,
 const struct match *md);
 static void pinctrl_handle_put_nd_ra_opts(
@@ -1154,7 +1155,11 @@ process_packet_in(const struct ofp_header *msg, struct 
controller_ctx *ctx)
 break;
 
 case ACTION_OPCODE_ND_NA:
-pinctrl_handle_nd_na(&headers, &pin.flow_metadata, &userdata);
+pinctrl_handle_nd_na(&headers, &pin.flow_metadata, &userdata, false);
+break;
+
+case ACTION_OPCODE_ND_NA_ROUTER:
+pinctrl_handle_nd_na(&headers, &pin.flow_metadata, &userdata, true);
 break;
 
 case ACTION_OPCODE_PUT_ND:
@@ -2308,7 +2313,7 @@ reload_metadata(struct ofpbuf *ofpacts, const struct 
match *md)
 
 static void
 pinctrl_handle_nd_na(const struct flow *ip_flow, const struct match *md,
- struct ofpbuf *userdata)
+ struct ofpbuf *userdata, bool is_router)
 {
 /* This action only works for IPv6 ND packets, and the switch should only
  * send us ND packets this way, but check here just to be sure. */
@@ -2322,13 +2327,15 @@ pinctrl_handle_nd_na(const struct flow *ip_flow, const 
struct match *md,
 struct dp_packet packet;
 dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
 
-/* xxx These flags are not exactly correct.  Look at section 7.2.4
- * xxx of RFC 4861.  For example, we need to set ND_RSO_ROUTER for
- * xxx router's interfaces and ND_RSO_SOLICITED only if it was
- * xxx requested. */
+/* These flags are not exactly correct.  Look at section 7.2.4
+ * of RFC 4861. */
+uint32_t rso_flags = ND_RSO_SOLICITED | ND_RSO_OVERRIDE;
+if (is_router) {
+rso_flags |= ND_RSO_ROUTER;
+}
 compose_nd_na(&packet, ip_flow->dl_dst, ip_flow->dl_src,
   &ip_flow->nd_target, &ip_flow->ipv6_src,
-  htonl(ND_RSO_SOLICITED | ND_RSO_OVERRIDE));
+  

Re: [ovs-dev] [PATCH] tests: Fix typo in test name.

[Alin Gabriel Serdean]

Acked-by: Alin Gabriel Serdean mailto:aserd...@ovn.org>>

> On 10 May 2018, at 20:26, Ben Pfaff  wrote:
> 
> On Fri, Apr 13, 2018 at 10:38:53AM -0700, Ben Pfaff wrote:
>> Signed-off-by: Ben Pfaff 
> 
> This still needs a review.
> ___
> 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] checkpatch: Fix filename matching.

[Numan Siddique]

On Wed, May 9, 2018 at 11:56 PM, Ben Pfaff  wrote:

> The .match() method only matches at the beginning of a string but the
> blacklists here need to match anywhere in a string.
>
> Signed-off-by: Ben Pfaff 
>

Acked-by: Numan Siddique 

Thanks Ben. With this patch I don't see the warnings which I observed
earlier with the IPv6 patch.



> ---
>  utilities/checkpatch.py | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index 1438e5aa4401..61c123829610 100755
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -458,14 +458,12 @@ file_checks = [
>
>  checks = [
>  {'regex': None,
> - 'match_name':
> - lambda x: not line_length_blacklist.match(x),
> + 'match_name': lambda x: not line_length_blacklist.search(x),
>   'check': lambda x: line_length_check(x),
>   'print': lambda: print_warning("Line length is >79-characters
> long")},
>
>  {'regex': None,
> - 'match_name':
> - lambda x: not leading_whitespace_blacklist.match(x),
> + 'match_name': lambda x: not leading_whitespace_blacklist.search(x),
>   'check': lambda x: not leading_whitespace_is_spaces(x),
>   'print': lambda: print_warning("Line has non-spaces leading
> whitespace")},
>
> --
> 2.16.1
>
> ___
> 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 v2] ovn: Add a new action 'nd_na_router' to handle NS requests for router IPs

[Ben Pfaff]

On Tue, May 08, 2018 at 03:20:25PM +0530, nusid...@redhat.com wrote:
> (Please note: There are 3 'Line length is >79-characters' warnings in ovn.at 
> which
> I couldn't resolve.)

That's actually a bug in checkpatch, see my fix here:
https://patchwork.ozlabs.org/patch/911103/
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] tests: Fix typo in test name.

[Ben Pfaff]

On Fri, Apr 13, 2018 at 10:38:53AM -0700, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff 

This still needs a review.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] faq: Start an OVN FAQ by giving a rationale for how it uses tunnels.

[Ben Pfaff]

On Mon, Apr 16, 2018 at 12:16:24PM -0700, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff 

This still needs a review.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 0/5] selinux: introduce a transition domain for loading kmods

[Aaron Conole]

Ansis Atteka  writes:

> On Fri, 4 May 2018 at 11:28, Aaron Conole  wrote:
>
>> On linux systems, the initial start of openvswitch attempts to load
>> the openvswitch.ko kernel module.  This module allows openvswitch to
>> utilize the kernel datapath.
>
>> Some of these linux systems, notably Fedora and RHEL, use selinux to
>> enforce additional restrictions on various processes by way of allowing
>> or disallowing access from a specific selinux domain to a particular
>> operation on an selinux type.  On these systems, the openvswitch
>> initialization will be run from the 'openvswitch_t' selinux domain.
>> Attempts by a process in the 'openvswitch_t' selinux domain to load a
>> kernel module will be denied.
>
>> One solution would be to simply allow 'openvswitch_t' to load a kernel
>> directly.  This essentially means that 'openvswitch_t' would really be
>> 'unconfined_t' - since an attacker that can control the code can issue
>> a kernel load.
>
>> The solution implemented here uses a labeled file in the openvswitch
>> scripts directory, which is writable only by root.  That file will force
>> a domain transition to the 'openvswitch_load_module_t' domain.  The
>> 'openvswitch_load_module_t' domain will then be granted permissions to
>> load a kernel module.
>
>> The labelling won't take place until after the changes implemented in 4/4,
>> so it is really important to test the automatic labelling after that
> point.
>
>> v1->v2:
>> * Added a new commit to set the selinux-policy module version
>> * Added changes to the centos build in 4/4 to match the fedora build
>> * Fixed the manpage in 1/5
>
>
> Thanks. I wanted to test this on Fedora 27 too (with poc/builders), but I am
> getting this error:
>
> [root@fedoraubuilder x86_64]# yum install openvswitch-2.9.90-1.x86_64.rpm
> Last metadata expiration check: 1:18:01 ago on Wed 09 May 2018 09:56:08 PM
> UTC.
> Error:
>Problem: conflicting requests
> - nothing provides /bin/python2 needed by openvswitch-2.9.90-1.x86_64
> [root@fedoraubuilder x86_64]# /bin/python2 --version
> Python 2.7.14
> [root@fedoraubuilder x86_64]# rpm -q --whatprovides /bin/python
> python2-2.7.14-10.fc27.x86_64
> [root@fedoraubuilder x86_64]# rpm -qR  openvswitch-2.9.90-1.x86_64.rpm  |
> grep -i python
> /bin/python2
>
> based on openvswitch-fedora.spec.in file my understanding is that it should
> have picked python3 right? Though, I also have python2 so that error seems
> strange to me.

Fyi - see:

https://mail.openvswitch.org/pipermail/ovs-dev/2018-May/346815.html

for an explanation on the reason.  I guess the POC ansible script
reorders the path and that causes the system to pick up something
incorrectly.  That's at least Timothy's explanation.

Try with that patch applied and see if it helps :)

>> Aaron Conole (5):
>> ovs-kmod-ctl: introduce a kernel module load script
>> selinux: create a transition type for module loading
>> selinux: tag the custom policy version
>> selinux: introduce domain transitioned kmod helper
>> rhel: selinux-policy to invoke proper label macros
>
>>debian/openvswitch-switch.install  |   1 +
>>debian/openvswitch-switch.manpages |   1 +
>>rhel/openvswitch-fedora.spec.in|  12 +-
>>rhel/openvswitch.spec.in   |  12 +-
>>selinux/.gitignore |   4 +
>>selinux/automake.mk|   3 +-
>>selinux/openvswitch-custom.fc.in   |   1 +
>>selinux/openvswitch-custom.te.in   |  81 -
>>utilities/.gitignore   |   1 +
>>utilities/automake.mk  |   5 +
>>utilities/ovs-ctl.in   |  32 +-
>>utilities/ovs-kmod-ctl.8   | 109 ++
>>utilities/ovs-kmod-ctl.in  | 228
> +
>>utilities/ovs-lib.in   |  12 +-
>>14 files changed, 454 insertions(+), 48 deletions(-)
>>create mode 100644 selinux/openvswitch-custom.fc.in
>>create mode 100644 utilities/ovs-kmod-ctl.8
>>create mode 100644 utilities/ovs-kmod-ctl.in
>
>> --
>> 2.14.3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v6] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support

[SatyaValli]

From: SatyaValli 

This Patch provides implementation Existing flow entry statistics are
redefined as standard OXS(OpenFlow Extensible Statistics) fields for
displaying the arbitrary flow stats.

To support this implementation below messages are newly added

OFPRAW_OFPT15_FLOW_REMOVED,
OFPRAW_OFPST15_AGGREGATE_REQUEST,
OFPRAW_OFPST15_FLOW_REPLY,
OFPRAW_OFPST15_AGGREGATE_REPLY,

The current commit adds support for the new feature in flow statistics
multipart messages,aggregate multipart messages and OXS support for flow
removal message, individual flow description messages.

"ovs-ofctl dump-flows" & "ovs-ofctl dump-aggregate" now accepts a new option
"--oxs-stats" provided with the arbitrary OXS fields for displaying the
desired flow stats.

Below are Commands to display OXS stats field wise

Flow Statistics Multipart
ovs-ofctl dump-flows -O OpenFlow15  --oxs-stats=idle_time
ovs-ofctl dump-flows -O OpenFlow15  --oxs-stats=packet_count
ovs-ofctl dump-flows -O OpenFlow15  --oxs-stats=byte_count
ovs-ofctl dump-flows -O OpenFlow15  --oxs-stats=duration

Aggregate Flow Statistics Multipart
ovs-ofctl dump-aggregate -O OpenFlow15  --oxs-stats=packet_count
ovs-ofctl dump-aggregate -O OpenFlow15  --oxs-stats=byte_count
ovs-ofctl dump-aggregate -O OpenFlow15  --oxs-stats=flow_count

Signed-off-by: Satya Valli 
Co-authored-by: Lavanya Harivelam 
Signed-off-by: Lavanya Harivelam 
Co-authored-by: Surya Muttamsetty 
Signed-off-by: Surya Muttamsetty 
Co-authored-by: Manasa Cherukupally 
Signed-off-by: Manasa Cherukupally 
Co-authored-by: Pavani Panthagada 
Signed-off-by: Pavani Panthagada 

---
 NEWS|   6 +
 include/openflow/openflow-1.5.h |  59 +++
 include/openvswitch/ofp-msgs.h  |  20 +-
 include/openvswitch/ofp-print.h |   7 +
 lib/automake.mk |   2 +
 lib/ofp-flow.c  | 141 +-
 lib/ofp-monitor.c   |  50 +-
 lib/ofp-print.c |  53 +++
 lib/ox-stat.c   | 982 
 lib/ox-stat.h   |  54 +++
 manpages.mk |   1 +
 tests/ofp-print.at  |  80 
 tests/ofproto-dpif.at   |  85 
 tests/ofproto.at|   1 +
 utilities/ovs-ofctl.8.in|  31 +-
 utilities/ovs-ofctl.c   | 127 +-
 16 files changed, 1660 insertions(+), 39 deletions(-)
 create mode 100644 lib/ox-stat.c
 create mode 100644 lib/ox-stat.h

diff --git a/NEWS b/NEWS
index f9aa9ed..4a5cd15 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,12 @@ Post-v2.9.0
  * Add minimum network namespace support for Linux.
  * New command "lacp/show-stats"
- ovs-ofctl:
+ * Existing flow entry statistics are redefined as standard OXS(OpenFlow
+   Extensible Statistics) fields for displaying the arbitrary flow stats.
+ * Now "ovs-ofctl dump-flows" and "ovs-ofctl dump-aggregate" accepts a new
+   option --oxs-stats provided with the arbitrary OXS fields i.e
+   flow duration, flow count, packet count, byte count for displaying
+   the desired flow stats. See ovs-ofctl(8) for details.
  * ovs-ofctl now accepts and display table names in place of numbers.  By
default it always accepts names and in interactive use it displays them;
use --names or --no-names to override.  See ovs-ofctl(8) for details.
diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 73b76d8..6a5e369 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -163,4 +163,63 @@ struct ofp15_packet_out {
 };
 OFP_ASSERT(sizeof(struct ofp15_packet_out) == 8);
 
+struct ofp_oxs_stat {
+ovs_be16 reserved;  /* Reserved for future use,
+ * currently zeroed. */
+ovs_be16 length;/* Stats Length */
+};
+
+OFP_ASSERT(sizeof(struct ofp_oxs_stat) == 4);
+
+/* Body of reply to OFPMP_FLOW_DESC request. */
+struct ofp15_flow_desc {
+ovs_be16 length;  /* Length of this entry. */
+uint8_t pad2[2];  /* Align to 64 bits. */
+uint8_t table_id; /* ID of table flow came from. */
+uint8_t pad;
+ovs_be16 priority;/* Priority of the entry. */
+ovs_be16 idle_timeout;/* Number of seconds
+ idle before expiration. */
+ovs_be16 hard_timeout;/* Number of seconds
+ before expiration. */
+ovs_be16 flags;   /* Bitmap of OFPFF_*. flags. */
+ovs_be16 importance;  /* Eviction precedence. */
+ovs_be64 cookie;  /* Opaque controller issued identifier. */
+};
+
+OFP_ASSERT(sizeof(struct ofp15_flow_desc) == 24);
+
+/* Body of reply to OFPMP_FLOW_STATS request
+ * and body for OFPIT_STAT_TRIGGER generated status. */
+struct ofp15_flow_stats_reply {
+ovs_be16 length;/* Length of this entry.  */
+uint8_t pad2[2];/* Align to 64 bits.  */
+uint8_t table_id;   /* ID of table f

[ovs-dev] Estrategia de Fijación de Precios

[Evite Errores en la Fijación de Precios]

 MAYO 23 - webinar Interactivo
Pricing: Estrategia de Fijación de Precios 

Introducción:

El Pricing es el proceso mediante el cual una empresa establece el precio al 
que venderá sus productos y servicios, y puede ser parte del plan de marketing 
de la empresa. 

Temas a tratar:

Factores psicológicos que influyen en la determinación del precio. 
Errores en la fijación de precios y consecuencias. 
Precios basados en márgenes. 
Comportamiento de la competencia.
Incidencia de la variación de precios en la rentabilidad. 
Objetivos de negocio. 
 
 
Temario e Inscripciones:

Respondiendo por este medio "Precio"+Teléfono + Empresa+ Nombre o marcando al:

045 + 5515546630 


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

Re: [ovs-dev] [PATCH] ovn: Add a new action 'nd_na_router' to handle NS requests for router IPs

[Daniel Alvarez Sanchez]

On Tue, May 8, 2018 at 3:26 PM, Mark Michelson  wrote:

> On 05/08/2018 05:36 AM, Numan Siddique wrote:
>
>>
>>
>> On Tue, May 8, 2018 at 1:20 PM, Miguel Angel Ajo Pelayo <
>> majop...@redhat.com > wrote:
>>
>> Thank you Numan!
>>
>> It took me a bit to find what the "RSO"  flag was, because they are
>> R, S and O, may be we can change that in the commit, or reference
>> the RFC/section  (RFC4861 page 23).
>>
>>
>> Ack. I will update the commit message and send v2.
>>
>>
>>   R  Router flag.  When set, the R-bit indicates that
>>   the sender is a router.  The R-bit is used by
>>   Neighbor Unreachability Detection to detect a
>>   router that changes to a host.
>>
>>S  Solicited flag.  When set, the S-bit indicates
>> that
>>   the advertisement was sent in response to a
>>   Neighbor Solicitation from the Destination
>> address.
>>   The S-bit is used as a reachability confirmation
>>   for Neighbor Unreachability Detection.  It MUST
>> NOT
>>   be set in multicast advertisements or in
>>   unsolicited unicast advertisements.
>>
>>O  Override flag.  When set, the O-bit indicates
>> that
>>   the advertisement should override an existing
>> cache
>>   entry and update the cached link-layer address.
>>   When it is not set the advertisement will not
>>   update a cached link-layer address though it
>> will
>>   update an existing Neighbor Cache entry for
>> which
>>   no link-layer address is known.  It SHOULD NOT
>> be
>>   set in solicited advertisements for anycast
>>   addresses and in solicited proxy advertisements.
>>   It SHOULD be set in other solicited
>> advertisements
>>   and in unsolicited advertisements.
>>
>>
>>
>>
>> And same question Mark did.
>>
>> Thanks for handling this, good work :)
>>
>> On Mon, May 7, 2018 at 9:16 PM Mark Michelson > > wrote:
>>
>> Hi Numan, thanks for the fix.
>>
>> Out of curiosity, why did you add a new action instead of adding
>> a new
>> logical field (something like nd.rso)?
>>
>>
>> The logical flow which uses nd_na looks like
>>
>>table=11(ls_in_arp_rsp  ), priority=50   , match=(nd_ns && ip6.dst
>> == {2002::f816:3eff:fefe:9e2e, ff02::1:fffe:9e2e} && nd.target ==
>> 2002::f816:3eff:fefe:9e2e), action=(nd_na { eth.src = fa:16:3e:fe:9e:2e;
>> ip6.src = 2002::f816:3eff:fefe:9e2e; nd.target = 2002::f816:3eff:fefe:9e2e;
>> nd.tll = fa:16:3e:fe:9e:2e; outport = inport; flags.loopback = 1; output;
>> };)
>>
>>
>> I suppose you are suggesting to have something like - " nd_na { ..,
>> nd.rso = router ..} ". All the inner logical fields are defined in expr
>> symtab table [1]. But we cannot add nd.rso there as there is no
>> corresponding ovs field to modify the ICMPv6 flags of a packet.
>>
>>  From the email discussion here [2] I suppose Zak is working to add this
>> feature. Once we have this support, we can make use of that in OVN. Right
>> now IPv6 feature is blocked in Openstack + OVN  because of this issue so we
>> need this fix.
>>
>
> Yes, this is what I had been thinking. It sounds like if the timing had
> been different, you could have waited for Zak's patch first. But given the
> circumstances, I suppose this will work.
>
+1 to this and then follow up :)

>
>
>> [1] - https://github.com/openvswitch/ovs/blob/master/ovn/lib/
>> logical-fields.c#L205 > h/ovs/blob/master/ovn/lib/logical-fields.c#L205>
>> [2] - https://mail.openvswitch.org/pipermail/ovs-discuss/2018-May/
>> 046662.html
>> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-April/046528.html
>>
>>
>> Thanks
>> Numan
>>
>>
>>
>> On 05/07/2018 02:36 PM, nusid...@redhat.com
>>  wrote:
>>  > From: Numan Siddique > >
>>  >
>>  > Presently when a VM's IPv6 stack sends a Neighbor
>> Solicitation request for its
>>  > router IP, (mostly when the ND cache entry for the router is
>> in STALE state)
>>  > ovn-controller responds with a Neighbor Adv packet (using the
>> action nd_na).
>>  > But it doesn't set 'ND_RSO_ROUTER' in the RSO flags. Because
>> of which, the
>>  > VM deletes the default route. The default route gets added
>> again when the next
>>  > RA is received (but would again gets deleted if its sends NS
>> request). And this
>> 

Re: [ovs-dev] ovsdb-tool: error while loading shared libraries: librte_eal.so.6.1

[Ben Pfaff]

I don't know how to configure mininet.

If CPqD solves your problems, then of course it's a reasonable choice.

On Thu, May 10, 2018 at 08:15:21AM +0800, Mingming Chen wrote:
> Hi Ben,
> I have solved that problem(set up all in the same terminal, and 
> http://dpdk.org/ml/archives/dev/2016-June/042113.html). Now I did 
> $ovs-vswitchd --version
> 
> ovs-vswitchd (Open vSwitch) 2.9.0
> DPDK 17.11.1
> 
> 
> and I also did
>  $ sudo ovs-vsctl list open-vswitch
> _uuid   : 048bdd81-a839-4232-b34d-93d85c3c5453
> bridges : [488018e3-f7dd-4646-ba1f-88277804ccf6, 
> 88ecba3d-dab9-4437-8800-77e35af4cbb7, a62f9392-b5fa-44f2-a841-3af6c932a359]
> cur_cfg : 6
> datapath_types  : [netdev, system]
> db_version  : "7.12.1"
> external_ids: {hostname=ubuntu, 
> rundir="/usr/local/var/run/openvswitch", 
> system-id="562cba16-381c-45d0-b39b-9add642a1a86"}
> iface_types : [geneve, gre, internal, lisp, patch, stt, system, tap, 
> vxlan]
> manager_options : []
> next_cfg: 18
> other_config: {dpdk-init="true", dpdk-socket-mem="1024"}
> ovs_version : "2.9.0"
> ssl : []
> statistics  : {}
> system_type : Ubuntu
> system_version  : "16.04-xenial"
> 
> 
> I just want a simple ovs which could support meter, the recent problem is 
> that when I start mininet, it gets stuck like below:
> *** Creating network
> *** Adding controller
> *** Adding hosts:
> h1 h2 h3 
> *** Adding switches:
> s1 s2 s3 
> *** Adding links:
> (h1, s1) (h2, s2) (h3, s3) (s2, s1) (s3, s2) 
> *** Configuring hosts
> h1 h2 h3 
> *** Starting controller
> c0 
> *** Starting 3 switches
> s1 s2 s3 ...
> 
> (no mininuet console)
> 
> 
> And I checked the controller, the connection isn't successful.  It will be 
> really complex, right? Should I switch to CPqD? Actually I have tried, but 
> some of the components source was removed online. Could you give me a 
> suggestion? Thank you!
> 
> 
> Best regards,
> Mingming
> At 2018-05-10 04:58:59, "Ben Pfaff"  wrote:
> >No.
> >
> >On Thu, May 10, 2018 at 04:53:59AM +0800, Mingming Chen wrote:
> >> BTW, I really didn’t find 
> >> /usr/local/etc/openvswitch/conf.db. 
> >> Is this the problem? Thanks a lot!
> >> 
> >> 
> >> Best regards,
> >> Mingming
> >> 
> >> 
> >> | |
> >> Mingming Chen
> >> 邮箱：mingmingmi...@126.com
> >> |
> >> 
> >> 签名由 网易邮箱大师 定制
> >> 
> >> On 05/09/2018 13:17, Mingming Chen wrote:
> >> Hi Ben,
> >> I have done below:
> >> 
> >> 
> >> $echo "/usr/src/dpdk-stable-17.11.1/x86_64-native-linuxapp-gcc/lib" >> 
> >> /etc/ld.so.conf
> >> $cat /etc/ld.so.conf
> >> include /etc/ld.so.conf.d/*.conf
> >> 
> >> 
> >> /usr/local/share
> >> /usr/src/dpdk-stable-17.11.1/x86_64-native-linuxapp-gcc/lib
> >> 
> >> 
> >> Then, do make, make install, ovs-ctl start, it is still doesn't work. 
> >> What's the problem?
> >> Thank you!
> >> 
> >> 
> >> Best regards,
> >> Mingming
> >> 
> >> 
> >> 
> >> 
> >> 
> >> At 2018-05-10 03:37:03, "Ben Pfaff"  wrote:
> >> >You didn't install DPDK in a place where the system can find it.  One
> >> >way to fix the problem is to set LD_LIBRARY_PATH appropriate (see
> >> >ld.so(8)).
> >> >
> >> >On Thu, May 10, 2018 at 03:33:48AM +0800, Mingming Chen wrote:
> >> >> Hi Ben,
> >> >> 
> >> >> 
> >> >> Thanks for your hint about ovs dpdk!. I have done something according 
> >> >> to: https://docs.openvswitch.org/en/latest/intro/install/dpdk/ and 
> >> >> https://docs.openvswitch.org/en/latest/intro/install/general/#general-building.
> >> >>  But now I met a problem. (I think I have done with dpdk part because 
> >> >> no errors showed)
> >> >> About the ovs part, I did
> >> >> $ ./configure 
> >> >> --with-dpdk=/usr/src/dpdk-stable-17.11.1/x86_64-native-linuxapp-gcc
> >> >> $ make
> >> >> $ make install
> >> >> $ export PATH=$PATH:/usr/local/share/openvswitch/scripts
> >> >> $ ovs-ctl start 
> >> >> (Here I met a problem) like:
> >> >> ovsdb-tool: error while loading shared libraries: librte_eal.so.6.1: 
> >> >> cannot open shared object file: No such file or directory
> >> >>  * /usr/local/etc/openvswitch/conf.db does not exist
> >> >> ovsdb-tool: error while loading shared libraries: librte_eal.so.6.1: 
> >> >> cannot open shared object file: No such file or directory
> >> >>  * Creating empty database /usr/local/etc/openvswitch/conf.db
> >> >> 
> >> >> 
> >> >> I have searched but didn't find a good one. Could you help me to figure 
> >> >> out where I am wrong? I am using Ubuntu 16.04.4, ovs 2.9.0, dpdk 
> >> >> 17.11.1. Thank you!
> >> >> 
> >> >> 
> >> >> 
> >> >> 
> >> >> Best regards,
> >> >> Mingming
> >> >> 
> >> >> At 2018-05-09 03:51:02, "Ben Pfaff"  wrote:
> >> >> >OVS has two main datapaths: kernel-based and userspace-based.  If you
> >> >> >use the default setup, you're using the kernel-based datapath.  The
> >> >> >documentation explains how to use the userspace datapath instead.
> >> >> >
> >> >> >On Tue, May 08, 2018 at 11:59:15PM +0800, Mingming Ch

[ovs-dev] [PATCH] rhel: openvswitch-fedora.spec.in: Specify PYTHON and PYTHON3

[Timothy Redaelli]

Currently python2 and python3 binaries are searched by following the
PATHs, but, on Fedora, the python2 package does not provides /bin/python2
and so if the PATH contains /bin before /usr/bin (for example by using
the ansible poc) then the resulting RPM file will require /bin/python2
instead of /usr/bin/python2 and this breaks some tools (for example
createrepo).

This patch specify the full path of python2 interpreter and,
if python3-openvswitch package is built, the full path of python3
interpreter.

Reported-by: Ansis Atteka 
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-May/346796.html
Signed-off-by: Timothy Redaelli 
---
 rhel/openvswitch-fedora.spec.in | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 6dbf20ce1..9462ce236 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -222,7 +222,13 @@ Docker network plugins for OVN.
--with-dpdk=$(dirname %{_datadir}/dpdk/*/.config) \
 %endif
--enable-ssl \
-   --with-pkidir=%{_sharedstatedir}/openvswitch/pki
+   --with-pkidir=%{_sharedstatedir}/openvswitch/pki \
+%if 0%{?fedora} > 22 || %{with build_python3}
+   PYTHON3=%{__python3} \
+   PYTHON=%{__python2}
+%else
+   PYTHON=%{__python}
+%endif
 
 build-aux/dpdkstrip.py \
 %if %{with dpdk}
-- 
2.17.0

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

[ovs-dev] Interop ITX Show 2018 Attendees List

[Lita Beavers]

Hi,

Good day to you,


This is an outstanding offer for the Interop ITX Show 2018.


I am writing to check if you would be interested in acquiring the list
of *19,623
*attendees participated in Interop ITX Show 2018 for your marketing and
sales initiatives.


This is an opportunity to acquire the list of attendees contact details for
a robust marketing campaign which will eventually help you convert the
compiled leads in to phenomenal sales deal and also you can per-invite them
to visit your booth.

You will receive the file for permanent usage where you can use this list
for multiple campaigns.

Please find below mentioned data fields for your review.

*Company Name, Company URL, Contact Name, Title, Phone number, Fax Number,
Email Address, Company Address, Industry type, SIC Code, and Social Media
Link.*


Please revert with your interest.

Regards,

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

Re: [ovs-dev] [PATCH v7] Configurable Link State Change (LSC) detection mode

[Stokes, Ian]

> It is possible to set LSC detection mode to polling or interrupt mode for
> DPDK interfaces. The default is polling mode. To set interrupt mode,
> option dpdk-lsc-interrupt has to be set to true.
> 
> For detailed description and usage see the dpdk install documentation.
> 

Thanks Robert, LGTM,

I've made the following incremental

diff --git a/NEWS b/NEWS
index 39f42c5..269faea 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,7 @@ Post-v2.9.0
- DPDK:
  * New 'check-dpdk' Makefile target to run a new system testsuite.
See Testing topic for the details.
+ * Add support for LSC interrupt mode for DPDK devices.

I've also moved the documentation you added from 

'Documentation/intro/install/dpdk.rst' to 'Documentation/topics/dpdk/phy.rst' 
as I think the proper location for the configuration options.

If this is ok with you I'll put this on DPDK_MERGE, I believe we said it should 
be backported also to OVS 2.9 and OVS 2.8 at least.

Thanks
Ian

> Signed-off-by: Robert Mulik 
> ---
> v5 -> v6:
> - DPDK install documentation updated.
> - Status of lsc_interrupt_mode of DPDK interfaces can be read by command
>   ovs-appctl dpif/show.
> - It was suggested to check if the HW supports interrupt mode, but it is
> not
>   possible to do without DPDK code change, so it is skipped from this
> patch.
> V6 -> v7
> - DPDK install documentation updated.
> - Commit message updated.
> - Error message updated with LSC failure.
> - Code refactorized based on the comments for v6.
> ---
>  Documentation/intro/install/dpdk.rst | 24 
>  lib/netdev-dpdk.c| 33 ---
> --
>  vswitchd/vswitch.xml | 17 +
>  3 files changed, 69 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/intro/install/dpdk.rst
> b/Documentation/intro/install/dpdk.rst
> index fea4890..c47ea13 100644
> --- a/Documentation/intro/install/dpdk.rst
> +++ b/Documentation/intro/install/dpdk.rst
> @@ -628,6 +628,30 @@ The average number of packets per output batch can be
> checked in PMD stats::
> 
>  $ ovs-appctl dpif-netdev/pmd-stats-show
> 
> +Link State Change (LSC) detection configuration
> +~~~
> +
> +There are two methods to get the information when Link State Change
> +(LSC) happens on a network interface: by polling or interrupt.
> +
> +Configuring the lsc detection mode has no direct effect on OVS itself,
> +instead it configures the NIC how it should handle link state changes.
> +Processing the link state update request triggered by OVS takes less
> +time using interrupt mode, since the NIC updates its link state in the
> +background, while in polling mode the link state has to be fetched from
> +the firmware every time to fulfil this request.
> +
> +Note that not all PMD drivers support LSC interrupts.
> +
> +The default configuration is polling mode. To set interrupt mode,
> +option ``dpdk-lsc-interrupt`` has to be set to ``true``.
> +
> +Command to set interrupt mode for a specific interface::
> +$ ovs-vsctl set interface 
> +options:dpdk-lsc-interrupt=true
> +
> +Command to set polling mode for a specific interface::
> +$ ovs-vsctl set interface 
> +options:dpdk-lsc-interrupt=false
> +
>  Limitations
>  
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ee39cbe..c2ec463
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -434,6 +434,12 @@ struct netdev_dpdk {
>  /* DPDK-ETH hardware offload features,
>   * from the enum set 'dpdk_hw_ol_features' */
>  uint32_t hw_ol_features;
> +
> +/* Properties for link state change detection mode.
> + * If lsc_interrupt_mode is set to false, poll mode is used,
> + * otherwise interrupt mode is used. */
> +bool requested_lsc_interrupt_mode;
> +bool lsc_interrupt_mode;
>  );
> 
>  PADDED_MEMBERS(CACHE_LINE_SIZE,
> @@ -689,12 +695,14 @@ dpdk_watchdog(void *dummy OVS_UNUSED)  }
> 
>  static int
> -dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq)
> +dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int n_rxq, int n_txq)
>  {
>  int diag = 0;
>  int i;
>  struct rte_eth_conf conf = port_conf;
> 
> +conf.intr_conf.lsc = dev->lsc_interrupt_mode;
> +
>  /* For some NICs (e.g. Niantic), scatter_rx mode needs to be
> explicitly
>   * enabled. */
>  if (dev->mtu > ETHER_MTU) {
> @@ -804,10 +812,13 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
>  n_rxq = MIN(info.max_rx_queues, dev->up.n_rxq);
>  n_txq = MIN(info.max_tx_queues, dev->up.n_txq);
> 
> -diag = dpdk_eth_dev_queue_setup(dev, n_rxq, n_txq);
> +diag = dpdk_eth_dev_port_config(dev, n_rxq, n_txq);
>  if (diag) {
> -VLOG_ERR("Interface %s(rxq:%d txq:%d) configure error: %s",
> - dev->up.name, n_rxq, n_txq, rte_strerror(-diag));
> +VLOG_ERR("Interface %s(rxq:%d txq:%d lsc interrupt mode:%

[ovs-dev] [PATCH] Fix datapath compilation on RHEL >= 7.5

[Daniel Alvarez]

On RHEL 7.5 we get compilation errors due to field ndo_change_mtu
missing. This patch checks the RHEL version and redefines it to
ndo_change_mtu_rh74.

Reported-by: Lucas Alvares 
Signed-off-by: Daniel Alvarez 
---
 datapath/datapath.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/datapath/datapath.h b/datapath/datapath.h
index 93c9ed505..d418bf381 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -36,6 +37,10 @@
 #define DP_MAX_PORTS   USHRT_MAX
 #define DP_VPORT_HASH_BUCKETS  1024
 
+#if RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7,5)
+#define ndo_change_mtu ndo_change_mtu_rh74
+#endif
+
 /**
  * struct dp_stats_percpu - per-cpu packet processing statistics for a given
  * datapath.
-- 
2.15.1 (Apple Git-101)

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

Re: [ovs-dev] [PATCH v2 0/5] selinux: introduce a transition domain for loading kmods

[Aaron Conole]

Ansis Atteka  writes:

> On Fri, 4 May 2018 at 11:28, Aaron Conole  wrote:
>
>> On linux systems, the initial start of openvswitch attempts to load
>> the openvswitch.ko kernel module.  This module allows openvswitch to
>> utilize the kernel datapath.
>
>> Some of these linux systems, notably Fedora and RHEL, use selinux to
>> enforce additional restrictions on various processes by way of allowing
>> or disallowing access from a specific selinux domain to a particular
>> operation on an selinux type.  On these systems, the openvswitch
>> initialization will be run from the 'openvswitch_t' selinux domain.
>> Attempts by a process in the 'openvswitch_t' selinux domain to load a
>> kernel module will be denied.
>
>> One solution would be to simply allow 'openvswitch_t' to load a kernel
>> directly.  This essentially means that 'openvswitch_t' would really be
>> 'unconfined_t' - since an attacker that can control the code can issue
>> a kernel load.
>
>> The solution implemented here uses a labeled file in the openvswitch
>> scripts directory, which is writable only by root.  That file will force
>> a domain transition to the 'openvswitch_load_module_t' domain.  The
>> 'openvswitch_load_module_t' domain will then be granted permissions to
>> load a kernel module.
>
>> The labelling won't take place until after the changes implemented in 4/4,
>> so it is really important to test the automatic labelling after that
> point.
>
>> v1->v2:
>> * Added a new commit to set the selinux-policy module version
>> * Added changes to the centos build in 4/4 to match the fedora build
>> * Fixed the manpage in 1/5
>
>
> Thanks. I wanted to test this on Fedora 27 too (with poc/builders), but I am
> getting this error:
>
> [root@fedoraubuilder x86_64]# yum install openvswitch-2.9.90-1.x86_64.rpm
> Last metadata expiration check: 1:18:01 ago on Wed 09 May 2018 09:56:08 PM
> UTC.
> Error:
>Problem: conflicting requests
> - nothing provides /bin/python2 needed by openvswitch-2.9.90-1.x86_64
> [root@fedoraubuilder x86_64]# /bin/python2 --version
> Python 2.7.14
> [root@fedoraubuilder x86_64]# rpm -q --whatprovides /bin/python
> python2-2.7.14-10.fc27.x86_64
> [root@fedoraubuilder x86_64]# rpm -qR  openvswitch-2.9.90-1.x86_64.rpm  |
> grep -i python
> /bin/python2
>
> based on openvswitch-fedora.spec.in file my understanding is that it should
> have picked python3 right? Though, I also have python2 so that error seems
> strange to me.

I think it's related to the changes that went in with commit
db8dcbaf1c57 ("packaging: Make Fedora spec file CentOS compatible")
but I don't know for sure.

Timothy? Leif?

>> Aaron Conole (5):
>> ovs-kmod-ctl: introduce a kernel module load script
>> selinux: create a transition type for module loading
>> selinux: tag the custom policy version
>> selinux: introduce domain transitioned kmod helper
>> rhel: selinux-policy to invoke proper label macros
>
>>debian/openvswitch-switch.install  |   1 +
>>debian/openvswitch-switch.manpages |   1 +
>>rhel/openvswitch-fedora.spec.in|  12 +-
>>rhel/openvswitch.spec.in   |  12 +-
>>selinux/.gitignore |   4 +
>>selinux/automake.mk|   3 +-
>>selinux/openvswitch-custom.fc.in   |   1 +
>>selinux/openvswitch-custom.te.in   |  81 -
>>utilities/.gitignore   |   1 +
>>utilities/automake.mk  |   5 +
>>utilities/ovs-ctl.in   |  32 +-
>>utilities/ovs-kmod-ctl.8   | 109 ++
>>utilities/ovs-kmod-ctl.in  | 228
> +
>>utilities/ovs-lib.in   |  12 +-
>>14 files changed, 454 insertions(+), 48 deletions(-)
>>create mode 100644 selinux/openvswitch-custom.fc.in
>>create mode 100644 utilities/ovs-kmod-ctl.8
>>create mode 100644 utilities/ovs-kmod-ctl.in
>
>> --
>> 2.14.3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Delivery In progress.

[FedEx Express via dev]

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

Re: [ovs-dev] [PATCH v9 0/7] OVS-DPDK flow offload with rte_flow

[Shahaf Shuler]

Hi,

Friday, April 20, 2018 11:07 AM, Stokes, Ian:
> Subject: RE: [PATCH v9 0/7] OVS-DPDK flow offload with rte_flow
> 
> > > Hi,
> > >
> > > Here is a joint work from Mellanox and Napatech, to enable the flow
> > > hw offload with the DPDK generic flow interface (rte_flow).
> >
> > Hi folks, I feel Mellanox/Netronome have reached the point where HWOL
> > can be introduced to OVS DPDK pending performance review.
> 
> Apologies , Mellanox an Napatech.

Any progress with this series? 


> 
> Ian
> >
> > This has not been an easy path, but I will put one final request out
> > there for comments.
> >
> > If there are none then pending on Travis compilation and final
> > performance checks I'm thinking of introducing this to the code base.
> >
> > Keep in mind this feature is experimental, not only can it be changed
> > but it can also be removed pending future review if not maintained.
> >
> > I invite all to give opinions here as I think this is an important
> > feature.
> >
> > Sincere thanks to all involved.
> >
> > Regards
> > Ian
> >
> >
> > >
> > > The basic idea is to associate the flow with a mark id (a unit32_t
> > > number).
> > > Later, we then get the flow directly from the mark id, which could
> > > bypass some heavy CPU operations, including but not limiting to mini
> > > flow extract, emc lookup, dpcls lookup, etc.
> > >
> > > The association is done with CMAP in patch 1. The CPU workload
> > > bypassing is done in patch 2. The flow offload is done in patch 3,
> > > which mainly does two things:
> > >
> > > - translate the ovs match to DPDK rte flow patterns
> > > - bind those patterns with a RSS + MARK action.
> > >
> > > Patch 5 makes the offload work happen in another thread, for leaving
> > > the datapath as light as possible.
> > >
> > > A PHY-PHY forwarding with 1000 mega flows (udp,tp_src=1000-1999) and
> > > 1 million streams (tp_src=1000-1999, tp_dst=2000-2999) show more
> > > than 260% performance boost.
> > >
> > > Note that it's disabled by default, which can be enabled by:
> > >
> > > $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
> > >
> > > v9: - introduced IP packet sanity checks in a seperate commit
> > > - moved structs and enums definition to the begining of the file
> > > - fixed sparse compilation error (error is assumed to be fixed,
> > issues
> > >   on Travis CI preventing from fully testing it.
> > >
> > > v8: - enhanced documentation with more details on supported protocols
> > > - fixed VLOG to start with capital letter
> > > - fixed compilation issues
> > > - fixed coding style
> > > - addressed the rest of Ian's comments
> > >
> > > v7: - fixed wrong hash for mark_to_flow that has been refactored in v6
> > > - set the rss_conf for rss action to NULL, to workaround a mlx5
> > change
> > >   in DPDK v17.11. Note that it will obey the rss settings
> > > OVS-DPDK
> > has
> > >   set in the beginning. Thus, nothing should be effected.
> > >
> > > v6: - fixed a sparse warning
> > > - added documentation
> > > - used hash_int to compute mark to flow hash
> > > - added more comments
> > > - added lock for pot lookup
> > > - rebased on top of the latest code
> > >
> > > v5: - fixed an issue that it took too long if we do flow add/remove
> > >   repeatedly.
> > > - removed an unused mutex lock
> > > - turned most of the log level to DBG
> > > - rebased on top of the latest code
> > >
> > > v4: - use RSS action instead of QUEUE action with MARK
> > > - make it work with multiple queue (see patch 1)
> > > - rebased on top of latest code
> > >
> > > v3: - The mark and id association is done with array instead of CMAP.
> > > - Added a thread to do hw offload operations
> > > - Removed macros completely
> > > - dropped the patch to set FDIR_CONF, which is a workround some
> > >   Intel NICs.
> > > - Added a debug patch to show all flow patterns we have created.
> > > - Misc fixes
> > >
> > > v2: - workaround the queue action issue
> > > - fixed the tcp_flags being skipped issue, which also fixed the
> > >   build warnings
> > > - fixed l2 patterns for Intel nic
> > > - Converted some macros to functions
> > > - did not hardcode the max number of flow/action
> > > - rebased on top of the latest code
> > >
> > > Thanks.
> > >
> > > ---
> > >
> > > Finn Christensen (1):
> > >   netdev-dpdk: implement flow offload with rte flow
> > >
> > > Yuanhan Liu (6):
> > >   dpif-netdev: associate flow with a mark id
> > >   flow: Introduce IP packet sanity checks
> > >   dpif-netdev: retrieve flow directly from the flow mark
> > >   netdev-dpdk: add debug for rte flow patterns
> > >   dpif-netdev: do hw flow offload in a thread
> > >   Documentation: document ovs-dpdk flow offload
> > >
> > >  Documentation/howto/dpdk.rst |  22 ++
> > >  NEWS |   3 +-
> > >  lib/dp-packet.h  |  13 +
> > >  lib/dpif-netdev.c   

Re: [ovs-dev] [PATCH] Factor prerequisites out of AND/OR trees with unique symbol

[Jakub Sitnicki]

On Wed, 9 May 2018 13:02:55 -0700
Ben Pfaff  wrote:

> On Thu, Apr 26, 2018 at 09:54:06PM +0200, Jakub Sitnicki wrote:
> > Appending prerequisites to sub-expressions of OR that are all over one
> > symbol prevents the expression-to-matches converter from applying
> > conjunctive matching. This happens during the annotation phase.
> > 
> > input:  s1 == { c1, c2 } && s2 == { c3, c4 }
> > expanded:   (s1 == c1 || s1 == c2) && (s2 == c3 || s2 == c4)
> > annotated:  ((p1 && s1 == c1) || (p1 && s1 == c2)) &&
> > ((p2 && s2 == c3) || (p2 && s2 == c4))
> > normalized: (p1 && p2 && s1 == c1 && s2 == c3) ||
> > (p1 && p2 && s1 == c1 && s2 == c4) ||
> > (p1 && p2 && s1 == c2 && s2 == c3) ||
> > (p1 && p2 && s1 == c2 && s2 == c4)
> > 
> > Where s1,s2 - symbols, c1..c4 - constants, p1,p2 - prerequisites.
> > 
> > Since sub-expressions of OR trees that are over one symbol all have the
> > same prerequisites, we can factor them out leaving the OR tree in tact,
> > and enabling the converter to apply conjunctive matching to
> > AND(OR(clause)) trees.
> > 
> > Going back to our example this change gives us:
> > 
> > input:  s1 == { c1, c2 } && s2 == { c3, c4 }
> > expanded:   (s1 == c1 || s1 == c2) && (s2 == c3 || s2 == c4)
> > annotated:  (s1 == c1 || s1 == c2) && p1 && (s2 == c3 || s2 == c4) && p2
> > normalized: p1 && p2 && (s1 == c1 || s1 == c2) && (s2 == c3 || s2 == c4)
> > 
> > We also factor out the prerequisites out of pure AND or mixed AND/OR
> > trees to keep the common code path, but in this case we don't gain
> > anything.
> > 
> > Signed-off-by: Jakub Sitnicki   
> 
> This is nice.  Thank you.
> 
> I suggest folding in the following to better document and to better
> match usual OVS style:
> 
> diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
> index 9dd8d6f17abe..bac8e1efd53f 100644
> --- a/ovn/lib/expr.c
> +++ b/ovn/lib/expr.c
> @@ -32,13 +32,10 @@
>  
>  VLOG_DEFINE_THIS_MODULE(expr);
>  
> -static const struct expr_symbol *
> -expr_get_unique_symbol(const struct expr *expr);
> -
> -struct expr *
> -expr_annotate__(struct expr *expr, const struct shash *symtab,
> -bool append_prereqs, struct ovs_list *nesting, char 
> **errorp);
> -
> +static const struct expr_symbol *expr_get_unique_symbol(const struct expr *);
> +struct expr *expr_annotate__(struct expr *, const struct shash *symtab,
> + bool append_prereqs, struct ovs_list *nesting,
> + char **errorp);
>  static struct expr *parse_and_annotate(const char *s,
> const struct shash *symtab,
> struct ovs_list *nesting,
> @@ -1771,6 +1768,15 @@ error:
>  return NULL;
>  }
>  
> +/* Same interface and purpose as expr_annotate(), with a couple of additional
> + * parameters for internal bookkeeping.
> + *
> + * Uses 'nesting' to ensure that a given symbol is not recursively expanded.
> + *
> + * Ordinarily, annotation adds prerequisites to the expression, and that's 
> what
> + * this function does if 'append_prereqs' is true.  If 'append_prereqs' is
> + * false, this function ignores prerequisites (in which case the caller must
> + * have arranged to deal with them). */
>  struct expr *
>  expr_annotate__(struct expr *expr, const struct shash *symtab,
>  bool append_prereqs, struct ovs_list *nesting, char **errorp)
> 
> I also wonder about the following, because it seems like currently the
> append_prereqs parameter is only being honored for a single level of
> AND/OR, not when expr_annotate__() recurses:
> 
> diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
> index 9dd8d6f17abe..a33b4eb46933 100644
> --- a/ovn/lib/expr.c
> +++ b/ovn/lib/expr.c
> @@ -1782,12 +1788,25 @@ expr_annotate__(struct expr *expr, const struct shash 
> *symtab,
>  
>  case EXPR_T_AND:
>  case EXPR_T_OR: {
> -const struct expr_symbol *unique_symbol = 
> expr_get_unique_symbol(expr);
> +/* Detect whether every term in 'expr' mentions the same symbol.  If
> + * so, then suppress prerequisites for that symbol for those terms 
> and
> + * instead apply them once at our higher level.
> + *
> + * If 'append_prereqs' is true, though, we're not supposed to handle
> + * prereqs at all (because our caller is already doing it). */
> +const struct expr_symbol *unique_symbol = NULL;
> +if (!append_prereqs) {
> +unique_symbol = expr_get_unique_symbol(expr);
> +if (unique_symbol) {
> +append_prereqs = false;
> +}
> +}
> +
>  struct expr *sub, *next;
>  
>  LIST_FOR_EACH_SAFE (sub, next, node, &expr->andor) {
>  ovs_list_remove(&sub->node);
> -struct expr *new_sub = expr_annotate__(sub, symtab, 
> !unique_symbol,
> +struct expr *new_sub = expr_annotate__(sub, symtab, 
> append_prereqs,
>   

[ovs-dev] Invitation: I NEED YOUR SUPORT @ Thu 10 May 2018 08:00 - 09:00 (EDT) (d...@openvswitch.org)

[aogani1900]

You have been invited to the following event.

Title: I NEED YOUR SUPORT
I am, 45 years old African American, from Ohio, United States of America.

I am a builder with over 15 ears experience building high end projects, Oil  
Rigs, Concrete Silos, Malls, High Rise Condominiums, Turnkey Government  
Projects and even low cost homes using insulated panels (SIPS). My Job has  
taken me beyond the USA Borders to Asia, South America, Middle East and  
Africa.

About 3 years ago, my wife and my only child died in a fatal auto
accident along Highway SR 174 near Cleveland suburb of Gates Mills.
I lost all interest in life and had to undergo therapy and counselling
before I returned to work.
I was afraid to contact people out there for the project i had have at  
hand, and my doctor advised me to try communicate people abroad in order to  
forget and start a new life.

I am therefore looking for a friend, comforter, business partner, and a
real person,
Are you the person to trust?, who will help and restore my miserable
life back,believe again, and become a real man?

Please respond if you care to explore the possibilities of life with me!
When: Thu 10 May 2018 08:00 – 09:00 Eastern Time
Calendar: d...@openvswitch.org
Who:
(Guest list has been hidden at organiser's request)

Event details:  
https://www.google.com/calendar/event?action=VIEW&eid=NWdkcXAwZzRiaXAyb2dsNXZwNTR0NWJnazEgZGV2QG9wZW52c3dpdGNoLm9yZw&tok=MjAjYW9nYW5pMTkwMEBnbWFpbC5jb203ODk1MGZjMzU3ZThjMTdlMGZhODM1ZDJmMDE2OTQwNjcxYjY0NWQy&ctz=America%2FNew_York&hl=en_GB&es=0


Invitation from Google Calendar: https://www.google.com/calendar/

You are receiving this courtesy email at the account d...@openvswitch.org  
because you are an attendee of this event.


To stop receiving future updates for this event, decline this event.  
Alternatively, you can sign up for a Google account at  
https://www.google.com/calendar/ and control your notification settings for  
your entire calendar.


Forwarding this invitation could allow any recipient to modify your RSVP  
response. Learn more at  
https://support.google.com/calendar/answer/37135#forwarding
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1] Fix crash when processing malformed Bundle Add message in OVS

[Anju Thomas]

Hi Ben,

Yes. This patch certainly looks more cleaner and better to me .

Thanks
Anju
 -Original Message-
From: Ben Pfaff [mailto:b...@ovn.org] 
Sent: Thursday, May 10, 2018 2:33 AM
To: Anju Thomas 
Cc: d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v1] Fix crash when processing malformed Bundle 
Add message in OVS

On Mon, May 07, 2018 at 10:58:06PM +0530, Anju Thomas wrote:
> When an OpenFlow Bundle Add message is received, a bundle entry is 
> created and the OpenFlow message embedded in the bundle add message is 
> processed.  If any error is encountered while processing the embedded 
> message, the bundle entry is freed. The bundle entry free function 
> assumes that the entry has been populated with a properly formatted 
> OpenFlow message and performs some message specific cleanup actions .
> This assumption does not hold true in the error case and OVS crashes 
> when performing the cleanup.
> 
> The fix is in case of errors, simply free the bundle entry without 
> attempting to perform any embedded message cleanup
> 
> Signed-off-by: Anju Thomas 

Thanks for the fix.

It's not really nice to have multiple levels of initialized-ness.  What do you 
think of this fix instead?

Thanks,

Ben.

--8<--cut here-->8--

From: Anju Thomas 
Date: Mon, 7 May 2018 22:58:06 +0530
Subject: [PATCH] Fix crash when processing malformed Bundle Add message in OVS

When an OpenFlow Bundle Add message is received, a bundle entry is created and 
the OpenFlow message embedded in the bundle add message is processed.  If any 
error is encountered while processing the embedded message, the bundle entry is 
freed. The bundle entry free function assumes that the entry has been populated 
with a properly formatted OpenFlow message and performs some message specific 
cleanup actions .
This assumption does not hold true in the error case and OVS crashes when 
performing the cleanup.

The fix is in case of errors, simply free the bundle entry without attempting 
to perform any embedded message cleanup

Signed-off-by: Anju Thomas 
Signed-off-by: Ben Pfaff 
---
 ofproto/bundles.h | 13 -
 ofproto/ofproto.c | 21 +
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/ofproto/bundles.h b/ofproto/bundles.h index 
806e853d6883..1164fddd8702 100644
--- a/ofproto/bundles.h
+++ b/ofproto/bundles.h
@@ -58,8 +58,6 @@ struct ofp_bundle {
 struct ovs_list   msg_list;  /* List of 'struct bundle_message's */
 };
 
-static inline struct ofp_bundle_entry *ofp_bundle_entry_alloc(
-enum ofptype type, const struct ofp_header *oh);
 static inline void ofp_bundle_entry_free(struct ofp_bundle_entry *);
 
 enum ofperr ofp_bundle_open(struct ofconn *, uint32_t id, uint16_t flags, @@ 
-72,17 +70,6 @@ enum ofperr ofp_bundle_add_message(struct ofconn *, uint32_t id,
 
 void ofp_bundle_remove__(struct ofconn *, struct ofp_bundle *);
 

-static inline struct ofp_bundle_entry * -ofp_bundle_entry_alloc(enum ofptype 
type, const struct ofp_header *oh) -{
-struct ofp_bundle_entry *entry = xmalloc(sizeof *entry);
-
-entry->type = type;
-entry->msg = xmemdup(oh, ntohs(oh->length));
-
-return entry;
-}
-
 static inline void
 ofp_bundle_entry_free(struct ofp_bundle_entry *entry)  { diff --git 
a/ofproto/ofproto.c b/ofproto/ofproto.c index 36f4c0b16d48..565a45db1507 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -7899,7 +7899,6 @@ handle_bundle_add(struct ofconn *ofconn, const struct 
ofp_header *oh)
 struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
 enum ofperr error;
 struct ofputil_bundle_add_msg badd;
-struct ofp_bundle_entry *bmsg;
 enum ofptype type;
 
 error = reject_slave_controller(ofconn); @@ -7912,7 +7911,8 @@ 
handle_bundle_add(struct ofconn *ofconn, const struct ofp_header *oh)
 return error;
 }
 
-bmsg = ofp_bundle_entry_alloc(type, badd.msg);
+/* Allocate bundle entry and decode the embedded message. */
+struct ofp_bundle_entry *bmsg = xmalloc(sizeof *bmsg);
 
 struct ofpbuf ofpacts;
 uint64_t ofpacts_stub[1024 / 8];
@@ -7951,18 +7951,23 @@ handle_bundle_add(struct ofconn *ofconn, const struct 
ofp_header *oh)
 } else {
 OVS_NOT_REACHED();
 }
-
 ofpbuf_uninit(&ofpacts);
-
-if (!error) {
-error = ofp_bundle_add_message(ofconn, badd.bundle_id, badd.flags,
-   bmsg, oh);
+if (error) {
+free(bmsg);
+return error;
 }
 
+/* Now that the embedded message has been successfully decoded, finish up
+ * initializing the bundle entry. */
+bmsg->type = type;
+bmsg->msg = xmemdup(oh, ntohs(oh->length));
+
+/* Add bundle entry to bundle. */
+error = ofp_bundle_add_message(ofconn, badd.bundle_id, badd.flags,
+   bmsg, oh);
 if (error) {
 ofp_bundle_entry_free(bmsg);
 }
-
 return error;
 }
 
--
2.16.1

__

Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action

[Anju Thomas]

Hi Ben,

Replies inline:

Regards & thanks
Anju

-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org] 
Sent: Thursday, May 10, 2018 1:59 AM
To: Anju Thomas 
Cc: d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action

On Tue, May 08, 2018 at 12:34:54AM +0530, Anju Thomas wrote:
> During slow path packet processing, if the action is to output to a 
> tunnel port, the slow path processing of the encapsulated packet 
> continues on the underlay bridge and additional actions (e.g. optional 
> VLAN encapsulation, bond link selection and finally output to port) 
> are collected there.
> 
> To prepare for a continuation of the processing of the original packet 
> (e.g. output to other tunnel ports in a flooding scenario), the 
> “tunnel_push” action and the actions of the underlay bridge are 
> encapsulated in a clone() action to preserve the original packet.
> 
> If the underlay bridge decides to drop the tunnel packet (for example 
> if both bonded ports are down simultaneously), the clone(tunnel_push)) 
> actions previously generated as part of translation of the output to 
> tunnel port are discarded and a stand-alone tunnel_push action is 
> added instead. Thus the tunnel header is pushed on to the original packet.
> This is the bug.
> 
> Consequences: If packet processing continues with sending to further 
> tunnel ports, multiple tunnel header pushes will happen on the 
> original packet as typically the tunnels all traverse the same 
> underlay bond which is down. The packet may not have enough headroom 
> to accommodate all the tunnel headers. OVS crashes if it runs out of 
> space while trying to push the tunnel headers.
> 
> Even in case there is enough headroom, the packet will not be freed 
> since the accumulated action list contains only the tunnel header push 
> action without any output port action. Thus, we either have a crash or 
> a packet buffer leak.
> 
> Signed-off-by: Anju Thomas 

Thanks for the patch.  It applies OK and all the tests pass.

It looks like your commit message describes at least two other bugs in OVS, 
though.  First, if OVS crashes when it pushes tunnel headers, even if there's 
not enough headroom, that's really bad.  At worst, it should drop the packet.  
Do you know where the crash occurs?  We should fix the problem, since it might 
recur in some other context.

[Anju] OVS was actually crashing in the dpdk datapath. The crash is a manual 
assert in our case.
The rootcause is that dp receives the actions after the upcall (say with >=3 
tunnel pushes ) . Now as part of action processing , since it is a tunnel push 
action , we try to find space in the dpdk mbuf packet headroom (which Is 128 
bytes). By the time we try to process the third tunnel push , there is no 
headroom left since each tunnel header is of 50 bytes (50 *3 > 128 bytes 
headroom). Hence it manually asserts .  This assert is to catch any unexpected 
code flow . Do you think that in this case we should still go ahead and  
prevent the crash ? 

The back trace was as below:-
(gdb) bt
#0  0x7ffa9a856c37 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x7ffa9a85a028 in __GI_abort () at abort.c:89
#2  0x005fc725 in dp_packet_resize__ (b=b@entry=0x7ffa87744680, 
new_headroom=new_headroom@entry=64, new_tailroom=)
at lib/dp-packet.c:258
#3  0x005fcb1f in dp_packet_prealloc_headroom 
(b=b@entry=0x7ffa87744680, size=size@entry=50) at lib/dp-packet.c:288
#4  0x005fcf91 in dp_packet_push_uninit (b=b@entry=0x7ffa87744680, 
size=size@entry=50) at lib/dp-packet.c:400
#5  0x0069466c in netdev_tnl_push_ip_header 
(packet=packet@entry=0x7ffa87744680, header=0x7ff85401bab0, size=50,
ip_tot_size=ip_tot_size@entry=0x7ff8117f89fc) at lib/netdev-native-tnl.c:152
#6  0x0069472a in netdev_tnl_push_udp_header (packet=0x7ffa87744680, 
data=) at lib/netdev-native-tnl.c:215
#7  0x00625627 in netdev_push_header (netdev=0x3ca3990, 
batch=batch@entry=0x7ff8117f9498, data=data@entry=0x7ff85401baa0)
at lib/netdev.c:874
#8  0x006069f2 in push_tnl_action (batch=0x7ff8117f9498, 
attr=0x7ff85401ba9c, pmd=0x33efb30) at lib/dpif-netdev.c:4629
#9  dp_execute_cb (aux_=aux_@entry=0x7ff8117f9840, 
packets_=packets_@entry=0x7ff8117f9498, a=a@entry=0x7ff85401ba9c, 
may_steal=false)


static void
dp_packet_resize__(struct dp_packet *b, size_t new_headroom, size_t 
new_tailroom)
{
void *new_base, *new_data;
size_t new_allocated;

new_allocated = new_headroom + dp_packet_size(b) + new_tailroom;

switch (b->source) {
case DPBUF_DPDK:

OVS_NOT_REACHED();-->crashes
 here



Second, it's a little shocking to hear that an encapsulation action without a 
following output action causes a memory leak.  We also need to fix that.  Do 
you have any more details?
[Anju] Now as expla

[ovs-dev] [PATCH v1] Avoid packet drop on LACP bond after link up

[Manohar Krishnappa Chidambaraswamy]

Problem:

During port DOWN->UP of link (slave) in a LACP bond, after receiving the
LACPDU with SYNC set for both actor and partner, the bond-slave remains
"disabled" until OVS main thread runs LACP state machine and eventually
"enables" the bond-slave. With this, we have observed delays in the order
of 350ms and packets are  dropped in OVS due to bond-admissibility check
(packets received on slave in "disabled" state are dropped).

Fix:

When a LACPDU is received, evaluate whether LACP slave can be enabled
(slave_may_enable()) and set LACP slave's may_enable from the datapath
thread itself. When may_enable = TRUE, it means L1 state is UP and
LACP-SYNC is done and it is waiting for the main thread to enable the
slave. Relax the check in bond_check_admissibility() to check for both
"enable" and "may_enable" of the LACP slave. This would avoid dropping
of packets until the main thread enables the slave from bundle_run().

Signed-off-by: Manohar K C

CC: Jan Scheurich 
CC: Nitin Katiyar 

---
 lib/lacp.c   | 14 --
 lib/lacp.h   |  3 ++-
 ofproto/bond.c   | 18 +++---
 ofproto/ofproto-dpif-xlate.c | 13 -
 4 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/lib/lacp.c b/lib/lacp.c
index 8353746..8d97ad5 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -154,6 +154,7 @@ static struct slave *slave_lookup(const struct lacp *, 
const void *slave)
 OVS_REQUIRES(mutex);
 static bool info_tx_equal(struct lacp_info *, struct lacp_info *)
 OVS_REQUIRES(mutex);
+static bool slave_may_enable__(struct slave *slave) OVS_REQUIRES(mutex);
 
 static unixctl_cb_func lacp_unixctl_show;
 static unixctl_cb_func lacp_unixctl_show_stats;
@@ -324,8 +325,8 @@ lacp_is_active(const struct lacp *lacp) OVS_EXCLUDED(mutex)
 /* Processes 'packet' which was received on 'slave_'.  This function should be
  * called on all packets received on 'slave_' with Ethernet Type ETH_TYPE_LACP.
  */
-void
-lacp_process_packet(struct lacp *lacp, const void *slave_,
+bool
+lacp_process_packet(struct lacp *lacp, const void *bond, const void *slave_,
 const struct dp_packet *packet)
 OVS_EXCLUDED(mutex)
 {
@@ -333,6 +334,7 @@ lacp_process_packet(struct lacp *lacp, const void *slave_,
 const struct lacp_pdu *pdu;
 long long int tx_rate;
 struct slave *slave;
+bool lacp_may_enable = false;
 
 lacp_lock();
 slave = slave_lookup(lacp, slave_);
@@ -362,8 +364,16 @@ lacp_process_packet(struct lacp *lacp, const void *slave_,
 slave->partner = pdu->actor;
 }
 
+/*
+ * Evaluate may_enable here to avoid dropping of packets till main thread
+ * sets may_enable to true.
+ */
+lacp_may_enable = slave_may_enable__(slave);
+
 out:
 lacp_unlock();
+
+return lacp_may_enable;
 }
 
 /* Returns the lacp_status of the given 'lacp' object (which may be NULL). */
diff --git a/lib/lacp.h b/lib/lacp.h
index f35cff5..1505c2c 100644
--- a/lib/lacp.h
+++ b/lib/lacp.h
@@ -46,7 +46,8 @@ struct lacp *lacp_ref(const struct lacp *);
 void lacp_configure(struct lacp *, const struct lacp_settings *);
 bool lacp_is_active(const struct lacp *);
 
-void lacp_process_packet(struct lacp *, const void *slave,
+bool lacp_process_packet(struct lacp *, const void *bond,
+ const void *slave,
  const struct dp_packet *packet);
 enum lacp_status lacp_status(const struct lacp *);
 
diff --git a/ofproto/bond.c b/ofproto/bond.c
index 11d28e1..7ca3687 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -777,6 +777,7 @@ bond_check_admissibility(struct bond *bond, const void 
*slave_,
 {
 enum bond_verdict verdict = BV_DROP;
 struct bond_slave *slave;
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 
 ovs_rwlock_rdlock(&rwlock);
 slave = bond_slave_lookup(bond, slave_);
@@ -794,7 +795,13 @@ bond_check_admissibility(struct bond *bond, const void 
*slave_,
  * drop all incoming traffic except if lacp_fallback_ab is enabled. */
 switch (bond->lacp_status) {
 case LACP_NEGOTIATED:
-verdict = slave->enabled ? BV_ACCEPT : BV_DROP;
+/*
+ * To reduce packet-drops due to delay in enabling of slave (post
+ * LACP-SYNC), from main thread, check for may_enable as well.
+ * When may_enable is TRUE, it means LACP is UP and waiting for
+ * the main thread to run LACP state machine and enable the slave.
+ */
+verdict = (slave->enabled || slave->may_enable) ? BV_ACCEPT : BV_DROP;
 goto out;
 case LACP_CONFIGURED:
 if (!bond->lacp_fallback_ab) {
@@ -830,8 +837,6 @@ bond_check_admissibility(struct bond *bond, const void 
*slave_,
 /* Drop all packets which arrive on backup slaves.  This is similar to
  * how Linux bonding handles active-backup bonds. */
 if (bond->active_slave != slave) {
-static struct vlog_rate_limit rl = VLO