Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Fix truncate and native tunnnel

2017-10-20 Thread Guru Shetty 
On 13 October 2017 at 21:33, William Tu  wrote:

> Previous commit a67b337dc281 breaks the truncate and native
> tunnel testcase by removing the truncate flag.  The patch fixes
> it by putting it back.  Reproduce the error by:
> > make check-system-userspace TESTSUITEFLAGS='17'
>
> Fixes: a67b337dc281 ("ofproto-dpif-xlate: Remove assertion for truncated")
> Cc: IWASE Yusuke 
> Signed-off-by: William Tu 
>

Thanks. I applied this to master.


> ---
>  ofproto/ofproto-dpif-xlate.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index cd3715562a57..ddcaf059ded2 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4829,13 +4829,14 @@ xlate_output_action(struct xlate_ctx *ctx,
>  bool is_last_action)
>  {
>  ofp_port_t prev_nf_output_iface = ctx->nf_output_iface;
> +bool truncate = max_len != 0;
>
>  ctx->nf_output_iface = NF_OUT_DROP;
>
>  switch (port) {
>  case OFPP_IN_PORT:
>  compose_output_action(ctx, ctx->xin->flow.in_port.ofp_port, NULL,
> -  is_last_action, false);
> +  is_last_action, truncate);
>  break;
>  case OFPP_TABLE:
>  xlate_table_action(ctx, ctx->xin->flow.in_port.ofp_port,
> @@ -4864,7 +4865,7 @@ xlate_output_action(struct xlate_ctx *ctx,
>  case OFPP_LOCAL:
>  default:
>  if (port != ctx->xin->flow.in_port.ofp_port) {
> -compose_output_action(ctx, port, NULL, is_last_action, false);
> +compose_output_action(ctx, port, NULL, is_last_action,
> truncate);
>  } else {
>  xlate_report(ctx, OFT_WARN, "skipping output to input port");
>  }
> --
> 2.7.4
>
> ___
> 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] rhel.rst: Add python-sphinx as a dependency.

2017-10-20 Thread Guru Shetty 
Thanks you Yi-Hung and Aaron. I applied this with your suggestions.

On 20 October 2017 at 13:12, Yi-Hung Wei  wrote:

> Thanks for the patch. LGTM.
>
> Acked-by: Yi-Hung Wei 
>
> BTW, I think we can also update the docs in fedora.rst. as the following.
>
> --- a/Documentation/intro/install/fedora.rst
> +++ b/Documentation/intro/install/fedora.rst
> @@ -43,7 +43,7 @@ in the :doc:`general`. Specific packages (by package
> name) include:
>  - autoconf automake libtool
>  - systemd-units openssl openssl-devel
>  - python2-devel python3-devel
> -- python2 python2-twisted python2-zope-interface python2-six
> +- python2 python2-twisted python2-zope-interface python2-six python-sphinx
>  - desktop-file-utils
>  - groff graphviz
>  - procps-ng
>
> On Fri, Oct 20, 2017 at 2:27 AM, Gurucharan Shetty  wrote:
> > Signed-off-by: Gurucharan Shetty 
> > ---
> >  Documentation/intro/install/rhel.rst | 6 +-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/intro/install/rhel.rst
> b/Documentation/intro/install/rhel.rst
> > index 86c5cf3..b68cb7d 100644
> > --- a/Documentation/intro/install/rhel.rst
> > +++ b/Documentation/intro/install/rhel.rst
> > @@ -76,7 +76,11 @@ the below command::
> >
> >  $ yum install gcc make python-devel openssl-devel kernel-devel
> graphviz \
> >  kernel-debug-devel autoconf automake rpm-build
> redhat-rpm-config \
> > -libtool checkpolicy selinux-policy-devel
> > +libtool checkpolicy selinux-policy-devel python-sphinx
> > +
> > +.. note::
> > +  If python-sphinx package is not available in RHEL, you can install it
> > +  via pip with 'pip install sphinx'.
> >
> >  .. _rhel-bootstrapping:
> >
> > --
> > 1.9.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] rhel.rst: Add python-sphinx as a dependency.

2017-10-20 Thread Yi-Hung Wei 
Thanks for the patch. LGTM.

Acked-by: Yi-Hung Wei 

BTW, I think we can also update the docs in fedora.rst. as the following.

--- a/Documentation/intro/install/fedora.rst
+++ b/Documentation/intro/install/fedora.rst
@@ -43,7 +43,7 @@ in the :doc:`general`. Specific packages (by package
name) include:
 - autoconf automake libtool
 - systemd-units openssl openssl-devel
 - python2-devel python3-devel
-- python2 python2-twisted python2-zope-interface python2-six
+- python2 python2-twisted python2-zope-interface python2-six python-sphinx
 - desktop-file-utils
 - groff graphviz
 - procps-ng

On Fri, Oct 20, 2017 at 2:27 AM, Gurucharan Shetty  wrote:
> Signed-off-by: Gurucharan Shetty 
> ---
>  Documentation/intro/install/rhel.rst | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/intro/install/rhel.rst 
> b/Documentation/intro/install/rhel.rst
> index 86c5cf3..b68cb7d 100644
> --- a/Documentation/intro/install/rhel.rst
> +++ b/Documentation/intro/install/rhel.rst
> @@ -76,7 +76,11 @@ the below command::
>
>  $ yum install gcc make python-devel openssl-devel kernel-devel graphviz \
>  kernel-debug-devel autoconf automake rpm-build redhat-rpm-config \
> -libtool checkpolicy selinux-policy-devel
> +libtool checkpolicy selinux-policy-devel python-sphinx
> +
> +.. note::
> +  If python-sphinx package is not available in RHEL, you can install it
> +  via pip with 'pip install sphinx'.
>
>  .. _rhel-bootstrapping:
>
> --
> 1.9.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

[ovs-dev] [PATCH v2] rhel.rst: Add python-sphinx as a dependency.

2017-10-20 Thread Gurucharan Shetty 
Signed-off-by: Gurucharan Shetty 
---
 Documentation/intro/install/rhel.rst | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/intro/install/rhel.rst 
b/Documentation/intro/install/rhel.rst
index 86c5cf3..b68cb7d 100644
--- a/Documentation/intro/install/rhel.rst
+++ b/Documentation/intro/install/rhel.rst
@@ -76,7 +76,11 @@ the below command::
 
 $ yum install gcc make python-devel openssl-devel kernel-devel graphviz \
 kernel-debug-devel autoconf automake rpm-build redhat-rpm-config \
-libtool checkpolicy selinux-policy-devel
+libtool checkpolicy selinux-policy-devel python-sphinx
+
+.. note::
+  If python-sphinx package is not available in RHEL, you can install it
+  via pip with 'pip install sphinx'.
 
 .. _rhel-bootstrapping:
 
-- 
1.9.1

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

Re: [ovs-dev] [PATCH] ovs-save: Handle different 'ip addr show' output.

2017-10-20 Thread Guru Shetty 
On 20 October 2017 at 11:25, Yi-Hung Wei  wrote:

> Acked-by: Yi-Hung Wei 
>

Thanks. I applied this to master and 2.8


>
> On Wed, Oct 18, 2017 at 10:45 PM, Gurucharan Shetty  wrote:
> > On RHEL 7.4 (with iproute-3.10.0-87), a DHCP provided
> > ipv4 address has the "dynamic" keyword set. For e.g
> > "ip addr show breth0 | grep inet" shows:
> >
> > inet 10.116.248.91/20 brd 10.116.255.255 scope global dynamic breth0
> > inet6 fe80::250:56ff:fea8:fdf0/64 scope link
> >
> > The keyword "dynamic" (according to 'man ip-address') is only
> > used for ipv6, but in this case this is not true. Our current
> > code will skip the ipv4 address restoration because of this.
> >
> > With this commit, we special case "dynamic" keyword to be valid
> > in case of ipv4.
> >
> > VMware-BZ: #1982196
> > Signed-off-by: Gurucharan Shetty 
> > ---
> >  utilities/ovs-lib.in | 8 
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> > index 8665698..ac2da85 100644
> > --- a/utilities/ovs-lib.in
> > +++ b/utilities/ovs-lib.in
> > @@ -329,6 +329,14 @@ move_ip_address () {
> >  while test $# != 0; do
> >  case $1 in
> >  dynamic)
> > +# XXX: According to 'man ip-address', "dynamic" is
> only
> > +# used for ipv6 addresses.  But, atleast on RHEL 7.4
> > +# (iproute-3.10.0-87), it is being used for ipv4
> > +# addresses assigned with dhcp.
> > +if [ "$family" = "inet" ]; then
> > +shift
> > +continue
> > +fi
> >  # Omit kernel-maintained route.
> >  continue 2
> >  ;;
> > --
> > 1.9.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

[ovs-dev] Bug#878249: Please package >= 2.7.0

2017-10-20 Thread Thomas Goirand 
Hi Ben,

I don't mind becoming a co-maintainer, though there's a few things that
need to change to facilitate packaging. First of all, I'd like to switch
to a Git packaging workflow, using git-buildpackage. The debian folder
upstream here should be removed:

https://github.com/openvswitch/ovs/tree/master/debian

otherwise, it will clash with what we have in Alioth. Then I'll push
openvswitch in the Alioth git repository under /git/third-party.

Also, I will remove the debian/automake.mk and the d/copyright hack.
Indeed, Debian needs list of copyright holders, not authors.

Is that fine for you? I'm starting...

Cheers,

Thomas Goirand (zigo)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovs-save: Handle different 'ip addr show' output.

2017-10-20 Thread Yi-Hung Wei 
Acked-by: Yi-Hung Wei 

On Wed, Oct 18, 2017 at 10:45 PM, Gurucharan Shetty  wrote:
> On RHEL 7.4 (with iproute-3.10.0-87), a DHCP provided
> ipv4 address has the "dynamic" keyword set. For e.g
> "ip addr show breth0 | grep inet" shows:
>
> inet 10.116.248.91/20 brd 10.116.255.255 scope global dynamic breth0
> inet6 fe80::250:56ff:fea8:fdf0/64 scope link
>
> The keyword "dynamic" (according to 'man ip-address') is only
> used for ipv6, but in this case this is not true. Our current
> code will skip the ipv4 address restoration because of this.
>
> With this commit, we special case "dynamic" keyword to be valid
> in case of ipv4.
>
> VMware-BZ: #1982196
> Signed-off-by: Gurucharan Shetty 
> ---
>  utilities/ovs-lib.in | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index 8665698..ac2da85 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -329,6 +329,14 @@ move_ip_address () {
>  while test $# != 0; do
>  case $1 in
>  dynamic)
> +# XXX: According to 'man ip-address', "dynamic" is only
> +# used for ipv6 addresses.  But, atleast on RHEL 7.4
> +# (iproute-3.10.0-87), it is being used for ipv4
> +# addresses assigned with dhcp.
> +if [ "$family" = "inet" ]; then
> +shift
> +continue
> +fi
>  # Omit kernel-maintained route.
>  continue 2
>  ;;
> --
> 1.9.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

[ovs-dev] [PATCH 2/3] system-common-macros: Check for ct_clear action in datapath

2017-10-20 Thread Eric Garver 
New macro OVS_CHECK_CT_CLEAR() to check if ct_clear action is supported
by the datapath.

Signed-off-by: Eric Garver 
---
 tests/system-common-macros.at | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 73ae4829dac4..f7d4adb947a0 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -319,3 +319,7 @@ m4_define([OVS_CHECK_8021AD],
 # OVS_CHECK_IPROUTE_ENCAP()
 m4_define([OVS_CHECK_IPROUTE_ENCAP],
 [AT_SKIP_IF([! ip route help 2>&1 |grep encap >/dev/null])])
+
+# OVS_CHECK_CT_CLEAR()
+m4_define([OVS_CHECK_CT_CLEAR],
+[AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" 
ovs-vswitchd.log])])
-- 
2.12.0

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

[ovs-dev] [PATCH 3/3] system-traffic: Add conntrack floating IP test

2017-10-20 Thread Eric Garver 
This test cases uses floating IP (FIP) addresses for each endpoint. If
the destination is a FIP, the packet will undergo a transformation of
the form (dst=FIP, src=non-FIP) --> (dst=non-FIP, src=FIP) before
egress. Otherwise the packet is untouched.

This exercises the ct_clear action in the datapath.

Signed-off-by: Eric Garver 
---
 tests/system-traffic.at | 73 +
 1 file changed, 73 insertions(+)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 522eaa615834..7cc1e1e21187 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -3996,6 +3996,79 @@ ovs-ofctl -O OpenFlow15 dump-group-stats br0
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([conntrack - floating IP])
+AT_SKIP_IF([test $HAVE_NC = no])
+CHECK_CONNTRACK()
+OVS_TRAFFIC_VSWITCHD_START()
+OVS_CHECK_CT_CLEAR()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01") dnl FIP 
10.254.254.1
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02") dnl FIP 
10.254.254.2
+
+dnl Static ARPs
+NS_CHECK_EXEC([at_ns0], [ip neigh add 10.1.1.2 lladdr f0:00:00:01:01:02 dev 
p0])
+NS_CHECK_EXEC([at_ns1], [ip neigh add 10.1.1.1 lladdr f0:00:00:01:01:01 dev 
p1])
+
+dnl Static ARP and route entries for the FIP "gateway"
+NS_CHECK_EXEC([at_ns0], [ip neigh add 10.1.1.254 lladdr f0:00:00:01:01:FE dev 
p0])
+NS_CHECK_EXEC([at_ns1], [ip neigh add 10.1.1.254 lladdr f0:00:00:01:01:FE dev 
p1])
+NS_CHECK_EXEC([at_ns0], [ip route add default nexthop via 10.1.1.254])
+NS_CHECK_EXEC([at_ns1], [ip route add default nexthop via 10.1.1.254])
+
+NETNS_DAEMONIZE([at_ns0], [nc -l -k > /dev/null], [nc0.pid])
+
+AT_DATA([flows.txt], [dnl
+table=0,priority=10  ip action=ct(table=1)
+table=0,priority=1   action=drop
+dnl dst FIP
+table=1,priority=20  ip,ct_state=+trk+est,nw_dst=10.254.254.0/24 
action=goto_table:10
+table=1,priority=20  ip,ct_state=+trk+new,nw_dst=10.254.254.0/24 
action=ct(commit,table=10)
+dnl dst local
+table=1,priority=10  ip,ct_state=+trk+est action=goto_table:20
+table=1,priority=10  ip,ct_state=+trk+new action=ct(commit,table=20)
+table=1,priority=1   ip,ct_state=+trk+inv action=drop
+dnl
+dnl FIP translation (dst FIP, src local) --> (dst local, src FIP)
+table=10 ip,nw_dst=10.254.254.1 
action=set_field:10.1.1.1->nw_dst,goto_table:11
+table=10 ip,nw_dst=10.254.254.2 
action=set_field:10.1.1.2->nw_dst,goto_table:11
+table=11 ip,nw_src=10.1.1.1 
action=set_field:10.254.254.1->nw_src,goto_table:12
+table=11 ip,nw_src=10.1.1.2 
action=set_field:10.254.254.2->nw_src,goto_table:12
+dnl clear conntrack and do another lookup since we changed the tuple
+table=12,priority=10 ip action=ct_clear,ct(table=13)
+table=12,priority=1  action=drop
+table=13 ip,ct_state=+trk+est action=goto_table:20
+table=13 ip,ct_state=+trk+new action=ct(commit,table=20)
+table=13 ip,ct_state=+trk+inv action=drop
+dnl
+dnl Output
+table=20 ip,nw_src=10.1.1.1 
action=set_field:f0:00:00:01:01:01->eth_src,goto_table:21
+table=20 ip,nw_src=10.1.1.2 
action=set_field:f0:00:00:01:01:02->eth_src,goto_table:21
+table=20 ip,nw_src=10.254.254.0/24 
action=set_field:f0:00:00:01:01:FE->eth_src,goto_table:21
+table=21 ip,nw_dst=10.1.1.1 
action=set_field:f0:00:00:01:01:01->eth_dst,output:ovs-p0
+table=21 ip,nw_dst=10.1.1.2 
action=set_field:f0:00:00:01:01:02->eth_dst,output:ovs-p1
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl non-FIP case
+NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.1.1.1])
+OVS_WAIT_UNTIL([[ovs-appctl dpctl/dump-conntrack | sed -e 
's/port=[0-9]*/port=/g' -e 's/id=[0-9]*/id=/g' |
+grep 
"tcp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),reply=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),protoinfo=(state=TIME_WAIT)"
+]])
+
+dnl Check that the full session ends as expected (i.e. TIME_WAIT). Otherwise it
+dnl means the datapath didn't process the ct_clear action. Ending in SYN_RECV
+dnl (OVS maps to ESTABLISHED) means the initial frame was committed, but not a
+dnl second time after the FIP translation (because ct_clear didn't occur).
+NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.254.254.1])
+OVS_WAIT_UNTIL([[ovs-appctl dpctl/dump-conntrack | sed -e 
's/port=[0-9]*/port=/g' -e 's/id=[0-9]*/id=/g' |
+grep 
"tcp,orig=(src=10.254.254.2,dst=10.1.1.1,sport=,dport=),reply=(src=10.1.1.1,dst=10.254.254.2,sport=,dport=),protoinfo=(state=TIME_WAIT)"
+]])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_BANNER([802.1ad])
 
 AT_SETUP([802.1ad - vlan_limit])
-- 
2.12.0

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

[ovs-dev] [PATCH 0/3] Add dpif support for ct_clear action

2017-10-20 Thread Eric Garver 
This series adds dpif support for OVS_ACTION_ATTR_CT_CLEAR. Previously the
ct_clear action was a userspace only operation, but it has use cases in the
dpif as well. Namely changing a packet's tuple after a ct() lookup has
occurred.

Kernel support has already be accepted upstream:
  b8226962b1c4 ("openvswitch: add ct_clear action").

travis-ci: https://travis-ci.org/erig0/ovs/builds/290487503

Eric Garver (3):
  dpif: Add support for OVS_ACTION_ATTR_CT_CLEAR
  system-common-macros: Check for ct_clear action in datapath
  system-traffic: Add conntrack floating IP test

 datapath/linux/compat/include/linux/openvswitch.h |  1 +
 lib/conntrack.c   | 10 
 lib/conntrack.h   |  1 +
 lib/dpif-netdev.c |  1 +
 lib/dpif.c|  1 +
 lib/odp-execute.c |  7 +++
 lib/odp-util.c|  4 ++
 lib/ofp-actions.c |  1 +
 ofproto/ofproto-dpif-ipfix.c  |  1 +
 ofproto/ofproto-dpif-sflow.c  |  1 +
 ofproto/ofproto-dpif-xlate.c  | 14 -
 ofproto/ofproto-dpif.c| 32 ++
 ofproto/ofproto-dpif.h|  5 +-
 tests/system-common-macros.at |  4 ++
 tests/system-traffic.at   | 73 +++
 15 files changed, 154 insertions(+), 2 deletions(-)

-- 
2.12.0

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

[ovs-dev] [PATCH] rhel.rst: Add python-sphinx as a dependency.

2017-10-20 Thread Gurucharan Shetty 
Signed-off-by: Gurucharan Shetty 
---
 Documentation/intro/install/rhel.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/intro/install/rhel.rst 
b/Documentation/intro/install/rhel.rst
index 86c5cf3..aff6ccf 100644
--- a/Documentation/intro/install/rhel.rst
+++ b/Documentation/intro/install/rhel.rst
@@ -76,7 +76,7 @@ the below command::
 
 $ yum install gcc make python-devel openssl-devel kernel-devel graphviz \
 kernel-debug-devel autoconf automake rpm-build redhat-rpm-config \
-libtool checkpolicy selinux-policy-devel
+libtool checkpolicy selinux-policy-devel python-sphinx
 
 .. _rhel-bootstrapping:
 
-- 
1.9.1

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

Re: [ovs-dev] [PATCH v2] datapath-windows: Update OvsIPv4TunnelKey flags in geneve decap.

2017-10-20 Thread Alin Serdean 
Thanks Anand and Sai. I applied this on master and on branch-2.8.

Should we apply this on branch-2.7 also?

Thanks,
Alin.

> -Original Message-
> From: Sairam Venugopal [mailto:vsai...@vmware.com]
> Sent: Friday, October 20, 2017 12:15 AM
> To: Anand Kumar ; d...@openvswitch.org
> Cc: Alin Serdean ; Gurucharan Shetty
> 
> Subject: Re: [ovs-dev] [PATCH v2] datapath-windows: Update
> OvsIPv4TunnelKey flags in geneve decap.
> 
> Thanks for fixing this.
> 
> Alin/Guru - Can we back port this to 2.8 too? This causes flow misses when
> geneve options are present.
> 
> Acked-by: Sairam Venugopal 
> 
> 
> 
> 
> 
> 
> On 10/19/17, 1:26 PM, "ovs-dev-boun...@openvswitch.org on behalf of
> Anand Kumar"  kumaran...@vmware.com> wrote:
> 
> >Currently, the OvsLookupFlow fails for the decap packet, when the
> >Geneve options are present in the packet as the OvsIPv4TunnelKey flags
> >are not set in the Geneve decap.
> >
> >Set the OvsIPv4TunnelKey flags OVS_TNL_F_OAM and
> OVS_TNL_F_CRT_OPT in
> >OvsDecapGeneve based on the geneve header. Also set
> >OVS_TNL_F_GENEVE_OPT if the packet has geneve options.
> >
> >Signed-off-by: Anand Kumar 
> >---
> > datapath-windows/ovsext/Geneve.c | 9 +
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> >diff --git a/datapath-windows/ovsext/Geneve.c
> >b/datapath-windows/ovsext/Geneve.c
> >index 43374e2..6dca69b 100644
> >--- a/datapath-windows/ovsext/Geneve.c
> >+++ b/datapath-windows/ovsext/Geneve.c
> >@@ -324,10 +324,10 @@ NDIS_STATUS
> OvsDecapGeneve(POVS_SWITCH_CONTEXT switchContext,
> > status = STATUS_NDIS_INVALID_PACKET;
> > goto dropNbl;
> > }
> >-tunKey->flags = OVS_TNL_F_KEY;
> >-if (geneveHdr->oam) {
> >-tunKey->flags |= OVS_TNL_F_OAM;
> >-}
> >+/* Update tunnelKey flags. */
> >+tunKey->flags = OVS_TNL_F_KEY | (geneveHdr->oam ?
> OVS_TNL_F_OAM : 0) |
> >+(geneveHdr->critical ? OVS_TNL_F_CRT_OPT : 0);
> >+
> > tunKey->tunnelId = GENEVE_VNI_TO_TUNNELID(geneveHdr->vni);
> > tunKey->tunOptLen = (uint8)geneveHdr->optLen * 4;
> > if (tunKey->tunOptLen > TUN_OPT_MAX_LEN || @@ -349,6 +349,7 @@
> >NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT switchContext,
> > memcpy(TunnelKeyGetOptions(tunKey), optStart, tunKey-
> >tunOptLen);
> > }
> > NdisAdvanceNetBufferDataStart(curNb, tunKey->tunOptLen, FALSE,
> >NULL);
> >+tunKey->flags |= OVS_TNL_F_GENEVE_OPT;
> > }
> >
> > return NDIS_STATUS_SUCCESS;
> >--
> >2.9.3.windows.1
> >
> >___
> >dev mailing list
> >d...@openvswitch.org
> >https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__mail.openvswitch.o
> >rg_mailman_listinfo_ovs-
> 2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=Z6vow
> >HUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=BRjHHHPFJJP1H7BWHj
> wF0OS4UFpTbB
> >iTQNM1SPvh51w=JCowvVY7CQ4CxJWnx8dPPqUbvS346xU066Dd0DnMI8
> A=
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] datapath-windows: Update OvsIPv4TunnelKey flags in geneve decap.

2017-10-20 Thread aserdean 
Acked-by: Alin Gabriel Serdean 

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Anand Kumar
> Sent: Thursday, October 19, 2017 11:26 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH v2] datapath-windows: Update
> OvsIPv4TunnelKey flags in geneve decap.
> 
> Currently, the OvsLookupFlow fails for the decap packet, when the Geneve
> options are present in the packet as the OvsIPv4TunnelKey flags are not
set
> in the Geneve decap.
> 
> Set the OvsIPv4TunnelKey flags OVS_TNL_F_OAM and
> OVS_TNL_F_CRT_OPT in OvsDecapGeneve based on the geneve header.
> Also set OVS_TNL_F_GENEVE_OPT if the packet has geneve options.
> 
> Signed-off-by: Anand Kumar 
> ---
>  datapath-windows/ovsext/Geneve.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/datapath-windows/ovsext/Geneve.c b/datapath-
> windows/ovsext/Geneve.c
> index 43374e2..6dca69b 100644
> --- a/datapath-windows/ovsext/Geneve.c
> +++ b/datapath-windows/ovsext/Geneve.c
> @@ -324,10 +324,10 @@ NDIS_STATUS
> OvsDecapGeneve(POVS_SWITCH_CONTEXT switchContext,
>  status = STATUS_NDIS_INVALID_PACKET;
>  goto dropNbl;
>  }
> -tunKey->flags = OVS_TNL_F_KEY;
> -if (geneveHdr->oam) {
> -tunKey->flags |= OVS_TNL_F_OAM;
> -}
> +/* Update tunnelKey flags. */
> +tunKey->flags = OVS_TNL_F_KEY | (geneveHdr->oam ?
> OVS_TNL_F_OAM : 0) |
> +(geneveHdr->critical ? OVS_TNL_F_CRT_OPT : 0);
> +
>  tunKey->tunnelId = GENEVE_VNI_TO_TUNNELID(geneveHdr->vni);
>  tunKey->tunOptLen = (uint8)geneveHdr->optLen * 4;
>  if (tunKey->tunOptLen > TUN_OPT_MAX_LEN || @@ -349,6 +349,7 @@
> NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT switchContext,
>  memcpy(TunnelKeyGetOptions(tunKey), optStart, tunKey-
> >tunOptLen);
>  }
>  NdisAdvanceNetBufferDataStart(curNb, tunKey->tunOptLen, FALSE,
> NULL);
> +tunKey->flags |= OVS_TNL_F_GENEVE_OPT;
>  }
> 
>  return NDIS_STATUS_SUCCESS;
> --
> 2.9.3.windows.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

[ovs-dev] [PATCH v2] Documentation: Add document describing RBAC

2017-10-20 Thread Mark Michelson 
Role based access control is a relatively new addition to OVS/OVN, and
aside from the database documentation in ovn-sb(5), there is not much
explaining what RBAC is, how to use it, and the available roles. This
document remedies that situation.

It is hopeful that any new roles added will be added to this document in
the future.

Signed-off-by: Mark Michelson 
---
Version 2 changes:
* There were references to a table called RBAC_Permissions. These have
  been changed to the correct "RBAC_Permission".
* Fixed a grammatical error in the final section.
---
 Documentation/automake.mk  |  1 +
 Documentation/topics/index.rst |  1 +
 Documentation/topics/role-based-access-control.rst | 97 ++
 3 files changed, 99 insertions(+)
 create mode 100644 Documentation/topics/role-based-access-control.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 6f38912f2..5344cde74 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -41,6 +41,7 @@ DOC_SOURCE = \
Documentation/topics/openflow.rst \
Documentation/topics/ovsdb-replication.rst \
Documentation/topics/porting.rst \
+   Documentation/topics/role-based-access-control.rst \
Documentation/topics/tracing.rst \
Documentation/topics/windows.rst \
Documentation/howto/index.rst \
diff --git a/Documentation/topics/index.rst b/Documentation/topics/index.rst
index f68334b4b..00d6b0b83 100644
--- a/Documentation/topics/index.rst
+++ b/Documentation/topics/index.rst
@@ -57,6 +57,7 @@ OVN
:maxdepth: 2
 
high-availability
+   role-based-access-control
 
 .. list-table::
 
diff --git a/Documentation/topics/role-based-access-control.rst 
b/Documentation/topics/role-based-access-control.rst
new file mode 100644
index 0..14b3bc8ac
--- /dev/null
+++ b/Documentation/topics/role-based-access-control.rst
@@ -0,0 +1,97 @@
+..
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+=
+Role Based Access Control
+=
+
+Where SSL provides authentication when connecting to an OVS database, role
+based access control (RBAC) provides authorization to operations performed
+by clients connecting to an OVS database. RBAC allows for administrators to
+restrict the database operations a client may perform and thus enhance the
+security already provided by SSL.
+
+In theory, any OVS database could define RBAC roles and permissions, but at
+present only the OVN southbound database has the appropriate tables defined to
+facilitate RBAC.
+
+Mechanics
+-
+RBAC is intended to supplement SSL. In order to enable RBAC, the connection to
+the database must use SSL. Some permissions in RBAC are granted based on the
+certificate common name (CN) of the connecting client.
+
+RBAC is controlled with two database tables, RBAC_Role and RBAC_Permission.
+The RBAC_Permission table contains records that describe a set of permissions
+for a given table in the database.
+
+The RBAC_Permission table contains the following columns:
+
+- table: The table in the database for which permissions are being described.
+- insert_delete: Describes whether insertion and deletion of records is
+  allowed.
+- update: A list of columns that are allowed to be updated.
+- authorization: A list of column names. One of the listed columns must match
+  the SSL certificate CN in order for the attempted operation on the table to
+  succeed. If a key-value pair is provided, then the key is the column name,
+  and the value is the name of a key in that column. An empty string gives
+  permission to all clients to perform operations.
+
+The RBAC_Role table contains the following columns:
+
+- name: The name of the role being defined
+- permissions: A list of key-value pairs. The key is the name of a table in the
+  database, and the value is a UUID of a record in the RBAC_Permission
+  table that describes the permissions the role has for that
+  table.
+
+.. note::
+
+   All tables not explicitly referenced in an RBAC_Role record are read-only
+
+In order to enable RBAC,

[ovs-dev] [PATCH] netdev-dpdk: replace uint8_t with dpdk_port_t

2017-10-20 Thread Mark Kavanagh 
netdev_dpdk_detach() declares a 'port_id' variable, of type uint8_t.
This variable should instead be of type dpdk_port_t.

Fixes: bb37956ac ("netdev-dpdk: Use uint8_t for port_id.")
CC: Ilya Maximets 
Signed-off-by: Mark Kavanagh 
---
 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c60f46f..1f6345d 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2549,7 +2549,7 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
 {
 int ret;
 char *response;
-uint8_t port_id;
+dpdk_port_t port_id;
 char devname[RTE_ETH_NAME_MAX_LEN];
 struct netdev_dpdk *dev;

--
1.9.3

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

[ovs-dev] [PATCH] netdev-dpdk: replace uint8_t with dpdk_port_t

2017-10-20 Thread Mark Kavanagh 
netdev_dpdk_detach() declares a 'port_id' variable, of type uint8_t.
This variable should instead be of type dpdk_port_t.

Fixes: bb37956ac ("netdev-dpdk: Use uint8_t for port_id.")
CC: Ilya Maximets 
Signed-off-by: Mark Kavanagh 
---
 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c60f46f..1f6345d 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2549,7 +2549,7 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
 {
 int ret;
 char *response;
-uint8_t port_id;
+dpdk_port_t port_id;
 char devname[RTE_ETH_NAME_MAX_LEN];
 struct netdev_dpdk *dev;

--
1.9.3

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

Re: [ovs-dev] [PATCH net-next v12] openvswitch: enable NSH support

2017-10-20 Thread Yang, Yi 
On Fri, Oct 20, 2017 at 04:05:35PM +0800, Jiri Benc wrote:
> On Fri, 20 Oct 2017 05:53:12 +0800, Yang, Yi wrote:
> > For push_nsh, my typical use scinario is push_nsh then set then output
> > to vxlangpe port.
> 
> Okay.

Then I just need to do the below change against v12.

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 1ab3c51..a3a663c 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -658,10 +658,8 @@ static int set_nsh(struct sk_buff *skb, struct
sw_flow_key *flow_key,
return err;

/* Make sure the NSH base header is there */
-   err = skb_ensure_writable(skb, skb_network_offset(skb) +
-  NSH_BASE_HDR_LEN);
-   if (unlikely(err))
-   return err;
+   if (!pskb_may_pull(skb, NSH_BASE_HDR_LEN))
+   return -ENOMEM;

nh = nsh_hdr(skb);
length = nsh_hdr_len(nh);

I'll send out v13 with this if you're ok.

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

Re: [ovs-dev] [PATCH 3/3] dpif-netdev: Calculate rxq cycles prior to rxq_cycle_sort calls.

2017-10-20 Thread Kevin Traynor 
Ping again. This was requested to make the sort function behave in a
more standard manner. I don't think there's anything controversial in it.

thanks,
Kevin.

On 09/22/2017 08:21 PM, Darrell Ball wrote:
> Are there any other comments for this patch?
> 
> 
> On 8/30/17, 11:01 AM, "Darrell Ball"  wrote:
> 
> Thanks for the patch Kevin
> 
> On 8/30/17, 10:45 AM, "Kevin Traynor"  wrote:
> 
> rxq_cycle_sort summed the latest cycles from each queue for sorting.
> While each comparison was correct with the latest cycles, the cycles
> could change between calls to rxq_cycle_sort. In order to use
> consistent values through each call to rxq_cycle_sort, sum the cycles
> prior to rxq_cycle_sort being called.
> 
> As discussed, these changes are optional and have some tradeoffs, but
> overall, I don’t see any major issue introduced here, since this is 
> understood to be a
> rough comparison anyways.
> 
> Also, change return to 0 when values are equal.
> 
> As discussed, this means the equal tie-breaker is done by qsort instead of
> the compare function; the net practical effect of this is nil, but this 
> is more
> standard.
> 
> Reported-by: Ilya Maximets 
> Signed-off-by: Kevin Traynor 
> ---
>  lib/dpif-netdev.c | 22 +-
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 1db9f10..7c21ee5 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3432,18 +3432,14 @@ rxq_cycle_sort(const void *a, const void *b)
>  struct dp_netdev_rxq *qb;
>  uint64_t total_qa, total_qb;
> -unsigned i;
>  
>  qa = *(struct dp_netdev_rxq **) a;
>  qb = *(struct dp_netdev_rxq **) b;
>  
> -total_qa = total_qb = 0;
> -for (i = 0; i < PMD_RXQ_INTERVAL_MAX; i++) {
> -total_qa += dp_netdev_rxq_get_intrvl_cycles(qa, i);
> -total_qb += dp_netdev_rxq_get_intrvl_cycles(qb, i);
> -}
> -dp_netdev_rxq_set_cycles(qa, RXQ_CYCLES_PROC_HIST, total_qa);
> -dp_netdev_rxq_set_cycles(qb, RXQ_CYCLES_PROC_HIST, total_qb);
> +total_qa = dp_netdev_rxq_get_cycles(qa, RXQ_CYCLES_PROC_HIST);
> +total_qb = dp_netdev_rxq_get_cycles(qb, RXQ_CYCLES_PROC_HIST);
>  
> -if (total_qa >= total_qb) {
> +if (total_qa == total_qb) {
> +return 0;
> +} else if (total_qa > total_qb) {
>  return -1;
>  }
> @@ -3493,4 +3489,6 @@ rxq_scheduling(struct dp_netdev *dp, bool 
> pinned) OVS_REQUIRES(dp->port_mutex)
>  }
>  } else if (!pinned && q->core_id == OVS_CORE_UNSPEC) {
> +uint64_t cycle_hist = 0;
> +
>  if (n_rxqs == 0) {
>  rxqs = xmalloc(sizeof *rxqs);
> @@ -3498,4 +3496,10 @@ rxq_scheduling(struct dp_netdev *dp, bool 
> pinned) OVS_REQUIRES(dp->port_mutex)
>  rxqs = xrealloc(rxqs, sizeof *rxqs * (n_rxqs + 
> 1));
>  }
> +
> +for (unsigned i = 0; i < PMD_RXQ_INTERVAL_MAX; i++) {
> +cycle_hist += dp_netdev_rxq_get_intrvl_cycles(q, 
> i);
> +}
> +dp_netdev_rxq_set_cycles(q, RXQ_CYCLES_PROC_HIST, 
> cycle_hist);
> +
>  /* Store the queue. */
>  rxqs[n_rxqs++] = q;
> -- 
> 1.8.3.1
> 
> 
> 
> 
> 

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

Re: [ovs-dev] [PATCH 1/3] dpif-netdev: Rename rxq_interval.

2017-10-20 Thread Kevin Traynor 
Ping again. This is a simple variable rename that was requested.

On 09/22/2017 08:22 PM, Darrell Ball wrote:
> Are there any other comments?
> 
> 
> 
> On 8/30/17, 10:49 AM, "Darrell Ball"  wrote:
> 
> Thanks Kevin
> 
> Naming is hard.
> The name looks a bit more intuitive and matches closely with the 
> description previously added.
> 
> Darrell
> 
> On 8/30/17, 10:45 AM, "Kevin Traynor"  wrote:
> 
> rxq_interval was added before there was other #defines
> and code related to rxq intervals.
> 
> Rename to rxq_next_cycles_store in order to make it more intuitive.
> 
> Reported-by: Ilya Maximets 
> Signed-off-by: Kevin Traynor 
> ---
>  lib/dpif-netdev.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 071ec14..55d5656 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -576,5 +576,5 @@ struct dp_netdev_pmd_thread {
>  /* End of the next time interval for which processing cycles
> are stored for each polled rxq. */
> -long long int rxq_interval;
> +long long int rxq_next_cycle_store;
>  
>  /* Statistics. */
> @@ -4507,5 +4507,5 @@ dp_netdev_configure_pmd(struct 
> dp_netdev_pmd_thread *pmd, struct dp_netdev *dp,
>  cmap_init(>classifiers);
>  pmd->next_optimization = time_msec() + 
> DPCLS_OPTIMIZATION_INTERVAL;
> -pmd->rxq_interval = time_msec() + PMD_RXQ_INTERVAL_LEN;
> +pmd->rxq_next_cycle_store = time_msec() + PMD_RXQ_INTERVAL_LEN;
>  hmap_init(>poll_list);
>  hmap_init(>tx_ports);
> @@ -5951,5 +5951,5 @@ dp_netdev_pmd_try_optimize(struct 
> dp_netdev_pmd_thread *pmd,
>  long long int now = time_msec();
>  
> -if (now > pmd->rxq_interval) {
> +if (now > pmd->rxq_next_cycle_store) {
>  /* Get the cycles that were used to process each queue and 
> store. */
>  for (unsigned i = 0; i < poll_cnt; i++) {
> @@ -5961,5 +5961,5 @@ dp_netdev_pmd_try_optimize(struct 
> dp_netdev_pmd_thread *pmd,
>  }
>  /* Start new measuring interval */
> -pmd->rxq_interval = now + PMD_RXQ_INTERVAL_LEN;
> +pmd->rxq_next_cycle_store = now + PMD_RXQ_INTERVAL_LEN;
>  }
>  
> -- 
> 1.8.3.1
> 
> 
> 
> 
> 

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

Re: [ovs-dev] OVS-DPDK public meeting

2017-10-20 Thread Kevin Traynor 
18th October 2017

Attendees: Aaron, Ian, Ciara, Sugesh, Billy, Thomas, Olga, Johann,
Simon, Mark, Zoltan, Kevin (may have missed some)

===
GENERAL
===

- OVS-DPDK branch
- Ian will be collating the DPDK patches on his branch here:
https://github.com/istokes/ovs/tree/dpdk_merge
- Ian will use VSperf to validate before pull requests. For details on
VSperf see https://wiki.opnfv.org/display/vsperf/VSperf+Home

- DPDK summit/OVS con
-- A number of DPDK related talks selected in OVS con
-- DPDK summit CFP results not available yet


PERFORMANCE/FEATURES

- Tx-batching
-- Bhanu reviewing some aspects around may_steal flag

- mempool fixes (Antonio)
-- Close to being ready, final few reviews needed
-- Fixes some broken functionality on head of master around
vhostuserclient, vhost numa awareness, mtu,

- Multiple DPDK devices behind one pci (Ciara/Olga/Kevin)
-- Reported by user with ConnectX-3
-- Will also be required for other NICs
-- Ciara has submitted patch to allow
-- Request to Hw vendors who need this to review


HARDWARE OFFLOAD


- rte_flow offload
-- Yuanhan is looking at some updates around the installation of flows
and threading
-- Question from about Simon about whether a single thread is enough for
flow installations

On 07/25/2017 02:25 PM, Kevin Traynor wrote:
> Hi All,
> 
> The OVS-DPDK public meetings have moved to Wednesday's at the same time.
> Everyone is welcome, it's a chance to share status/plans etc.
> 
> It's scheduled for every 2 weeks starting 26th July. Meeting time is
> currently @ 4pm UTC.
> 
> You can connect through Bluejeans or through dial in:
> 
> https://bluejeans.com/139318596
> 
> US: +1.408.740.7256 
> UK: +44.203.608.5256 
> Germany: +49.32.221.091256 
> Ireland: +353.1.697.1256 
> Other numbers at https://www.bluejeans.com/numbers
> Meeting ID: 139318596
> 
> thanks,
> Kevin.
> 

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

Re: [ovs-dev] [PATCH v8 1/6] netdev-dpdk: fix management of pre-existing mempools.

2017-10-20 Thread Kavanagh, Mark B 
>From: Fischetti, Antonio
>Sent: Thursday, October 19, 2017 5:54 PM
>To: d...@openvswitch.org
>Cc: Kavanagh, Mark B ; Aaron Conole
>; Darrell Ball ; Fischetti, Antonio
>
>Subject: [PATCH v8 1/6] netdev-dpdk: fix management of pre-existing mempools.
>
>Fix an issue on reconfiguration of pre-existing mempools.
>This patch avoids to call dpdk_mp_put() - and erroneously
>release the mempool - when it already exists.
>
>CC: Mark B Kavanagh 
>CC: Aaron Conole 
>CC: Darrell Ball 
>Acked-by: Kevin Traynor 
>Reported-by: Ciara Loftus 
>Tested-by: Ciara Loftus 
>Reported-by: Róbert Mulik 
>Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
>port.")
>Signed-off-by: Antonio Fischetti 

LGTM - thanks for making the suggested mods!

Acked-by: Mark Kavanagh 


>---
>I've tested this patch by
>  - changing at run-time the number of Rx queues:
>  ovs-vsctl set Interface dpdk0 type=dpdk options:n_rxq=4
>
>  - reducing the MTU of the dpdk ports of 1 byte to force
>the configuration of an existing mempool:
>  ovs-vsctl set Interface dpdk0 mtu_request=1499
>
>This issue was observed in a PVP test topology with dpdkvhostuserclient
>ports. It can happen also with dpdk type ports, eg by reducing the MTU
>of 1 byte.
>
>To replicate the bug scenario in the PVP case it's sufficient to
>set 1 dpdkvhostuserclient port, and just boot the VM.
>
>Below some more details on my own test setup.
>
> PVP test setup
> --
>CLIENT_SOCK_DIR=/tmp
>SOCK0=dpdkvhostuser0
>SOCK1=dpdkvhostuser1
>
>1 PMD
>Add 2 dpdk ports, n_rxq=1
>Add 2 vhu ports both of type dpdkvhostuserclient and specify vhost-server-path
> ovs-vsctl set Interface dpdkvhostuser0 options:vhost-server-
>path="$CLIENT_SOCK_DIR/$SOCK0"
> ovs-vsctl set Interface dpdkvhostuser1 options:vhost-server-
>path="$CLIENT_SOCK_DIR/$SOCK1"
>
>Set port-based rules: dpdk0 <--> vhu0 and dpdk1 <--> vhu1
> add-flow br0 in_port=1,action=output:3
> add-flow br0 in_port=3,action=output:1
> add-flow br0 in_port=4,action=output:2
> add-flow br0 in_port=2,action=output:4
>
> Launch QEMU
> ---
>As OvS vhu ports are acting as clients, we must specify 'server' in the next
>command.
>VM_IMAGE=
>
> sudo -E taskset 0x3F00 $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -name us-
>vhost-vm1 -cpu host -enable-kvm -m 4096M -object memory-backend-
>file,id=mem,size=4096M,mem-path=/dev/hugepages,share=on -numa node,memdev=mem
>-mem-prealloc -smp 4 -drive file=$VM_IMAGE -chardev
>socket,id=char0,path=$CLIENT_SOCK_DIR/$SOCK0,server -netdev type=vhost-
>user,id=mynet1,chardev=char0,vhostforce -device virtio-net-
>pci,mac=00:00:00:00:00:01,netdev=mynet1,mrg_rxbuf=off -chardev
>socket,id=char1,path=$CLIENT_SOCK_DIR/$SOCK1,server -netdev type=vhost-
>user,id=mynet2,chardev=char1,vhostforce -device virtio-net-
>pci,mac=00:00:00:00:00:02,netdev=mynet2,mrg_rxbuf=off --nographic
>
> Expected behavior
> -
>With this fix OvS shouldn't crash.
>---
> lib/netdev-dpdk.c | 44 
> 1 file changed, 28 insertions(+), 16 deletions(-)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index c60f46f..45a81f2 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -508,12 +508,13 @@ dpdk_mp_name(struct dpdk_mp *dmp)
> }
>
> static struct dpdk_mp *
>-dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
>+dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool *mp_exists)
> {
> struct dpdk_mp *dmp = dpdk_rte_mzalloc(sizeof *dmp);
> if (!dmp) {
> return NULL;
> }
>+*mp_exists = false;
> dmp->socket_id = dev->requested_socket_id;
> dmp->mtu = mtu;
> ovs_strzcpy(dmp->if_name, dev->up.name, IFNAMSIZ);
>@@ -530,8 +531,6 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
> + MIN(RTE_MAX_LCORE, dev->requested_n_rxq) * NETDEV_MAX_BURST
> + MIN_NB_MBUF;
>
>-bool mp_exists = false;
>-
> do {
> char *mp_name = dpdk_mp_name(dmp);
>
>@@ -559,7 +558,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
> /* As the mempool create returned EEXIST we can expect the
>  * lookup has returned a valid pointer.  If for some reason
>  * that's not the case we keep track of it. */
>-mp_exists = true;
>+*mp_exists = true;
> } else {
> VLOG_ERR("Failed mempool \"%s\" create request of %u mbufs",
>  mp_name, dmp->mp_size);
>@@ -573,20 +572,23 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
> rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
> return dmp;
> }
>-} while (!mp_exists &&
>+} while (!(*mp_exists) &&
>

Re: [ovs-dev] [PATCH v8 6/6] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.

2017-10-20 Thread Fischetti, Antonio 


> -Original Message-
> From: Kavanagh, Mark B
> Sent: Friday, October 20, 2017 9:44 AM
> To: Fischetti, Antonio ; d...@openvswitch.org
> Cc: Darrell Ball ; Loftus, Ciara ;
> Kevin Traynor ; Aaron Conole 
> Subject: RE: [PATCH v8 6/6] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.
> 
> >From: Fischetti, Antonio
> >Sent: Thursday, October 19, 2017 5:54 PM
> >To: d...@openvswitch.org
> >Cc: Kavanagh, Mark B ; Darrell Ball
> >; Loftus, Ciara ; Kevin Traynor
> >; Aaron Conole ; Fischetti, Antonio
> >
> >Subject: [PATCH v8 6/6] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.
> >
> >For readability purposes dpdk_mp_put is renamed as dpdk_mp_free.
> >
> >CC: Mark B Kavanagh 
> >CC: Darrell Ball 
> >CC: Ciara Loftus 
> >CC: Kevin Traynor 
> >CC: Aaron Conole 
> >Signed-off-by: Antonio Fischetti 
> >---
> 
> 
> Hi Antonio,
> 
> I already Acked this patch in v6 (it was patch 4/5 of that series) - in 
> future,
> please carry forward Acks ;)

[Antonio] My bad, sorry for that.

> 
> Acked-by: Mark Kavanagh 
> 
> Cheers,
> Mark
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name creation.

2017-10-20 Thread Kavanagh, Mark B 
>From: Fischetti, Antonio
>Sent: Friday, October 20, 2017 10:38 AM
>To: Kavanagh, Mark B ; d...@openvswitch.org
>Cc: Darrell Ball ; Loftus, Ciara ;
>Kevin Traynor ; Aaron Conole 
>Subject: RE: [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name
>creation.
>
>
>
>> -Original Message-
>> From: Kavanagh, Mark B
>> Sent: Friday, October 20, 2017 10:28 AM
>> To: Fischetti, Antonio ; d...@openvswitch.org
>> Cc: Darrell Ball ; Loftus, Ciara ;
>> Kevin Traynor ; Aaron Conole 
>> Subject: RE: [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name
>> creation.
>>
>> >From: Fischetti, Antonio
>> >Sent: Thursday, October 19, 2017 5:54 PM
>> >To: d...@openvswitch.org
>> >Cc: Kavanagh, Mark B ; Darrell Ball
>> >; Loftus, Ciara ; Kevin Traynor
>> >; Aaron Conole ; Fischetti,
>Antonio
>> >
>> >Subject: [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name
>creation.
>> >
>> >In case a mempool name could not be generated log a message
>> >and return a null mempool pointer to the caller.
>> >
>> >CC: Mark B Kavanagh 
>> >CC: Darrell Ball 
>> >CC: Ciara Loftus 
>> >CC: Kevin Traynor 
>> >CC: Aaron Conole 
>> >Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
>> >port.")
>> >Signed-off-by: Antonio Fischetti 
>> >---
>> > lib/netdev-dpdk.c | 7 +++
>> > 1 file changed, 7 insertions(+)
>> >
>> >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> >index dc1e9c3..6fc6e1b 100644
>> >--- a/lib/netdev-dpdk.c
>> >+++ b/lib/netdev-dpdk.c
>> >@@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp)
>> > int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
>> >h, dmp->socket_id, dmp->mtu, dmp->mp_size);
>> > if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
>> >+VLOG_DBG("snprintf returned %d. Failed to generate a mempool "
>> >+"name for \"%s\". Hash:0x%x, mtu:%d, mbufs:%u.",
>> >+ret, dmp->if_name, h, dmp->mtu, dmp->mp_size);
>> > return NULL;
>> > }
>> > return mp_name;
>> >@@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
>> >*mp_exists)
>> >
>> > do {
>> > char *mp_name = dpdk_mp_name(dmp);
>> >+if (!mp_name) {
>> >+rte_free(dmp);
>> >+return NULL;
>> >+}
>>
>>
>> This is a fix, and as such, needs to include 'Fixes: " in the
>commit
>> message.
>>
>> I believe that Kevin made the same comment on this patch in v7.
>
>[Antonio] Actually I added
>   Fixes: d555d9bded5f ("netdev-dpdk: Create separate...
>right before the Signed-off-by line.
>Is that what you meant?

Perfect - I actually hadn't spotted that.

With that: Acked-by: Mark Kavanagh 

Now, it seems that I really do need to go get that coffee...  ;)

>
>
>>
>> Thanks,
>> Mark
>>
>> >
>> > VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
>> >   "on socket %d for %d Rx and %d Tx queues.",
>> >--
>> >2.4.11

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

Re: [ovs-dev] [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name creation.

2017-10-20 Thread Fischetti, Antonio 


> -Original Message-
> From: Kavanagh, Mark B
> Sent: Friday, October 20, 2017 10:28 AM
> To: Fischetti, Antonio ; d...@openvswitch.org
> Cc: Darrell Ball ; Loftus, Ciara ;
> Kevin Traynor ; Aaron Conole 
> Subject: RE: [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name
> creation.
> 
> >From: Fischetti, Antonio
> >Sent: Thursday, October 19, 2017 5:54 PM
> >To: d...@openvswitch.org
> >Cc: Kavanagh, Mark B ; Darrell Ball
> >; Loftus, Ciara ; Kevin Traynor
> >; Aaron Conole ; Fischetti, Antonio
> >
> >Subject: [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name creation.
> >
> >In case a mempool name could not be generated log a message
> >and return a null mempool pointer to the caller.
> >
> >CC: Mark B Kavanagh 
> >CC: Darrell Ball 
> >CC: Ciara Loftus 
> >CC: Kevin Traynor 
> >CC: Aaron Conole 
> >Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
> >port.")
> >Signed-off-by: Antonio Fischetti 
> >---
> > lib/netdev-dpdk.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >index dc1e9c3..6fc6e1b 100644
> >--- a/lib/netdev-dpdk.c
> >+++ b/lib/netdev-dpdk.c
> >@@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp)
> > int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
> >h, dmp->socket_id, dmp->mtu, dmp->mp_size);
> > if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
> >+VLOG_DBG("snprintf returned %d. Failed to generate a mempool "
> >+"name for \"%s\". Hash:0x%x, mtu:%d, mbufs:%u.",
> >+ret, dmp->if_name, h, dmp->mtu, dmp->mp_size);
> > return NULL;
> > }
> > return mp_name;
> >@@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
> >*mp_exists)
> >
> > do {
> > char *mp_name = dpdk_mp_name(dmp);
> >+if (!mp_name) {
> >+rte_free(dmp);
> >+return NULL;
> >+}
> 
> 
> This is a fix, and as such, needs to include 'Fixes: " in the 
> commit
> message.
> 
> I believe that Kevin made the same comment on this patch in v7.

[Antonio] Actually I added 
   Fixes: d555d9bded5f ("netdev-dpdk: Create separate...
right before the Signed-off-by line.
Is that what you meant?


> 
> Thanks,
> Mark
> 
> >
> > VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
> >   "on socket %d for %d Rx and %d Tx queues.",
> >--
> >2.4.11

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

Re: [ovs-dev] [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name creation.

2017-10-20 Thread Kavanagh, Mark B 
>From: Fischetti, Antonio
>Sent: Thursday, October 19, 2017 5:54 PM
>To: d...@openvswitch.org
>Cc: Kavanagh, Mark B ; Darrell Ball
>; Loftus, Ciara ; Kevin Traynor
>; Aaron Conole ; Fischetti, Antonio
>
>Subject: [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name creation.
>
>In case a mempool name could not be generated log a message
>and return a null mempool pointer to the caller.
>
>CC: Mark B Kavanagh 
>CC: Darrell Ball 
>CC: Ciara Loftus 
>CC: Kevin Traynor 
>CC: Aaron Conole 
>Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
>port.")
>Signed-off-by: Antonio Fischetti 
>---
> lib/netdev-dpdk.c | 7 +++
> 1 file changed, 7 insertions(+)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index dc1e9c3..6fc6e1b 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp)
> int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
>h, dmp->socket_id, dmp->mtu, dmp->mp_size);
> if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
>+VLOG_DBG("snprintf returned %d. Failed to generate a mempool "
>+"name for \"%s\". Hash:0x%x, mtu:%d, mbufs:%u.",
>+ret, dmp->if_name, h, dmp->mtu, dmp->mp_size);
> return NULL;
> }
> return mp_name;
>@@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
>*mp_exists)
>
> do {
> char *mp_name = dpdk_mp_name(dmp);
>+if (!mp_name) {
>+rte_free(dmp);
>+return NULL;
>+}


This is a fix, and as such, needs to include 'Fixes: " in the commit 
message.

I believe that Kevin made the same comment on this patch in v7.

Thanks,
Mark
 
>
> VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
>   "on socket %d for %d Rx and %d Tx queues.",
>--
>2.4.11

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

Re: [ovs-dev] [PATCH RFC v2] netdev-dpdk: Allow specification of index for PCI devices

2017-10-20 Thread Kevin Traynor 
On 10/19/2017 07:27 PM, Loftus, Ciara wrote:
>>
>> On 10/17/2017 11:48 AM, Ciara Loftus wrote:
>>> Some NICs have only one PCI address associated with multiple ports. This
>>> patch extends the dpdk-devargs option's format to cater for such
>>> devices. Whereas before only one of N ports associated with one PCI
>>> address could be added, now N can.
>>>
>>> This patch allows for the following use of the dpdk-devargs option:
>>>
>>> ovs-vsctl set Interface myport options:dpdk-devargs=:06:00.0,X
>>>
>>> Where X is an unsigned integer representing one of multiple ports
>>> associated with the PCI address provided.
>>>
>>> This patch has not been tested.
>>>
>>> Signed-off-by: Ciara Loftus 
>>> ---
>>> v2:
>>> * Simplify function to find port ID of indexed device
>>> * Hotplug compatibility
>>> * Detach compatibility
>>> * Documentation
>>> * Use strtol instead of atoi
>>>
>>>  Documentation/howto/dpdk.rst |  9 +
>>>  Documentation/intro/install/dpdk.rst |  9 +
>>>  NEWS |  2 ++
>>>  lib/netdev-dpdk.c| 67 ++---
>> ---
>>>  vswitchd/vswitch.xml | 11 --
>>>  5 files changed, 85 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/Documentation/howto/dpdk.rst
>> b/Documentation/howto/dpdk.rst
>>> index d123819..9447b71 100644
>>> --- a/Documentation/howto/dpdk.rst
>>> +++ b/Documentation/howto/dpdk.rst
>>> @@ -48,6 +48,15 @@ number of dpdk devices found in the log file::
>>>  $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \
>>>  options:dpdk-devargs=:01:00.1
>>>
>>> +If your PCI device has multiple ports under the same PCI ID, you can use
>> the
>>> +following notation to indicate the specific device you wish to add::
>>> +
>>> +$ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
>>> +options:dpdk-devargs=:01:00.0,0
>>> +
>>> +The above would attempt to use the first device (0) associated with that
>> PCI
>>> +ID. Use ,1 ,2 etc. to access the next.
>>> +
>>>  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::
>>> diff --git a/Documentation/intro/install/dpdk.rst
>> b/Documentation/intro/install/dpdk.rst
>>> index bb69ae5..d0e87f5 100644
>>> --- a/Documentation/intro/install/dpdk.rst
>>> +++ b/Documentation/intro/install/dpdk.rst
>>> @@ -271,6 +271,15 @@ ports. For example, to create a userspace bridge
>> named ``br0`` and add two
>>>  DPDK devices will not be available for use until a valid dpdk-devargs is
>>>  specified.
>>>
>>> +If your PCI device has multiple ports under the same PCI ID, you can use
>> the
>>> +following notation to indicate the specific device you wish to add::
>>> +
>>> +$ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
>>> +options:dpdk-devargs=:01:00.0,0
>>> +
>>> +The above would attempt to use the first device (0) associated with that
>> PCI
>>> +ID. Use ,1 ,2 etc. to access the next.
>>> +
>>>  Refer to ovs-vsctl(8) and :doc:`/howto/dpdk` for more details.
>>>
>>>  Performance Tuning
>>> diff --git a/NEWS b/NEWS
>>> index 75f5fa5..ca885a6 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -5,6 +5,8 @@ Post-v2.8.0
>>> chassis "hostname" in addition to a chassis "name".
>>> - Linux kernel 4.13
>>>   * Add support for compiling OVS with the latest Linux 4.13 kernel
>>> +   - DPDK:
>>> + * Support for adding devices with multiple DPDK ports under one PCI
>> ID.
>>>
>>>  v2.8.0 - 31 Aug 2017
>>> - ovs-ofctl:
>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>> index c60f46f..35e15da 100644
>>> --- a/lib/netdev-dpdk.c
>>> +++ b/lib/netdev-dpdk.c
>>> @@ -1214,16 +1214,40 @@
>> netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id)
>>>  }
>>>
>>>  static dpdk_port_t
>>> +dpdk_get_port_by_name_with_index(char *name, int idx, int base_id)
>>> +{
>>> +struct rte_eth_dev_info dev_info;
>>> +char pci_addr[PCI_PRI_STR_SIZE];
>>> +dpdk_port_t offset_port_id = base_id + idx;
>>> +
>>> +if (rte_eth_dev_is_valid_port(offset_port_id)) {
>>> +rte_eth_dev_info_get(offset_port_id, _info);
>>> +rte_pci_device_name(_info.pci_dev->addr, pci_addr,
>>> +sizeof(pci_addr));
>>> +if (!strcmp(pci_addr, name)) {
>>> +return offset_port_id;
>>> +}
>>> +}
>>> +
>>> +VLOG_INFO("Invalid PCI offset %i for device %s", idx, name);
>>> +
>>> +return DPDK_ETH_PORT_ID_INVALID;
>>> +}
>>> +
>>> +static dpdk_port_t
>>>  netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
>>>  const char *devargs, char **errp)
>>>  {
>>> -/* Get the name up to the first comma. */
>>> -char *name = xmemdup0(devargs, strcspn(devargs, ","));
>>> +char *devargs_copy = xmemdup0((devargs), strlen(devargs));
>>> +

Re: [ovs-dev] [PATCH v8 2/6] Fix mempool names to reflect socket id.

2017-10-20 Thread Kavanagh, Mark B 

>From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-boun...@openvswitch.org]
>On Behalf Of Kavanagh, Mark B
>Sent: Friday, October 20, 2017 9:40 AM
>To: Fischetti, Antonio ; d...@openvswitch.org
>Subject: Re: [ovs-dev] [PATCH v8 2/6] Fix mempool names to reflect socket id.
>
>>From: Fischetti, Antonio
>>Sent: Thursday, October 19, 2017 5:54 PM
>>To: d...@openvswitch.org
>>Cc: Kavanagh, Mark B ; Aaron Conole
>>; Fischetti, Antonio 
>>Subject: [PATCH v8 2/6] Fix mempool names to reflect socket id.
>>
>>Create mempool names by considering also the NUMA socket number.
>>So a name reflects what socket the mempool is allocated on.
>>This change is needed for the NUMA-awareness feature.
>>
>>CC: Mark B Kavanagh 
>>CC: Aaron Conole 
>>Acked-by: Kevin Traynor 
>>Reported-by: Ciara Loftus 
>>Tested-by: Ciara Loftus 
>>Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
>>port.")
>>Signed-off-by: Antonio Fischetti 
>
>LGTM -  Signed-off-by: Mark Kavanagh 

s/Signed-off/Acked/

Haven't had coffee yet...

>
>
>>---
>>Mempool names now contains the requested socket id and become like:
>>"ovs_4adb057e_1_2030_20512".
>>
>>Tested with DPDK 17.05.2 (from dpdk-stable branch).
>>NUMA-awareness feature enabled (DPDK/config/common_base).
>>
>>Created 1 single dpdkvhostuser port type.
>>OvS pmd-cpu-mask=FF3 # enable cores on both numa nodes
>>QEMU core mask = 0xFC000 # cores for qemu on numa node 1 only
>>
>> Before launching the VM:
>> 
>>ovs-appctl dpif-netdev/pmd-rxq-show
>>shows core #1 is serving the vhu port.
>>
>>pmd thread numa_id 0 core_id 1:
>>isolated : false
>>port: dpdkvhostuser0queue-id: 0
>>
>> After launching the VM:
>> ---
>>the vhu port is now managed by core #27
>>pmd thread numa_id 1 core_id 27:
>>isolated : false
>>port: dpdkvhostuser0queue-id: 0
>>
>>and the log shows a new mempool is allocated on NUMA node 1, while
>>the previous one is deleted:
>>
>>2017-10-06T14:04:55Z|00105|netdev_dpdk|DBG|Allocated
>>"ovs_4adb057e_1_2030_20512" mempool with 20512 mbufs
>>2017-10-06T14:04:55Z|00106|netdev_dpdk|DBG|Releasing
>>"ovs_4adb057e_0_2030_20512" mempool
>>---
>> lib/netdev-dpdk.c | 13 +++--
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>index 45a81f2..7e95f36 100644
>>--- a/lib/netdev-dpdk.c
>>+++ b/lib/netdev-dpdk.c
>>@@ -499,8 +499,8 @@ dpdk_mp_name(struct dpdk_mp *dmp)
>> {
>> uint32_t h = hash_string(dmp->if_name, 0);
>> char *mp_name = xcalloc(RTE_MEMPOOL_NAMESIZE, sizeof *mp_name);
>>-int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%u",
>>-   h, dmp->mtu, dmp->mp_size);
>>+int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
>>+   h, dmp->socket_id, dmp->mtu, dmp->mp_size);
>> if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
>> return NULL;
>> }
>>@@ -534,10 +534,11 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
>>*mp_exists)
>> do {
>> char *mp_name = dpdk_mp_name(dmp);
>>
>>-VLOG_DBG("Requesting a mempool of %u mbufs for netdev %s "
>>- "with %d Rx and %d Tx queues.",
>>- dmp->mp_size, dev->up.name,
>>- dev->requested_n_rxq, dev->requested_n_txq);
>>+VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
>>+  "on socket %d for %d Rx and %d Tx queues.",
>>+  dev->up.name, dmp->mp_size,
>>+  dev->requested_socket_id,
>>+  dev->requested_n_rxq, dev->requested_n_txq);
>>
>> dmp->mp = rte_pktmbuf_pool_create(mp_name, dmp->mp_size,
>>   MP_CACHE_SZ,
>>--
>>2.4.11
>
>___
>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 v8 0/6] netdev-dpdk: Fix mempool management and other cleanup.

2017-10-20 Thread Kevin Traynor 
On 10/19/2017 07:00 PM, Fischetti, Antonio wrote:
> 
> 
>> -Original Message-
>> From: Stokes, Ian
>> Sent: Thursday, October 19, 2017 6:22 PM
>> To: Fischetti, Antonio ; d...@openvswitch.org
>> Subject: RE: [ovs-dev] [PATCH v8 0/6] netdev-dpdk: Fix mempool management and
>> other cleanup.
>>
>>> Patch #1, #2 and #4 contain the fixes.
> 
> [Antonio] Further to previous line: patches #1 and #2 do fix the issues we 
> saw: 
>  - issue with vhostuserclient in a PVP test
>  - issue of new MTU not displayed when an existing mempool is returned.
>  - issue with the NUMA-Aware usecase 
> 
> All other patches #3, #4, #5 and #6 are small improvements or just clean-up 
> that we 
> could even skip at all.
> Actually patch #4 is not just a clean-up, it's managing an unlikely event 
> that 
> 'might' happen (I've never seen it), that's why I promoted it as a 'fix' in 
> the
> line above.
> 
> So just the first 2 patches are needed to fix the mempool issues.
> 

Agreed. Let's not hold these 2 up if there are more comments/changes
needed on the other patches, as these 2 fix commonly used functionality
that is broken.

thanks,
Kevin

> 
>>> All other patches in this series are a clean up for code readability or
>>> small improvements.
>>>
>>
>> Hi Antonio, if the fixes are in patches 1 2 and 4 is there a reason they have
>> not been grouped in a patchset and patch 1,2,3?
>>
>> The other patches could be applied separately afterwards?
> 
> [Antonio] Patches #1 and #2 are the ones needed to fix the issues.
> All other patches are clean-up/small improvement, that could be optionally
> applied. I think it's better to apply them in the given order.
> 
> 
>>
>>> List of versions:
>>>  - v8:
>>>- Debug message rephrased in patch #2.
>>>- Reworked patch #4 for snprintf error code.
>>>- Comments in patch #6 moved into patch #1.
>>>
>>>  - v7:
>>>- Restored 2 separate patches for the 2 fixes.
>>>- patch #1: detect when previous mempools must be released.
>>>- patch #2: mempool name generation for NUMA-awareness test case.
>>>- patch "netdev-dpdk: manage empty mempool names." renamed as
>>>  "netdev-dpdk: manage failure in mempool name creation."
>>>- Various rework based on comments.
>>>
>>>  - v6:
>>>- patches #1 and #2 squashed into one.
>>>- Reworked to consider the latest comments.
>>>- tested the release of pre-existing mempools (reported by Ciara L.)
>>>- tested the change of MTU when an existing mempool is returned
>>>  (reported by Robert M.)
>>>- tested the NUMA-Awareness usecase (reported by Ciara L.)
>>>  - v5: manage new MTU value when a pre-existing mempool is returned.
>>>  - v4: fix NUMA awareness usecase
>>>  - v3: avoid deletion of pre-existing mempools
>>>  - v2: rework to accomodate code changes for dpdk ports too
>>>  - v1: 1st implementation.
>>>
>>> Fischetti, Antonio (6):
>>>   netdev-dpdk: fix management of pre-existing mempools.
>>>   Fix mempool names to reflect socket id.
>>>   netdev-dpdk: skip init for existing mempools.
>>>   netdev-dpdk: manage failure in mempool name creation.
>>>   netdev-dpdk: Reword mp_size as n_mbufs.
>>>   netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.
>>>
>>>  lib/netdev-dpdk.c | 96 ++
>>> -
>>>  1 file changed, 59 insertions(+), 37 deletions(-)
>>>
>>> --
>>> 2.4.11
>>>
>>> ___
>>> 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
> 

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

Re: [ovs-dev] [PATCH v8 6/6] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.

2017-10-20 Thread Kevin Traynor 
On 10/19/2017 05:54 PM, antonio.fische...@intel.com wrote:
> For readability purposes dpdk_mp_put is renamed as dpdk_mp_free.
> 
> CC: Mark B Kavanagh 
> CC: Darrell Ball 
> CC: Ciara Loftus 
> CC: Kevin Traynor 
> CC: Aaron Conole 
> Signed-off-by: Antonio Fischetti 
> ---

Acked-by: Kevin Traynor 

>  lib/netdev-dpdk.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index bf143e0..82652f0 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -602,8 +602,9 @@ dpdk_mp_get(struct netdev_dpdk *dev, int mtu, bool 
> *mp_exists)
>  return dmp;
>  }
>  
> +/* Release an existing mempool. */
>  static void
> -dpdk_mp_put(struct dpdk_mp *dmp)
> +dpdk_mp_free(struct dpdk_mp *dmp)
>  {
>  char *mp_name;
>  
> @@ -649,7 +650,8 @@ netdev_dpdk_mempool_configure(struct netdev_dpdk *dev)
>  dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
>  return EEXIST;
>  } else {
> -dpdk_mp_put(dev->dpdk_mp);
> +/* A new mempool was created, release the previous one. */
> +dpdk_mp_free(dev->dpdk_mp);
>  dev->dpdk_mp = mp;
>  dev->mtu = dev->requested_mtu;
>  dev->socket_id = dev->requested_socket_id;
> @@ -1094,7 +1096,7 @@ common_destruct(struct netdev_dpdk *dev)
>  OVS_EXCLUDED(dev->mutex)
>  {
>  rte_free(dev->tx_q);
> -dpdk_mp_put(dev->dpdk_mp);
> +dpdk_mp_free(dev->dpdk_mp);
>  
>  ovs_list_remove(>list_node);
>  free(ovsrcu_get_protected(struct ingress_policer *,
> 

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

Re: [ovs-dev] [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name creation.

2017-10-20 Thread Kevin Traynor 
On 10/19/2017 05:54 PM, antonio.fische...@intel.com wrote:
> In case a mempool name could not be generated log a message
> and return a null mempool pointer to the caller.
> 
> CC: Mark B Kavanagh 
> CC: Darrell Ball 
> CC: Ciara Loftus 
> CC: Kevin Traynor 
> CC: Aaron Conole 
> Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each 
> port.")
> Signed-off-by: Antonio Fischetti 
> ---

Acked-by: Kevin Traynor 

>  lib/netdev-dpdk.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index dc1e9c3..6fc6e1b 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp)
>  int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
> h, dmp->socket_id, dmp->mtu, dmp->mp_size);
>  if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
> +VLOG_DBG("snprintf returned %d. Failed to generate a mempool "
> +"name for \"%s\". Hash:0x%x, mtu:%d, mbufs:%u.",
> +ret, dmp->if_name, h, dmp->mtu, dmp->mp_size);
>  return NULL;
>  }
>  return mp_name;
> @@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
> *mp_exists)
>  
>  do {
>  char *mp_name = dpdk_mp_name(dmp);
> +if (!mp_name) {
> +rte_free(dmp);
> +return NULL;
> +}
>  
>  VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
>"on socket %d for %d Rx and %d Tx queues.",
> 

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

Re: [ovs-dev] [PATCH v8 6/6] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.

2017-10-20 Thread Kavanagh, Mark B 
>From: Fischetti, Antonio
>Sent: Thursday, October 19, 2017 5:54 PM
>To: d...@openvswitch.org
>Cc: Kavanagh, Mark B ; Darrell Ball
>; Loftus, Ciara ; Kevin Traynor
>; Aaron Conole ; Fischetti, Antonio
>
>Subject: [PATCH v8 6/6] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.
>
>For readability purposes dpdk_mp_put is renamed as dpdk_mp_free.
>
>CC: Mark B Kavanagh 
>CC: Darrell Ball 
>CC: Ciara Loftus 
>CC: Kevin Traynor 
>CC: Aaron Conole 
>Signed-off-by: Antonio Fischetti 
>---


Hi Antonio,

I already Acked this patch in v6 (it was patch 4/5 of that series) - in future, 
please carry forward Acks ;)

Acked-by: Mark Kavanagh 

Cheers,
Mark 


> lib/netdev-dpdk.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index bf143e0..82652f0 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -602,8 +602,9 @@ dpdk_mp_get(struct netdev_dpdk *dev, int mtu, bool
>*mp_exists)
> return dmp;
> }
>
>+/* Release an existing mempool. */
> static void
>-dpdk_mp_put(struct dpdk_mp *dmp)
>+dpdk_mp_free(struct dpdk_mp *dmp)
> {
> char *mp_name;
>
>@@ -649,7 +650,8 @@ netdev_dpdk_mempool_configure(struct netdev_dpdk *dev)
> dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
> return EEXIST;
> } else {
>-dpdk_mp_put(dev->dpdk_mp);
>+/* A new mempool was created, release the previous one. */
>+dpdk_mp_free(dev->dpdk_mp);
> dev->dpdk_mp = mp;
> dev->mtu = dev->requested_mtu;
> dev->socket_id = dev->requested_socket_id;
>@@ -1094,7 +1096,7 @@ common_destruct(struct netdev_dpdk *dev)
> OVS_EXCLUDED(dev->mutex)
> {
> rte_free(dev->tx_q);
>-dpdk_mp_put(dev->dpdk_mp);
>+dpdk_mp_free(dev->dpdk_mp);
>
> ovs_list_remove(>list_node);
> free(ovsrcu_get_protected(struct ingress_policer *,
>--
>2.4.11

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

Re: [ovs-dev] [PATCH v8 2/6] Fix mempool names to reflect socket id.

2017-10-20 Thread Kavanagh, Mark B 
>From: Fischetti, Antonio
>Sent: Thursday, October 19, 2017 5:54 PM
>To: d...@openvswitch.org
>Cc: Kavanagh, Mark B ; Aaron Conole
>; Fischetti, Antonio 
>Subject: [PATCH v8 2/6] Fix mempool names to reflect socket id.
>
>Create mempool names by considering also the NUMA socket number.
>So a name reflects what socket the mempool is allocated on.
>This change is needed for the NUMA-awareness feature.
>
>CC: Mark B Kavanagh 
>CC: Aaron Conole 
>Acked-by: Kevin Traynor 
>Reported-by: Ciara Loftus 
>Tested-by: Ciara Loftus 
>Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
>port.")
>Signed-off-by: Antonio Fischetti 

LGTM -  Signed-off-by: Mark Kavanagh 


>---
>Mempool names now contains the requested socket id and become like:
>"ovs_4adb057e_1_2030_20512".
>
>Tested with DPDK 17.05.2 (from dpdk-stable branch).
>NUMA-awareness feature enabled (DPDK/config/common_base).
>
>Created 1 single dpdkvhostuser port type.
>OvS pmd-cpu-mask=FF3 # enable cores on both numa nodes
>QEMU core mask = 0xFC000 # cores for qemu on numa node 1 only
>
> Before launching the VM:
> 
>ovs-appctl dpif-netdev/pmd-rxq-show
>shows core #1 is serving the vhu port.
>
>pmd thread numa_id 0 core_id 1:
>isolated : false
>port: dpdkvhostuser0queue-id: 0
>
> After launching the VM:
> ---
>the vhu port is now managed by core #27
>pmd thread numa_id 1 core_id 27:
>isolated : false
>port: dpdkvhostuser0queue-id: 0
>
>and the log shows a new mempool is allocated on NUMA node 1, while
>the previous one is deleted:
>
>2017-10-06T14:04:55Z|00105|netdev_dpdk|DBG|Allocated
>"ovs_4adb057e_1_2030_20512" mempool with 20512 mbufs
>2017-10-06T14:04:55Z|00106|netdev_dpdk|DBG|Releasing
>"ovs_4adb057e_0_2030_20512" mempool
>---
> lib/netdev-dpdk.c | 13 +++--
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index 45a81f2..7e95f36 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -499,8 +499,8 @@ dpdk_mp_name(struct dpdk_mp *dmp)
> {
> uint32_t h = hash_string(dmp->if_name, 0);
> char *mp_name = xcalloc(RTE_MEMPOOL_NAMESIZE, sizeof *mp_name);
>-int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%u",
>-   h, dmp->mtu, dmp->mp_size);
>+int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
>+   h, dmp->socket_id, dmp->mtu, dmp->mp_size);
> if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
> return NULL;
> }
>@@ -534,10 +534,11 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
>*mp_exists)
> do {
> char *mp_name = dpdk_mp_name(dmp);
>
>-VLOG_DBG("Requesting a mempool of %u mbufs for netdev %s "
>- "with %d Rx and %d Tx queues.",
>- dmp->mp_size, dev->up.name,
>- dev->requested_n_rxq, dev->requested_n_txq);
>+VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
>+  "on socket %d for %d Rx and %d Tx queues.",
>+  dev->up.name, dmp->mp_size,
>+  dev->requested_socket_id,
>+  dev->requested_n_rxq, dev->requested_n_txq);
>
> dmp->mp = rte_pktmbuf_pool_create(mp_name, dmp->mp_size,
>   MP_CACHE_SZ,
>--
>2.4.11

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

[ovs-dev] Adding new action

2017-10-20 Thread Prabhu 

Hello all,
I want to implement a custom action which should handle the below 
process defined as FLOW actions:


Buffer size : 1000

Table:0
Rule 1 ->  IN_PORT  match MAC Address1 and ACTION:send_to_buffer_0, 
go_to_table1 (buffer should log its backlog)
Rule 2 ->  IN_PORT match MAC Address2 and ACTION:send_to_buffer_1, 
go_to_table1 (buffer should log its backlog)


Table:1
Rule 1-> match MAC Address1 and ACTION: output:1
Rule 2-> match MAC Address2 and ACTION: output:2

Buffer 0 and Buffer 1 can have the same buffer size 1000 packets (e.g)

*The FAQ on http://docs.openvswitch.org/en/latest/faq/contributing/ says:*

Q: How do I add support for a new OpenFlow action?

   A: Add your new action to |enum ofp_raw_action_type| in
   |lib/ofp-actions.c|, following the existing pattern. Then recompile
   and fix all of the new warnings, implementing new functionality for
   the new action as needed. (If you configure with |--enable-Werror|,
   as described in the Open vSwitch on Linux, FreeBSD and NetBSD
   , then
   it is impossible to miss any warnings.)

   If you need to add an OpenFlow vendor extension action for a vendor
   that doesn’t yet have any extension actions, then you will also need
   to edit |build-aux/extract-ofp-actions|.

In order to implement new experimental action just following the above 
step should work or should be it has be done in more detailed way ?


The feature I need is not defined in Openflow specifications and I am 
looking to test my case in Mininet + Openvswitch-2.7.0







- Prabhu -


   *
   *

   *
   *

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

Re: [ovs-dev] [PATCH net-next v12] openvswitch: enable NSH support

2017-10-20 Thread Jiri Benc 
On Fri, 20 Oct 2017 05:53:12 +0800, Yang, Yi wrote:
> For push_nsh, my typical use scinario is push_nsh then set then output
> to vxlangpe port.

Okay.

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