Re: [ovs-dev] [PATCH net-next v8] net: openvswitch: IPv6: Add IPv6 extension header support

[Pravin Shelar]

()

On Mon, Dec 6, 2021 at 3:00 PM Cpp Code  wrote:
>
> On Thu, Dec 2, 2021 at 9:28 PM Pravin Shelar  wrote:
> >
> > On Thu, Dec 2, 2021 at 12:20 PM Cpp Code  wrote:
> > >
> > > On Wed, Dec 1, 2021 at 11:34 PM Pravin Shelar  
> > > wrote:
> > > >
> > > > On Wed, Nov 24, 2021 at 11:33 AM Toms Atteka  
> > > > wrote:
> > > > >
> > > > > This change adds a new OpenFlow field OFPXMT_OFB_IPV6_EXTHDR and
> > > > > packets can be filtered using ipv6_ext flag.
> > > > >
> > > > > Signed-off-by: Toms Atteka 
> > > > > ---
> > > > >  include/uapi/linux/openvswitch.h |   6 ++
> > > > >  net/openvswitch/flow.c   | 140 
> > > > > +++
> > > > >  net/openvswitch/flow.h   |  14 
> > > > >  net/openvswitch/flow_netlink.c   |  26 +-
> > > > >  4 files changed, 184 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/include/uapi/linux/openvswitch.h 
> > > > > b/include/uapi/linux/openvswitch.h
> > > > > index a87b44cd5590..43790f07e4a2 100644
> > > > > --- a/include/uapi/linux/openvswitch.h
> > > > > +++ b/include/uapi/linux/openvswitch.h
> > > > > @@ -342,6 +342,7 @@ enum ovs_key_attr {
> > > > > OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4,   /* struct 
> > > > > ovs_key_ct_tuple_ipv4 */
> > > > > OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6,   /* struct 
> > > > > ovs_key_ct_tuple_ipv6 */
> > > > > OVS_KEY_ATTR_NSH,   /* Nested set of ovs_nsh_key_* */
> > > > > +   OVS_KEY_ATTR_IPV6_EXTHDRS,  /* struct ovs_key_ipv6_exthdr */
> > > > >
> > > > >  #ifdef __KERNEL__
> > > > > OVS_KEY_ATTR_TUNNEL_INFO,  /* struct ip_tunnel_info */
> > > > > @@ -421,6 +422,11 @@ struct ovs_key_ipv6 {
> > > > > __u8   ipv6_frag;   /* One of OVS_FRAG_TYPE_*. */
> > > > >  };
> > > > >
> > > > > +/* separate structure to support backward compatibility with older 
> > > > > user space */
> > > > > +struct ovs_key_ipv6_exthdrs {
> > > > > +   __u16  hdrs;
> > > > > +};
> > > > > +
> > > > >  struct ovs_key_tcp {
> > > > > __be16 tcp_src;
> > > > > __be16 tcp_dst;
> > > > > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> > > > > index 9d375e74b607..28acb40437ca 100644
> > > > > --- a/net/openvswitch/flow.c
> > > > > +++ b/net/openvswitch/flow.c
> > > > > @@ -239,6 +239,144 @@ static bool icmphdr_ok(struct sk_buff *skb)
> > > > >   sizeof(struct icmphdr));
> > > > >  }
> > > > >
> > > > > +/**
> > > > > + * get_ipv6_ext_hdrs() - Parses packet and sets IPv6 extension 
> > > > > header flags.
> > > > > + *
> > > > > + * @skb: buffer where extension header data starts in packet
> > > > > + * @nh: ipv6 header
> > > > > + * @ext_hdrs: flags are stored here
> > > > > + *
> > > > > + * OFPIEH12_UNREP is set if more than one of a given IPv6 extension 
> > > > > header
> > > > > + * is unexpectedly encountered. (Two destination options headers may 
> > > > > be
> > > > > + * expected and would not cause this bit to be set.)
> > > > > + *
> > > > > + * OFPIEH12_UNSEQ is set if IPv6 extension headers were not in the 
> > > > > order
> > > > > + * preferred (but not required) by RFC 2460:
> > > > > + *
> > > > > + * When more than one extension header is used in the same packet, 
> > > > > it is
> > > > > + * recommended that those headers appear in the following order:
> > > > > + *  IPv6 header
> > > > > + *  Hop-by-Hop Options header
> > > > > + *  Destination Options header
> > > > > + *  Routing header
> > > > > + *  Fragment header
> > > > > + *  Authentication header
> > > > > + *  Encapsulating Security Payload header
> > > > > + *  Destination Options header
> > > > > + *  upper-layer header
> > > > > + */
> > > > > +static void get_ipv6_ext_hdrs(struct sk_buff *skb, struct ipv6hdr 
> > > > > *nh,
> > > > > + u16 *ext_hdrs)
> > > > > +{
> > > > > +   u8 next_type = nh->nexthdr;
> > > > > +   unsigned int start = skb_network_offset(skb) + sizeof(struct 
> > > > > ipv6hdr);
> > > > > +   int dest_options_header_count = 0;
> > > > > +
> > > > > +   *ext_hdrs = 0;
> > > > > +
> > > > > +   while (ipv6_ext_hdr(next_type)) {
> > > > > +   struct ipv6_opt_hdr _hdr, *hp;
> > > > > +
> > > > > +   switch (next_type) {
> > > > > +   case IPPROTO_NONE:
> > > > > +   *ext_hdrs |= OFPIEH12_NONEXT;
> > > > > +   /* stop parsing */
> > > > > +   return;
> > > > > +
> > > > > +   case IPPROTO_ESP:
> > > > > +   if (*ext_hdrs & OFPIEH12_ESP)
> > > > > +   *ext_hdrs |= OFPIEH12_UNREP;
> > > > > +   if ((*ext_hdrs & ~(OFPIEH12_HOP | 
> > > > > OFPIEH12_DEST |
> > > > > +  OFPIEH12_ROUTER | 
> > > > > IPPROTO_FRAGMENT |
> > > > > +  OFPIEH12_AUTH | 
> > > > > OFPIEH12_UNREP)) ||
> > > 

Re: [ovs-dev] [ovs-discuss] [PATCH] datapath: fix crash when ipv6 fragment pkt recalculate L4 checksum

[Tonghao Zhang]

On Fri, Dec 10, 2021 at 10:59 AM zhounan (E) via discuss
 wrote:
>
> From: Zhou Nan 
>
> When we set ipv6 addr, we need to recalculate checksum of L4 header.
> In our testcase, after send ipv6 fragment package, KASAN detect "use after 
> free" when calling function update_ipv6_checksum, and crash occurred after a 
> while.
> If ipv6 package is fragment, and it is not first seg, we should not 
> recalculate checksum of L4 header since this kind of package has no
> L4 header.
> To prevent crash, we set "recalc_csum" "false" when calling function 
> "set_ipv6_addr".
> We also find that function skb_ensure_writable (make sure L4 header is 
> writable) is helpful before calling inet_proto_csum_replace16 to recalculate 
> checksum.
>
> Fixes: ada5efce102d6191e5c66fc385ba52a2d340ef50
>("datapath: Fix IPv6 later frags parsing")
>
> Signed-off-by: Zhou Nan 
> ---
>  datapath/actions.c | 20 +++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/datapath/actions.c b/datapath/actions.c index fbf4457..52cf03e 
> 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> @@ -456,12 +456,21 @@ static void update_ipv6_checksum(struct sk_buff *skb, 
> u8 l4_proto,
>  __be32 addr[4], const __be32 new_addr[4])  {
> int transport_len = skb->len - skb_transport_offset(skb);
> +   int err;
>
> if (l4_proto == NEXTHDR_TCP) {
> +   err = skb_ensure_writable(skb, skb_transport_offset(skb) +
> +   sizeof(struct tcphdr));
> +   if (unlikely(err))
> +   return;
> if (likely(transport_len >= sizeof(struct tcphdr)))
> inet_proto_csum_replace16(_hdr(skb)->check, skb,
>   addr, new_addr, true);
> } else if (l4_proto == NEXTHDR_UDP) {
> +   err = skb_ensure_writable(skb, skb_transport_offset(skb) +
> +   sizeof(struct udphdr));
> +   if (unlikely(err))
> +   return;
> if (likely(transport_len >= sizeof(struct udphdr))) {
> struct udphdr *uh = udp_hdr(skb);
>
> @@ -473,6 +482,10 @@ static void update_ipv6_checksum(struct sk_buff *skb, u8 
> l4_proto,
> }
> }
> } else if (l4_proto == NEXTHDR_ICMP) {
> +   err = skb_ensure_writable(skb, skb_transport_offset(skb) +
> +   sizeof(struct icmp6hdr));
> +   if (unlikely(err))
> +   return;
> if (likely(transport_len >= sizeof(struct icmp6hdr)))
> 
> inet_proto_csum_replace16(_hdr(skb)->icmp6_cksum,
>   skb, addr, new_addr, true);
> @@ -589,12 +602,15 @@ static int set_ipv6(struct sk_buff *skb, struct 
> sw_flow_key *flow_key,
> if (is_ipv6_mask_nonzero(mask->ipv6_src)) {
> __be32 *saddr = (__be32 *)>saddr;
> __be32 masked[4];
> +   bool recalc_csum = true;
>
> mask_ipv6_addr(saddr, key->ipv6_src, mask->ipv6_src, masked);
>
> if (unlikely(memcmp(saddr, masked, sizeof(masked {
> +   if (flow_key->ip.frag == OVS_FRAG_TYPE_LATER)
> +   recalc_csum = false;
> set_ipv6_addr(skb, flow_key->ip.proto, saddr, masked,
> - true);
> + recalc_csum);
> memcpy(_key->ipv6.addr.src, masked,
>sizeof(flow_key->ipv6.addr.src));
> }
> @@ -614,6 +630,8 @@ static int set_ipv6(struct sk_buff *skb, struct 
> sw_flow_key *flow_key,
>  NEXTHDR_ROUTING,
>  NULL, )
>!= NEXTHDR_ROUTING);
> +   if (flow_key->ip.frag == OVS_FRAG_TYPE_LATER)
> +   recalc_csum = false;
>
> set_ipv6_addr(skb, flow_key->ip.proto, daddr, masked,
>   recalc_csum);
> --
> 2.27.0
>
> ___
> discuss mailing list

As Gregory said, you should rebase your patch on linux upstream. and
patch is reviewd in net...@vger.kernel.org mail list.
OvS kernel module in upstream is:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/net/openvswitch

When the patch is applied in linux upstream, you can backport it.
Please see the section "Changes to Linux kernel components"
https://docs.openvswitch.org/en/latest/internals/contributing/backporting-patches/

> disc...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-discuss



-- 
Best regards, 

Re: [ovs-dev] [PATCH] datapath: fix crash when ipv6 fragment pkt recalculate L4 checksum

[0-day Robot]

Bleep bloop.  Greetings zhounan (E), I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
error: patch fragment without header at line 15: @@ -589,12 +602,15 @@ static 
int set_ipv6(struct sk_buff *skb, struct sw_flow_key *flow_key,
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 datapath: fix crash when ipv6 fragment pkt recalculate L4 
checksum
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Patch skipped due to previous failure.

Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] datapath: fix crash when ipv6 fragment pkt recalculate L4 checksum

[zhounan (E) via dev]

From: Zhou Nan 

When we set ipv6 addr, we need to recalculate checksum of L4 header.
In our testcase, after send ipv6 fragment package, KASAN detect "use after 
free" when calling function update_ipv6_checksum, and crash occurred after a 
while.
If ipv6 package is fragment, and it is not first seg, we should not recalculate 
checksum of L4 header since this kind of package has no
L4 header.
To prevent crash, we set "recalc_csum" "false" when calling function 
"set_ipv6_addr".
We also find that function skb_ensure_writable (make sure L4 header is 
writable) is helpful before calling inet_proto_csum_replace16 to recalculate 
checksum.

Fixes: ada5efce102d6191e5c66fc385ba52a2d340ef50
   ("datapath: Fix IPv6 later frags parsing")

Signed-off-by: Zhou Nan 
---
 datapath/actions.c | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/datapath/actions.c b/datapath/actions.c index fbf4457..52cf03e 
100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -456,12 +456,21 @@ static void update_ipv6_checksum(struct sk_buff *skb, u8 
l4_proto,
 __be32 addr[4], const __be32 new_addr[4])  {
int transport_len = skb->len - skb_transport_offset(skb);
+   int err;
 
if (l4_proto == NEXTHDR_TCP) {
+   err = skb_ensure_writable(skb, skb_transport_offset(skb) +
+   sizeof(struct tcphdr));
+   if (unlikely(err))
+   return;
if (likely(transport_len >= sizeof(struct tcphdr)))
inet_proto_csum_replace16(_hdr(skb)->check, skb,
  addr, new_addr, true);
} else if (l4_proto == NEXTHDR_UDP) {
+   err = skb_ensure_writable(skb, skb_transport_offset(skb) +
+   sizeof(struct udphdr));
+   if (unlikely(err))
+   return;
if (likely(transport_len >= sizeof(struct udphdr))) {
struct udphdr *uh = udp_hdr(skb);
 
@@ -473,6 +482,10 @@ static void update_ipv6_checksum(struct sk_buff *skb, u8 
l4_proto,
}
}
} else if (l4_proto == NEXTHDR_ICMP) {
+   err = skb_ensure_writable(skb, skb_transport_offset(skb) +
+   sizeof(struct icmp6hdr));
+   if (unlikely(err))
+   return;
if (likely(transport_len >= sizeof(struct icmp6hdr)))
inet_proto_csum_replace16(_hdr(skb)->icmp6_cksum,
  skb, addr, new_addr, true);
@@ -589,12 +602,15 @@ static int set_ipv6(struct sk_buff *skb, struct 
sw_flow_key *flow_key,
if (is_ipv6_mask_nonzero(mask->ipv6_src)) {
__be32 *saddr = (__be32 *)>saddr;
__be32 masked[4];
+   bool recalc_csum = true;
 
mask_ipv6_addr(saddr, key->ipv6_src, mask->ipv6_src, masked);
 
if (unlikely(memcmp(saddr, masked, sizeof(masked {
+   if (flow_key->ip.frag == OVS_FRAG_TYPE_LATER)
+   recalc_csum = false;
set_ipv6_addr(skb, flow_key->ip.proto, saddr, masked,
- true);
+ recalc_csum);
memcpy(_key->ipv6.addr.src, masked,
   sizeof(flow_key->ipv6.addr.src));
}
@@ -614,6 +630,8 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key 
*flow_key,
 NEXTHDR_ROUTING,
 NULL, )
   != NEXTHDR_ROUTING);
+   if (flow_key->ip.frag == OVS_FRAG_TYPE_LATER)
+   recalc_csum = false;
 
set_ipv6_addr(skb, flow_key->ip.proto, daddr, masked,
  recalc_csum);
--
2.27.0

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

Re: [ovs-dev] [PATCH] dpif-netdev: Use PMD context to get the port for HW miss recovery.

[Ilya Maximets]

On 12/5/21 07:13, Eli Britstein wrote:
> Acked-by: Eli Britstein 

Thanks!  Applied.

Best regards, Ilya Maximets.

> 
> On 12/3/2021 11:12 PM, Ilya Maximets wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> Last RX queue, from which the packet got received, is already stored
>> in the PMD context.  So, we can get the netdev from it without the
>> expensive hash map lookup.
>>
>> In my V2V testing this patch improves performance in case HW offload
>> and experimental APIs are enabled by about 3%.  That narrows down the
>> performance difference with the case with experimental API disabled
>> to about 0.5%, which is way within a margin of error for that setup.
>>
>> Signed-off-by: Ilya Maximets 
>> ---
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/1] dpdk: Update to use DPDK v21.11.

[Stokes, Ian]

> On 09/12/2021 15:21, Ian Stokes wrote:
> > This commit adds support for DPDK v21.11, it includes the following
> > changes.
> >
> > 1. ci: Install python elftools for DPDK 21.02.
> > 2. ci: Update meson requirement for DPDK 21.05.
> > 3. netdev-dpdk: Fix build with 21.05.
> > 4. ci: Compile DPDK in non developer mode.
> >
> >
> http://patchwork.ozlabs.org/project/openvswitch/list/?series=242480=*
> >
> > 5. netdev-dpdk: Remove access to DPDK internals.
> > 6. netdev-dpdk: Remove unused attribute from rte_flow rule.
> > 7. netdev-dpdk: Fix mbuf macros namespace with 21.11-rc1.
> > 8. netdev-dpdk: Fix vhost namespace with 21.11-rc2.
> >
> >
> http://patchwork.ozlabs.org/project/openvswitch/list/?series=271040=*
> >
> 
> There was a v3, so it should be:
> http://patchwork.ozlabs.org/project/openvswitch/list/?series=271159=*
> 
> > In addition documentaion and DPDK unit tests were also updated in this
> > commit for use with DPDK v21.11.
> >
> 
> typo, documentation
> 
> > For credit all authors of the original commits to 'dpdk-latest' with the 
> > above
> > changes have been added as co-authors for this commit.
> >
> > Signed-off-by: David Marchand
> > Co-authored-by: David Marchand
> > Reviewed-by: Maxime Coquelin
> > Signed-off-by: Ian Stokes
> >
> 
> There's no tested-by or mention of testing above. I didn't see it in the
> email threads either. Maybe that can be mentioned in the commit message
> to confirm it was done.
> 
> > ---
> 
> Just a few nits in the commit message that can be fixed on applying.
> 
> Otherwise assuming it went through Intel OVS ci, the patches lgtm
> (though i didn't review the ethdev internals one in detail). Built and
> ran a basic test to move some pkts, no issue.
> 
> btw, having links to the relevant DPDK commits in the ovs commit logs is
> really helpful for cross referencing, we should keep doing that. David++.
+1

Thanks for the review Kevin, I've applied the changes and pushed to master.

Thanks all for all the work on this, much appreciated.

Thanks
Ian



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

Re: [ovs-dev] [PATCH ovn 2/2] Prepare for post-21.12.0.

[Numan Siddique]

On Thu, Dec 9, 2021 at 10:46 AM Dumitru Ceara  wrote:
>
> On 12/9/21 16:32, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > Signed-off-by: Numan Siddique 
> > ---
> >  NEWS | 3 +++
> >  configure.ac | 2 +-
> >  debian/changelog | 6 ++
> >  3 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/NEWS b/NEWS
> > index d725480649..c54be39b8d 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -1,3 +1,6 @@
> > +Post v21.09.0
> > +-
>
> This should be v21.12.0, I think.

Oops.  Thanks for pointing this out.

I fixed this and applied both the patches to the main  and created a
new branch - branch-21.12 out of the first patch.

So we have officially branched out for the 21.12 release.

Regards
Numan

>
> With this updated:
>
> Acked-by: Dumitru Ceara 
>
> > +
> >  OVN v21.12.0 - xx xxx 
> >  --
> >- Set ignore_lsp_down to true as default, so that ARP responder flows are
> > diff --git a/configure.ac b/configure.ac
> > index 48b4662f0d..a6561b039c 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -13,7 +13,7 @@
> >  # limitations under the License.
> >
> >  AC_PREREQ(2.63)
> > -AC_INIT(ovn, 21.12.0, b...@openvswitch.org)
> > +AC_INIT(ovn, 21.12.90, b...@openvswitch.org)
> >  AC_CONFIG_MACRO_DIR([m4])
> >  AC_CONFIG_AUX_DIR([build-aux])
> >  AC_CONFIG_HEADERS([config.h])
> > diff --git a/debian/changelog b/debian/changelog
> > index ce78ddef70..7b01e5ef83 100644
> > --- a/debian/changelog
> > +++ b/debian/changelog
> > @@ -1,3 +1,9 @@
> > +ovn (21.12.90-1) unstable; urgency=low
> > +
> > +   * New upstream version
> > +
> > + -- OVN team   Thu, 09 Dec 2021 10:27:00 -0500
> > +
> >  ovn (21.12.0-1) unstable; urgency=low
> >
> > * New upstream version
> >
>
> ___
> 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 v1 1/1] dpdk: Update to use DPDK v21.11.

[Kevin Traynor]

On 09/12/2021 16:35, Stokes, Ian wrote:

On 12/9/21 16:24, Stokes, Ian wrote:

On Thu, Dec 9, 2021 at 1:27 PM Ian Stokes  wrote:


This commit adds support for DPDK v21.11, it includes the following
changes.

1. ci: Install python elftools for DPDK 21.02.
2. ci: Update meson requirement for DPDK 21.05.
3. netdev-dpdk: Fix build with 21.05.
4. ci: Compile DPDK in non developer mode.





http://patchwork.ozlabs.org/project/openvswitch/list/?series=242480=*


5. netdev-dpdk: Remove access to DPDK internals.
6. netdev-dpdk: Remove unused attribute from rte_flow rule.
7. netdev-dpdk: Fix mbuf macros namespace with 21.11-rc1.
8. netdev-dpdk: Fix vhost namespace with 21.11-rc2.





http://patchwork.ozlabs.org/project/openvswitch/list/?series=271040=*


In addition documentaion and DPDK unit tests were also updated in this
commit for use with DPDK v21.11.

For credit all authors of the original commits to 'dpdk-latest' with the above
changes have been added as co-authors for this commit.

Signed-off-by: David Marchand 
Co-authored-by: David Marchand 
Reviewed-by: Maxime Coquelin 
Signed-off-by: Ian Stokes 

---
RFC -> V1
* Add telemetry warning to OVS DPDK unit tests.
* Update TSO documentation link to 21.11.
* Update fedora spec to use 21.11.
---
  .ci/linux-build.sh   |   6 +-
  .ci/linux-prepare.sh |   4 +-
  Documentation/faq/releases.rst   |   2 +-
  Documentation/intro/install/dpdk.rst |  16 +-
  Documentation/topics/dpdk/phy.rst|   8 +-
  Documentation/topics/dpdk/vdev.rst   |   2 +-
  Documentation/topics/dpdk/vhost-user.rst |   2 +-
  Documentation/topics/testing.rst |   2 +-
  Documentation/topics/userspace-tso.rst   |   2 +-
  NEWS |   1 +
  lib/dp-packet.h  |  26 +-
  lib/netdev-dpdk.c| 115 +++
  rhel/openvswitch-fedora.spec | 515

+++

You should update rhel/openvswitch-fedora.spec.in not the generated
rhel/openvswitch-fedora.spec.


Ah, apologies, I've spun a v2 and corrected this + rebased on master.

[snip]


This log is displayed when DPDK is compiled without libjansson (in
this case, the metrics library register no "legacy" callbacks, hence
the log..).
Ok for waiving it for now.
I'll submit a patch on DPDK side to lower it to INFO level because I
find it useless.


Rest lgtm.

Thanks Ian!


Thanks David, as I said, I've sent a v2.

@Ilya Maximets Are there any blockers you see or am I good to  merge this?


On a brief look-through I didn't notice anything blocking in v2.
Though, I didn't try to use it.  So, if David and Kevin are OK with
the patch, then it should be good to go.

Best regards, Ilya Maximets.


Thanks Ilya,

@Kevin Traynor any blockers your side on above?



No blockers, just a few nits on the commit message. Replied directly to 
the v2 patch 
https://mail.openvswitch.org/pipermail/ovs-dev/2021-December/390066.html


thanks,
Kevin.


Thanks
Ian




Thanks
Ian



--
David Marchand






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

Re: [ovs-dev] [PATCH v2 1/1] dpdk: Update to use DPDK v21.11.

[Kevin Traynor]

On 09/12/2021 15:21, Ian Stokes wrote:

This commit adds support for DPDK v21.11, it includes the following
changes.

1. ci: Install python elftools for DPDK 21.02.
2. ci: Update meson requirement for DPDK 21.05.
3. netdev-dpdk: Fix build with 21.05.
4. ci: Compile DPDK in non developer mode.

http://patchwork.ozlabs.org/project/openvswitch/list/?series=242480=*

5. netdev-dpdk: Remove access to DPDK internals.
6. netdev-dpdk: Remove unused attribute from rte_flow rule.
7. netdev-dpdk: Fix mbuf macros namespace with 21.11-rc1.
8. netdev-dpdk: Fix vhost namespace with 21.11-rc2.

http://patchwork.ozlabs.org/project/openvswitch/list/?series=271040=*



There was a v3, so it should be:
http://patchwork.ozlabs.org/project/openvswitch/list/?series=271159=*


In addition documentaion and DPDK unit tests were also updated in this
commit for use with DPDK v21.11.



typo, documentation


For credit all authors of the original commits to 'dpdk-latest' with the above
changes have been added as co-authors for this commit.

Signed-off-by: David Marchand
Co-authored-by: David Marchand
Reviewed-by: Maxime Coquelin
Signed-off-by: Ian Stokes



There's no tested-by or mention of testing above. I didn't see it in the 
email threads either. Maybe that can be mentioned in the commit message 
to confirm it was done.



---


Just a few nits in the commit message that can be fixed on applying.

Otherwise assuming it went through Intel OVS ci, the patches lgtm 
(though i didn't review the ethdev internals one in detail). Built and 
ran a basic test to move some pkts, no issue.


btw, having links to the relevant DPDK commits in the ovs commit logs is 
really helpful for cross referencing, we should keep doing that. David++.


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

Re: [ovs-dev] [PATCH] compat: handle NF_REPEAT error on nf_conntrack_in

[Vladislav Odintsov]

Thanks Ilya!

Regards,
Vladislav Odintsov

> On 9 Dec 2021, at 20:05, Ilya Maximets  wrote:
> 
> On 12/6/21 18:46, Gregory Rose wrote:
>> 
>> 
>> On 11/26/2021 12:59 PM, Vladislav Odintsov wrote:
>>> In patch [1] rpl_nf_conntrack_in was backported as static inline
>>> function without do..while loop handling NF_REPEAT error.
>>> In patch [2] rpl_nf_conntrack_in backported function was removed
>>> from compat/include/net/netfilter/nf_conntrack_core.h as an unused.
>>> 
>>> As a result the do..while loop around nf_conntrack_in was lost and
>>> this caused problems on old RHEL kernels with the tcp SYN
>>> loss on a connection with same 5-tuple, which ran in last
>>> nf_conntrack_tcp_timeout_time_wait. The connection could be
>>> initiated on a tcp SYN retry after one second.
>>> 
>>> 1: 
>>> https://github.com/openvswitch/ovs/commit/4fdec8986a203b0dc9d9c183c932826967572e0f
>>> 2: 
>>> https://github.com/openvswitch/ovs/commit/e9b33ad780f3bc712a5de6be9e1e0803fadcd249
>>> 
>>> Reported-at: 
>>> https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387623.html
>>> Reported-at: 
>>> https://mail.openvswitch.org/pipermail/ovs-dev/2021-October/388424.html
>>> Signed-off-by: Vladislav Odintsov 
>> 
>> LGTM - Thanks!
>> 
>> Reviewed-by: Greg Rose 
> 
> Thanks!  Applied and backported.
> 
> Best regards, Ilya Maximets.
> ___
> 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 v1] ofp-actions: Always validate action size

[Ilya Maximets]

On 11/26/21 23:16, Mike Pattrick wrote:
> Currently, ovs-ofctl and other associated tools will validate the size
> of flow actions. However, there are some code paths that do not validate
> the size correctly.
> 
> When adding more than 1000 logical switch ports to an OVS bridge in OVN,
> OVN will happily create a flow with potentially unlimited actions. This
> can cause OVS to call abort() when it attempts to re-serialize the flow
> actions.


Hello, Mike.

This Looks like a tricky issue.  Could you, please, include a unit test
for this case?

Best regards, Ilya Maximets.

> 
> This change will validate the size with every call to ofpacts_verify,
> which should cover all remaining code paths.
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2020770
> Signed-off-by: Mike Pattrick 
> ---
>  lib/ofp-actions.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index ecf914eac..74b8b65ac 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -8749,6 +8749,10 @@ ofpacts_verify(const struct ofpact ofpacts[], size_t 
> ofpacts_len,
>  const struct ofpact *a;
>  enum ovs_instruction_type inst;
>  
> +if (ofpacts_len > ROUND_DOWN(UINT16_MAX, OFP_ACTION_ALIGN)) {
> +return OFPERR_OFPBAC_BAD_LEN;
> +}
> +
>  inst = OVSINST_OFPIT13_METER;
>  OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
>  enum ovs_instruction_type next;
> 

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

Re: [ovs-dev] [PATCH] compat: handle NF_REPEAT error on nf_conntrack_in

[Ilya Maximets]

On 12/6/21 18:46, Gregory Rose wrote:
> 
> 
> On 11/26/2021 12:59 PM, Vladislav Odintsov wrote:
>> In patch [1] rpl_nf_conntrack_in was backported as static inline
>> function without do..while loop handling NF_REPEAT error.
>> In patch [2] rpl_nf_conntrack_in backported function was removed
>> from compat/include/net/netfilter/nf_conntrack_core.h as an unused.
>>
>> As a result the do..while loop around nf_conntrack_in was lost and
>> this caused problems on old RHEL kernels with the tcp SYN
>> loss on a connection with same 5-tuple, which ran in last
>> nf_conntrack_tcp_timeout_time_wait. The connection could be
>> initiated on a tcp SYN retry after one second.
>>
>> 1: 
>> https://github.com/openvswitch/ovs/commit/4fdec8986a203b0dc9d9c183c932826967572e0f
>> 2: 
>> https://github.com/openvswitch/ovs/commit/e9b33ad780f3bc712a5de6be9e1e0803fadcd249
>>
>> Reported-at: 
>> https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387623.html
>> Reported-at: 
>> https://mail.openvswitch.org/pipermail/ovs-dev/2021-October/388424.html
>> Signed-off-by: Vladislav Odintsov 
> 
> LGTM - Thanks!
> 
> Reviewed-by: Greg Rose 

Thanks!  Applied and backported.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 ovn] inc-proc-eng: move inc-proc code in an isolated strucuture

[Lorenzo Bianconi]

Remove global state variable and move move inc-proc code in an isolated
strucuture. This is a preliminary patch to add the capability to run
multiple inc-proc engines.

Signed-off-by: Lorenzo Bianconi 
---
Changes since v1:
- fix unixctl commands for IP engine.
---
 controller/ovn-controller.c |  65 ++--
 lib/inc-proc-eng.c  | 199 ++--
 lib/inc-proc-eng.h  |  40 ++--
 northd/en-lflow.c   |   2 +-
 northd/en-northd.c  |   2 +-
 northd/inc-proc-northd.c|  28 ++---
 6 files changed, 202 insertions(+), 134 deletions(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 5069aedfc..9f5e55bac 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -114,6 +114,8 @@ static unixctl_cb_func debug_delay_nb_cfg_report;
 #define OVS_NB_CFG_TS_NAME "ovn-nb-cfg-ts"
 #define OVS_STARTUP_TS_NAME "ovn-startup-ts"
 
+static struct engine *flow_engine;
+
 static char *parse_options(int argc, char *argv[]);
 OVS_NO_RETURN static void usage(void);
 
@@ -557,7 +559,7 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl 
*ovnsb_idl,
 }
 if (reset_ovnsb_idl_min_index && *reset_ovnsb_idl_min_index) {
 VLOG_INFO("Resetting southbound database cluster state");
-engine_set_force_recompute(true);
+engine_set_force_recompute(flow_engine, true);
 ovsdb_idl_reset_min_index(ovnsb_idl);
 *reset_ovnsb_idl_min_index = false;
 }
@@ -1011,7 +1013,8 @@ en_ofctrl_is_connected_cleanup(void *data OVS_UNUSED)
 static void
 en_ofctrl_is_connected_run(struct engine_node *node, void *data)
 {
-struct controller_engine_ctx *ctrl_ctx = engine_get_context()->client_ctx;
+struct controller_engine_ctx *ctrl_ctx =
+engine_get_context(flow_engine)->client_ctx;
 struct ed_type_ofctrl_is_connected *of_data = data;
 if (of_data->connected != ofctrl_is_connected()) {
 of_data->connected = !of_data->connected;
@@ -1226,10 +1229,11 @@ init_binding_ctx(struct engine_node *node,
 engine_get_input("SB_port_binding", node),
 "datapath");
 
-struct controller_engine_ctx *ctrl_ctx = engine_get_context()->client_ctx;
+struct controller_engine_ctx *ctrl_ctx =
+engine_get_context(flow_engine)->client_ctx;
 
-b_ctx_in->ovnsb_idl_txn = engine_get_context()->ovnsb_idl_txn;
-b_ctx_in->ovs_idl_txn = engine_get_context()->ovs_idl_txn;
+b_ctx_in->ovnsb_idl_txn = engine_get_context(flow_engine)->ovnsb_idl_txn;
+b_ctx_in->ovs_idl_txn = engine_get_context(flow_engine)->ovs_idl_txn;
 b_ctx_in->sbrec_datapath_binding_by_key = sbrec_datapath_binding_by_key;
 b_ctx_in->sbrec_port_binding_by_datapath = sbrec_port_binding_by_datapath;
 b_ctx_in->sbrec_port_binding_by_name = sbrec_port_binding_by_name;
@@ -2387,7 +2391,8 @@ en_lflow_output_run(struct engine_node *node, void *data)
 lflow_conj_ids_clear(>conj_ids);
 }
 
-struct controller_engine_ctx *ctrl_ctx = engine_get_context()->client_ctx;
+struct controller_engine_ctx *ctrl_ctx =
+engine_get_context(flow_engine)->client_ctx;
 
 fo->pd.lflow_cache = ctrl_ctx->lflow_cache;
 
@@ -3040,7 +3045,7 @@ check_northd_version(struct ovsdb_idl *ovs_idl, struct 
ovsdb_idl *ovnsb_idl,
  * full recompute.
  */
 if (version_mismatch) {
-engine_set_force_recompute(true);
+engine_set_force_recompute(flow_engine, true);
 }
 version_mismatch = false;
 return true;
@@ -3344,7 +3349,7 @@ main(int argc, char *argv[])
 .sb_idl = ovnsb_idl_loop.idl,
 .ovs_idl = ovs_idl_loop.idl,
 };
-engine_init(_flow_output, _arg);
+engine_init(_engine, _flow_output, _arg, "flow_engine");
 
 engine_ovsdb_node_add_index(_sb_chassis, "name", sbrec_chassis_by_name);
 engine_ovsdb_node_add_index(_sb_multicast_group, "name_datapath",
@@ -3396,7 +3401,7 @@ main(int argc, char *argv[])
 
 unixctl_command_register("recompute", "[deprecated]", 0, 0,
  engine_recompute_cmd,
- NULL);
+ flow_engine);
 unixctl_command_register("lflow-cache/flush", "", 0, 0,
  lflow_cache_flush_cmd,
  _output_data->pd);
@@ -3480,7 +3485,7 @@ main(int argc, char *argv[])
 goto loop_done;
 }
 
-engine_init_run();
+engine_init_run(flow_engine);
 
 struct ovsdb_idl_txn *ovs_idl_txn = ovsdb_idl_loop_run(_idl_loop);
 unsigned int new_ovs_cond_seqno
@@ -3488,7 +3493,7 @@ main(int argc, char *argv[])
 if (new_ovs_cond_seqno != ovs_cond_seqno) {
 if (!new_ovs_cond_seqno) {
 VLOG_INFO("OVS IDL reconnected, force recompute.");
-engine_set_force_recompute(true);
+engine_set_force_recompute(flow_engine, true);
 }
 ovs_cond_seqno = 

Re: [ovs-dev] [PATCH v1 1/1] dpdk: Update to use DPDK v21.11.

[Stokes, Ian]

> On 12/9/21 16:24, Stokes, Ian wrote:
> >> On Thu, Dec 9, 2021 at 1:27 PM Ian Stokes  wrote:
> >>>
> >>> This commit adds support for DPDK v21.11, it includes the following
> >>> changes.
> >>>
> >>> 1. ci: Install python elftools for DPDK 21.02.
> >>> 2. ci: Update meson requirement for DPDK 21.05.
> >>> 3. netdev-dpdk: Fix build with 21.05.
> >>> 4. ci: Compile DPDK in non developer mode.
> >>>
> >>>
> >>
> http://patchwork.ozlabs.org/project/openvswitch/list/?series=242480=*
> >>>
> >>> 5. netdev-dpdk: Remove access to DPDK internals.
> >>> 6. netdev-dpdk: Remove unused attribute from rte_flow rule.
> >>> 7. netdev-dpdk: Fix mbuf macros namespace with 21.11-rc1.
> >>> 8. netdev-dpdk: Fix vhost namespace with 21.11-rc2.
> >>>
> >>>
> >>
> http://patchwork.ozlabs.org/project/openvswitch/list/?series=271040=*
> >>>
> >>> In addition documentaion and DPDK unit tests were also updated in this
> >>> commit for use with DPDK v21.11.
> >>>
> >>> For credit all authors of the original commits to 'dpdk-latest' with the 
> >>> above
> >>> changes have been added as co-authors for this commit.
> >>>
> >>> Signed-off-by: David Marchand 
> >>> Co-authored-by: David Marchand 
> >>> Reviewed-by: Maxime Coquelin 
> >>> Signed-off-by: Ian Stokes 
> >>>
> >>> ---
> >>> RFC -> V1
> >>> * Add telemetry warning to OVS DPDK unit tests.
> >>> * Update TSO documentation link to 21.11.
> >>> * Update fedora spec to use 21.11.
> >>> ---
> >>>  .ci/linux-build.sh   |   6 +-
> >>>  .ci/linux-prepare.sh |   4 +-
> >>>  Documentation/faq/releases.rst   |   2 +-
> >>>  Documentation/intro/install/dpdk.rst |  16 +-
> >>>  Documentation/topics/dpdk/phy.rst|   8 +-
> >>>  Documentation/topics/dpdk/vdev.rst   |   2 +-
> >>>  Documentation/topics/dpdk/vhost-user.rst |   2 +-
> >>>  Documentation/topics/testing.rst |   2 +-
> >>>  Documentation/topics/userspace-tso.rst   |   2 +-
> >>>  NEWS |   1 +
> >>>  lib/dp-packet.h  |  26 +-
> >>>  lib/netdev-dpdk.c| 115 +++
> >>>  rhel/openvswitch-fedora.spec | 515
> >> +++
> >>
> >> You should update rhel/openvswitch-fedora.spec.in not the generated
> >> rhel/openvswitch-fedora.spec.
> >
> > Ah, apologies, I've spun a v2 and corrected this + rebased on master.
> >
> > [snip]
> >
> >> This log is displayed when DPDK is compiled without libjansson (in
> >> this case, the metrics library register no "legacy" callbacks, hence
> >> the log..).
> >> Ok for waiving it for now.
> >> I'll submit a patch on DPDK side to lower it to INFO level because I
> >> find it useless.
> >>
> >>
> >> Rest lgtm.
> >>
> >> Thanks Ian!
> >
> > Thanks David, as I said, I've sent a v2.
> >
> > @Ilya Maximets Are there any blockers you see or am I good to  merge this?
> 
> On a brief look-through I didn't notice anything blocking in v2.
> Though, I didn't try to use it.  So, if David and Kevin are OK with
> the patch, then it should be good to go.
> 
> Best regards, Ilya Maximets.

Thanks Ilya,

@Kevin Traynor any blockers your side on above?

Thanks
Ian
> 
> >
> > Thanks
> > Ian
> >>
> >>
> >> --
> >> David Marchand
> >

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

Re: [ovs-dev] [PATCH 1/4] netdev-dpdk: Introduce per rxq/txq Vhost-user statistics

[Maxime Coquelin]

On 12/9/21 17:29, Ilya Maximets wrote:

On 12/9/21 16:04, David Marchand wrote:

On Thu, Dec 9, 2021 at 3:51 PM Maxime Coquelin
 wrote:

On 12/7/21 21:37, David Marchand wrote:

+struct netdev_dpdk_vhost_q_stats {
+uint64_t bytes;
+uint64_t packets;
+uint64_t errors;
+};
+
   /* Custom software stats for dpdk ports */
   struct netdev_dpdk_sw_stats {
   /* No. of retries when unable to transmit. */
@@ -206,6 +213,10 @@ struct netdev_dpdk_sw_stats {
   uint64_t rx_qos_drops;
   /* Packet drops in HWOL processing. */
   uint64_t tx_invalid_hwol_drops;
+/* Per-queue Vhost Tx stats */
+struct netdev_dpdk_vhost_q_stats *txq;
+/* Per-queue Vhost Rx stats */
+struct netdev_dpdk_vhost_q_stats *rxq;

|Here, we add "driver" specific stats, while netdev_dpdk_sw_stats struct
carries OVS "own" stats. This netdev_dpdk_sw_stats struct is converted
by netdev_dpdk_get_sw_custom_stats and there is a small framework on
adding custom OVS stats (using some macros "trick"). I'd rather leave
netdev_dpdk_sw_stats struct untouched for consistency. Pointers to vhost
specific stats can be added to the netdev_dpdk struct (we have some
spare space after the pointer to netdev_dpdk_sw_stats). |


I checked with pahole, and the comment stating "/*36 pad bytes here.*/"
is outdated. Since it is difficult to maintain it given it needs to be
updated when struct netdev_stats is modified, I will just remove it in
next revision.


Yeah... I don't like those comments which become obsolete too easily.
+1


For the reference:
d9d73f84ea22 ("Revert "dpif_netdev: Refactor dp_netdev_pmd_thread structure."")

This one needs a follow up. :)
Not sure if you want to go into that rabbit hole right now though.


Right now, not really. :)
But I can for sure take the action for the next release.

Regards,
Maxime


Best regards, Ilya Maximets.



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

Re: [ovs-dev] [PATCH 1/4] netdev-dpdk: Introduce per rxq/txq Vhost-user statistics

[Ilya Maximets]

On 12/9/21 16:04, David Marchand wrote:
> On Thu, Dec 9, 2021 at 3:51 PM Maxime Coquelin
>  wrote:
>> On 12/7/21 21:37, David Marchand wrote:
 +struct netdev_dpdk_vhost_q_stats {
 +uint64_t bytes;
 +uint64_t packets;
 +uint64_t errors;
 +};
 +
   /* Custom software stats for dpdk ports */
   struct netdev_dpdk_sw_stats {
   /* No. of retries when unable to transmit. */
 @@ -206,6 +213,10 @@ struct netdev_dpdk_sw_stats {
   uint64_t rx_qos_drops;
   /* Packet drops in HWOL processing. */
   uint64_t tx_invalid_hwol_drops;
 +/* Per-queue Vhost Tx stats */
 +struct netdev_dpdk_vhost_q_stats *txq;
 +/* Per-queue Vhost Rx stats */
 +struct netdev_dpdk_vhost_q_stats *rxq;
>>> |Here, we add "driver" specific stats, while netdev_dpdk_sw_stats struct
>>> carries OVS "own" stats. This netdev_dpdk_sw_stats struct is converted
>>> by netdev_dpdk_get_sw_custom_stats and there is a small framework on
>>> adding custom OVS stats (using some macros "trick"). I'd rather leave
>>> netdev_dpdk_sw_stats struct untouched for consistency. Pointers to vhost
>>> specific stats can be added to the netdev_dpdk struct (we have some
>>> spare space after the pointer to netdev_dpdk_sw_stats). |
>>
>> I checked with pahole, and the comment stating "/*36 pad bytes here.*/"
>> is outdated. Since it is difficult to maintain it given it needs to be
>> updated when struct netdev_stats is modified, I will just remove it in
>> next revision.
> 
> Yeah... I don't like those comments which become obsolete too easily.
> +1

For the reference:  
d9d73f84ea22 ("Revert "dpif_netdev: Refactor dp_netdev_pmd_thread structure."")

This one needs a follow up. :)
Not sure if you want to go into that rabbit hole right now though.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1 1/1] dpdk: Update to use DPDK v21.11.

[Ilya Maximets]

On 12/9/21 16:24, Stokes, Ian wrote:
>> On Thu, Dec 9, 2021 at 1:27 PM Ian Stokes  wrote:
>>>
>>> This commit adds support for DPDK v21.11, it includes the following
>>> changes.
>>>
>>> 1. ci: Install python elftools for DPDK 21.02.
>>> 2. ci: Update meson requirement for DPDK 21.05.
>>> 3. netdev-dpdk: Fix build with 21.05.
>>> 4. ci: Compile DPDK in non developer mode.
>>>
>>>
>> http://patchwork.ozlabs.org/project/openvswitch/list/?series=242480=*
>>>
>>> 5. netdev-dpdk: Remove access to DPDK internals.
>>> 6. netdev-dpdk: Remove unused attribute from rte_flow rule.
>>> 7. netdev-dpdk: Fix mbuf macros namespace with 21.11-rc1.
>>> 8. netdev-dpdk: Fix vhost namespace with 21.11-rc2.
>>>
>>>
>> http://patchwork.ozlabs.org/project/openvswitch/list/?series=271040=*
>>>
>>> In addition documentaion and DPDK unit tests were also updated in this
>>> commit for use with DPDK v21.11.
>>>
>>> For credit all authors of the original commits to 'dpdk-latest' with the 
>>> above
>>> changes have been added as co-authors for this commit.
>>>
>>> Signed-off-by: David Marchand 
>>> Co-authored-by: David Marchand 
>>> Reviewed-by: Maxime Coquelin 
>>> Signed-off-by: Ian Stokes 
>>>
>>> ---
>>> RFC -> V1
>>> * Add telemetry warning to OVS DPDK unit tests.
>>> * Update TSO documentation link to 21.11.
>>> * Update fedora spec to use 21.11.
>>> ---
>>>  .ci/linux-build.sh   |   6 +-
>>>  .ci/linux-prepare.sh |   4 +-
>>>  Documentation/faq/releases.rst   |   2 +-
>>>  Documentation/intro/install/dpdk.rst |  16 +-
>>>  Documentation/topics/dpdk/phy.rst|   8 +-
>>>  Documentation/topics/dpdk/vdev.rst   |   2 +-
>>>  Documentation/topics/dpdk/vhost-user.rst |   2 +-
>>>  Documentation/topics/testing.rst |   2 +-
>>>  Documentation/topics/userspace-tso.rst   |   2 +-
>>>  NEWS |   1 +
>>>  lib/dp-packet.h  |  26 +-
>>>  lib/netdev-dpdk.c| 115 +++
>>>  rhel/openvswitch-fedora.spec | 515
>> +++
>>
>> You should update rhel/openvswitch-fedora.spec.in not the generated
>> rhel/openvswitch-fedora.spec.
> 
> Ah, apologies, I've spun a v2 and corrected this + rebased on master.
> 
> [snip]
> 
>> This log is displayed when DPDK is compiled without libjansson (in
>> this case, the metrics library register no "legacy" callbacks, hence
>> the log..).
>> Ok for waiving it for now.
>> I'll submit a patch on DPDK side to lower it to INFO level because I
>> find it useless.
>>
>>
>> Rest lgtm.
>>
>> Thanks Ian!
> 
> Thanks David, as I said, I've sent a v2.
> 
> @Ilya Maximets Are there any blockers you see or am I good to  merge this?

On a brief look-through I didn't notice anything blocking in v2.
Though, I didn't try to use it.  So, if David and Kevin are OK with
the patch, then it should be good to go.

Best regards, Ilya Maximets.

> 
> Thanks
> Ian
>>
>>
>> --
>> David Marchand
> 

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

Re: [ovs-dev] [PATCH ovn 3/3] northd: support mix of stateless ACL with lower priority than stateful

[Vladislav Odintsov]

Hi Dumitru,

thanks for review & testing and pointing for the broken behaviour.
I’ve missed such case, and I think this usecase should be of course supported. 
Maybe, it should be listed in a ovn testcases for not loose it one day?
That’s very interesting situation as for me, I have to think how we can solve 
it.

Anyway, I’ve managed to resolve my personal task, which initially was the 
reason for this patch, and if it's not needed for the community, I’m okay to 
close it for now.
If somebody is interested in it, please let me know.

What do you think?

Regards,
Vladislav Odintsov

> On 9 Dec 2021, at 13:45, Dumitru Ceara  wrote:
> 
> On 12/1/21 13:56, Vladislav Odintsov wrote:
>> It was unsupported - to have a mix of stateless ACLs with lower than
>> stateful priority at the same time with stateful (related) ACLs.
>> 
>> This patch changes stateless ACLs behaviour, but this change should not
>> be harmful for users. Likely nobody mixed rules in the described manner,
>> so this change should be okay.
>> 
>> Signed-off-by: Vladislav Odintsov > >
>> ---
> 
> Hi Vladislav,
> 
> We also need to update the documentation here:
> https://github.com/ovn-org/ovn/blob/a2e7f69d6684766dadfb0a1eb455e123f6bd200a/ovn-nb.xml#L1934
>  
> 
> 
> Should this be a "Fixes: 3187b9fef1 ("ovn-northd: introduce new
> allow-stateless ACL verb")"?  CC-ing Ihar and Han for more input on this
> matter.
> 
> I guess this should give more context on the original implementation:
> https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/383513.html 
> 
> 
> However, I think there are some issues with your approach; using the
> example Han gave there (adding it here for clarity):
> 
>  All traffic from A to B (egress) is stateless:
>  ACL1: from-lport 100 'inport == "A" && ip.dst == B' allow-stateless
> 
>  So does the return traffic:
>  ACL2: to-lport 100 'outport == "A" && ip.src == B' allow-stateless
> 
>  Now we need the traffic initiated from A to B's TCP port 80 to be
>  stateful:
>  ACL3: from-lport 200 'inport == "A" && ip.dst == B && tcp.dst == 80' 
> allow-related
> 
> My topology consists of one logical switch and two ports:
> 
>  ovn-nbctl ls-add ls
>  ovn-nbctl lsp-add ls vm1
>  ovn-nbctl lsp-set-addresses vm1 00:00:00:00:00:01
> 
>  ip netns add vm1
>  ovs-vsctl add-port br-int vm1 -- set interface vm1 type=internal
>  ip link set vm1 netns vm1
>  ip netns exec vm1 ip link set vm1 address 00:00:00:00:00:01
>  ip netns exec vm1 ip addr add 42.42.42.2/24 dev vm1
>  ip netns exec vm1 ip link set vm1 up
>  ovs-vsctl set Interface vm1 external_ids:iface-id=vm1
> 
>  ovn-nbctl lsp-add ls vm2
>  ovn-nbctl lsp-set-addresses vm2 00:00:00:00:00:02
> 
>  ip netns add vm2
>  ovs-vsctl add-port br-int vm2 -- set interface vm2 type=internal
>  ip link set vm2 netns vm2
>  ip netns exec vm2 ip link set vm2 address 00:00:00:00:00:02
>  ip netns exec vm2 ip addr add 42.42.42.3/24 dev vm2
>  ip netns exec vm2 ip link set vm2 up
>  ovs-vsctl set Interface vm2 external_ids:iface-id=vm2
> 
>  ovn-nbctl acl-add ls from-lport 100 'inport == "vm1" && ip4.dst == 
> 42.42.42.3' allow-stateless
>  ovn-nbctl acl-add ls to-lport 100 'outport == "vm1" && ip4.src == 
> 42.42.42.3' allow-stateless
>  ovn-nbctl acl-add ls from-lport 200 'inport == "vm1" && ip4.dst == 
> 42.42.42.3 && tcp.dst == 80' allow-related
> 
> I'd expect TCP sessions to always be established with this config and
> that is the case without your patch, sessions can be established fine
> between A:X -> B:80:
>  tcp  6 431998 ESTABLISHED src=42.42.42.2 dst=42.42.42.3 sport=52994 
> dport=80 src=42.42.42.3 dst=42.42.42.2 sport=80 dport=52994 [ASSURED] mark=0 
> secctx=system_u:object_r:unlabeled_t:s0 zone=1 use=1
> 
> While, when applying your patch, the connection is not fully established:
>  tcp  6 19 SYN_RECV src=42.42.42.2 dst=42.42.42.3 sport=60730 dport=8080 
> src=42.42.42.3 dst=42.42.42.2 sport=8080 dport=60730 mark=0 
> secctx=system_u:object_r:unlabeled_t:s0 zone=1 use=1
>  tcp  6 79 SYN_SENT src=42.42.42.2 dst=42.42.42.3 sport=60730 dport=8080 
> [UNREPLIED] src=42.42.42.3 dst=42.42.42.2 sport=8080 dport=60730 mark=0 
> secctx=system_u:object_r:unlabeled_t:s0 zone=3 use=1
> 
> Zone 1 is vm1's conntrack zone and zone 3 corresponds to vm2.
> 
>> northd/northd.c | 92 +
>> northd/ovn-northd.8.xml |  4 +-
>> tests/ovn-northd.at | 48 -
>> 3 files changed, 87 insertions(+), 57 deletions(-)
>> 
>> diff --git a/northd/northd.c b/northd/northd.c
>> index 2efc4bb1f..1831e6e79 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -592,6 +592,7 @@ struct ovn_datapath {
>> uint32_t port_key_hint;
>> 
>> bool has_stateful_acl;
>> +bool has_stateless_acl;
>> bool has_lb_vip;
>> bool 

Re: [ovs-dev] [PATCH ovn 2/2] Prepare for post-21.12.0.

[Dumitru Ceara]

On 12/9/21 16:32, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> Signed-off-by: Numan Siddique 
> ---
>  NEWS | 3 +++
>  configure.ac | 2 +-
>  debian/changelog | 6 ++
>  3 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/NEWS b/NEWS
> index d725480649..c54be39b8d 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,3 +1,6 @@
> +Post v21.09.0
> +-

This should be v21.12.0, I think.

With this updated:

Acked-by: Dumitru Ceara 

> +
>  OVN v21.12.0 - xx xxx 
>  --
>- Set ignore_lsp_down to true as default, so that ARP responder flows are
> diff --git a/configure.ac b/configure.ac
> index 48b4662f0d..a6561b039c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -13,7 +13,7 @@
>  # limitations under the License.
>  
>  AC_PREREQ(2.63)
> -AC_INIT(ovn, 21.12.0, b...@openvswitch.org)
> +AC_INIT(ovn, 21.12.90, b...@openvswitch.org)
>  AC_CONFIG_MACRO_DIR([m4])
>  AC_CONFIG_AUX_DIR([build-aux])
>  AC_CONFIG_HEADERS([config.h])
> diff --git a/debian/changelog b/debian/changelog
> index ce78ddef70..7b01e5ef83 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,9 @@
> +ovn (21.12.90-1) unstable; urgency=low
> +
> +   * New upstream version
> +
> + -- OVN team   Thu, 09 Dec 2021 10:27:00 -0500
> +
>  ovn (21.12.0-1) unstable; urgency=low
>  
> * New upstream version
> 

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

Re: [ovs-dev] [PATCH ovn 1/2] Prepare for 21.12.0.

[Dumitru Ceara]

On 12/9/21 16:32, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> Signed-off-by: Numan Siddique 
> ---

I compared this to the one used for 21.09.0 and looks ok to me:

Acked-by: Dumitru Ceara 


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

[ovs-dev] [PATCH ovn 2/2] Prepare for post-21.12.0.

[numans]

From: Numan Siddique 

Signed-off-by: Numan Siddique 
---
 NEWS | 3 +++
 configure.ac | 2 +-
 debian/changelog | 6 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index d725480649..c54be39b8d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,6 @@
+Post v21.09.0
+-
+
 OVN v21.12.0 - xx xxx 
 --
   - Set ignore_lsp_down to true as default, so that ARP responder flows are
diff --git a/configure.ac b/configure.ac
index 48b4662f0d..a6561b039c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(ovn, 21.12.0, b...@openvswitch.org)
+AC_INIT(ovn, 21.12.90, b...@openvswitch.org)
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
diff --git a/debian/changelog b/debian/changelog
index ce78ddef70..7b01e5ef83 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+ovn (21.12.90-1) unstable; urgency=low
+
+   * New upstream version
+
+ -- OVN team   Thu, 09 Dec 2021 10:27:00 -0500
+
 ovn (21.12.0-1) unstable; urgency=low
 
* New upstream version
-- 
2.33.1

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

[ovs-dev] [PATCH ovn 1/2] Prepare for 21.12.0.

[numans]

From: Numan Siddique 

Signed-off-by: Numan Siddique 
---
 NEWS | 4 ++--
 configure.ac | 2 +-
 debian/changelog | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index b2af2788e9..d725480649 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,5 @@
-Post v21.09.0
--
+OVN v21.12.0 - xx xxx 
+--
   - Set ignore_lsp_down to true as default, so that ARP responder flows are
 installed together with other flows when a logical switch port is created,
 without having to wait for the port to be UP.  CMS should set it to false
diff --git a/configure.ac b/configure.ac
index 5a3a5987bc..48b4662f0d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(ovn, 21.09.90, b...@openvswitch.org)
+AC_INIT(ovn, 21.12.0, b...@openvswitch.org)
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
diff --git a/debian/changelog b/debian/changelog
index ed1dbe019e..ce78ddef70 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,8 +1,8 @@
-ovn (21.09.90-1) unstable; urgency=low
+ovn (21.12.0-1) unstable; urgency=low
 
* New upstream version
 
- -- OVN team   Fri, 17 Sep 2021 12:00:00 -0400
+ -- OVN team   Thu, 09 Dec 2021 10:25:00 -0500
 
 ovn (21.09.0-1) unstable; urgency=low
 
-- 
2.33.1

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

Re: [ovs-dev] [PATCH v1 1/1] dpdk: Update to use DPDK v21.11.

[Stokes, Ian]

> On Thu, Dec 9, 2021 at 1:27 PM Ian Stokes  wrote:
> >
> > This commit adds support for DPDK v21.11, it includes the following
> > changes.
> >
> > 1. ci: Install python elftools for DPDK 21.02.
> > 2. ci: Update meson requirement for DPDK 21.05.
> > 3. netdev-dpdk: Fix build with 21.05.
> > 4. ci: Compile DPDK in non developer mode.
> >
> >
> http://patchwork.ozlabs.org/project/openvswitch/list/?series=242480=*
> >
> > 5. netdev-dpdk: Remove access to DPDK internals.
> > 6. netdev-dpdk: Remove unused attribute from rte_flow rule.
> > 7. netdev-dpdk: Fix mbuf macros namespace with 21.11-rc1.
> > 8. netdev-dpdk: Fix vhost namespace with 21.11-rc2.
> >
> >
> http://patchwork.ozlabs.org/project/openvswitch/list/?series=271040=*
> >
> > In addition documentaion and DPDK unit tests were also updated in this
> > commit for use with DPDK v21.11.
> >
> > For credit all authors of the original commits to 'dpdk-latest' with the 
> > above
> > changes have been added as co-authors for this commit.
> >
> > Signed-off-by: David Marchand 
> > Co-authored-by: David Marchand 
> > Reviewed-by: Maxime Coquelin 
> > Signed-off-by: Ian Stokes 
> >
> > ---
> > RFC -> V1
> > * Add telemetry warning to OVS DPDK unit tests.
> > * Update TSO documentation link to 21.11.
> > * Update fedora spec to use 21.11.
> > ---
> >  .ci/linux-build.sh   |   6 +-
> >  .ci/linux-prepare.sh |   4 +-
> >  Documentation/faq/releases.rst   |   2 +-
> >  Documentation/intro/install/dpdk.rst |  16 +-
> >  Documentation/topics/dpdk/phy.rst|   8 +-
> >  Documentation/topics/dpdk/vdev.rst   |   2 +-
> >  Documentation/topics/dpdk/vhost-user.rst |   2 +-
> >  Documentation/topics/testing.rst |   2 +-
> >  Documentation/topics/userspace-tso.rst   |   2 +-
> >  NEWS |   1 +
> >  lib/dp-packet.h  |  26 +-
> >  lib/netdev-dpdk.c| 115 +++
> >  rhel/openvswitch-fedora.spec | 515
> +++
> 
> You should update rhel/openvswitch-fedora.spec.in not the generated
> rhel/openvswitch-fedora.spec.

Ah, apologies, I've spun a v2 and corrected this + rebased on master.

[snip]

> This log is displayed when DPDK is compiled without libjansson (in
> this case, the metrics library register no "legacy" callbacks, hence
> the log..).
> Ok for waiving it for now.
> I'll submit a patch on DPDK side to lower it to INFO level because I
> find it useless.
> 
> 
> Rest lgtm.
> 
> Thanks Ian!

Thanks David, as I said, I've sent a v2.

@Ilya Maximets Are there any blockers you see or am I good to  merge this?

Thanks
Ian
> 
> 
> --
> David Marchand

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

[ovs-dev] [PATCH v2 1/1] dpdk: Update to use DPDK v21.11.

[Ian Stokes]

This commit adds support for DPDK v21.11, it includes the following
changes.

1. ci: Install python elftools for DPDK 21.02.
2. ci: Update meson requirement for DPDK 21.05.
3. netdev-dpdk: Fix build with 21.05.
4. ci: Compile DPDK in non developer mode.

   http://patchwork.ozlabs.org/project/openvswitch/list/?series=242480=*

5. netdev-dpdk: Remove access to DPDK internals.
6. netdev-dpdk: Remove unused attribute from rte_flow rule.
7. netdev-dpdk: Fix mbuf macros namespace with 21.11-rc1.
8. netdev-dpdk: Fix vhost namespace with 21.11-rc2.

   http://patchwork.ozlabs.org/project/openvswitch/list/?series=271040=*

In addition documentaion and DPDK unit tests were also updated in this
commit for use with DPDK v21.11.

For credit all authors of the original commits to 'dpdk-latest' with the above
changes have been added as co-authors for this commit.

Signed-off-by: David Marchand 
Co-authored-by: David Marchand 
Reviewed-by: Maxime Coquelin 
Signed-off-by: Ian Stokes 

---
v1 -> v2
* Modified openvswitch-fedora.spec.in instead of generated files.

RFC -> V1
* Add telemetry warning to OVS DPDK unit tests.
* Update TSO documentation link to 21.11.
* Update fedora spec to use 21.11.
---
 .ci/linux-build.sh   |   6 +-
 .ci/linux-prepare.sh |   4 +-
 Documentation/faq/releases.rst   |   2 +-
 Documentation/intro/install/dpdk.rst |  16 ++---
 Documentation/topics/dpdk/phy.rst|   8 +--
 Documentation/topics/dpdk/vdev.rst   |   2 +-
 Documentation/topics/dpdk/vhost-user.rst |   2 +-
 Documentation/topics/testing.rst |   2 +-
 Documentation/topics/userspace-tso.rst   |   2 +-
 NEWS |   1 +
 lib/dp-packet.h  |  26 +++
 lib/netdev-dpdk.c| 115 ---
 rhel/openvswitch-fedora.spec.in  |   2 +-
 tests/system-dpdk.at |  16 +++--
 14 files changed, 110 insertions(+), 94 deletions(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 863f02388..ff6ae4b10 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -159,6 +159,10 @@ function install_dpdk()
 # Disable building DPDK unit tests. Not needed for OVS build or tests.
 DPDK_OPTS="$DPDK_OPTS -Dtests=false"
 
+# Disable DPDK developer mode, this results in less build checks and less
+# meson verbose outputs.
+DPDK_OPTS="$DPDK_OPTS -Ddeveloper_mode=disabled"
+
 # Install DPDK using prefix.
 DPDK_OPTS="$DPDK_OPTS --prefix=$(pwd)/build"
 
@@ -216,7 +220,7 @@ fi
 
 if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
 if [ -z "$DPDK_VER" ]; then
-DPDK_VER="20.11.1"
+DPDK_VER="21.11"
 fi
 install_dpdk $DPDK_VER
 if [ "$CC" = "clang" ]; then
diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
index 360c0a68e..b3addf404 100755
--- a/.ci/linux-prepare.sh
+++ b/.ci/linux-prepare.sh
@@ -21,8 +21,8 @@ make -j4 HAVE_LLVM= HAVE_SQLITE= install
 cd ..
 
 pip3 install --disable-pip-version-check --user \
-flake8 hacking sphinx wheel setuptools
-pip3 install --user  'meson==0.47.1'
+flake8 hacking sphinx wheel setuptools pyelftools
+pip3 install --user  'meson==0.49.2'
 
 if [ "$M32" ]; then
 # Installing 32-bit libraries.
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 64bc577e0..59d55202d 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -225,7 +225,7 @@ Q: Are all the DPDK releases that OVS versions work with 
maintained?
 The latest information about DPDK stable and LTS releases can be found
 at `DPDK stable`_.
 
-.. _DPDK stable: http://doc.dpdk.org/guides-20.11/contributing/stable.html
+.. _DPDK stable: http://doc.dpdk.org/guides-21.11/contributing/stable.html
 
 Q: I get an error like this when I configure Open vSwitch:
 
diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index d554409fc..d9f44055d 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -42,7 +42,7 @@ Build requirements
 In addition to the requirements described in :doc:`general`, building Open
 vSwitch with DPDK will require the following:
 
-- DPDK 20.11.1
+- DPDK 21.11
 
 - A `DPDK supported NIC`_
 
@@ -59,8 +59,8 @@ vSwitch with DPDK will require the following:
 
 Detailed system requirements can be found at `DPDK requirements`_.
 
-.. _DPDK supported NIC: https://doc.dpdk.org/guides-20.11/nics/index.html
-.. _DPDK requirements: 
https://doc.dpdk.org/guides-20.11/linux_gsg/sys_reqs.html
+.. _DPDK supported NIC: https://doc.dpdk.org/guides-21.11/nics/index.html
+.. _DPDK requirements: 
https://doc.dpdk.org/guides-21.11/linux_gsg/sys_reqs.html
 
 .. _dpdk-install:
 
@@ -73,9 +73,9 @@ Install DPDK
 #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
 
$ cd /usr/src/
-   $ wget https://fast.dpdk.org/rel/dpdk-20.11.1.tar.xz
-   $ tar xf 

Re: [ovs-dev] [PATCH v3 ovn] ovn-trace: honor ct state in execute_ct_lb

[Dumitru Ceara]

On 12/9/21 14:54, Lorenzo Bianconi wrote:
> When performing CT_LB action in ovn-trace, take into account current
> connection tracking state provided by the user.
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1980702
> Fixes: 8accd26cb2 ("OVN: add CT_LB action to ovn-trace")
> Signed-off-by: Lorenzo Bianconi 
> ---

Looks good to me, thanks!

Acked-by: Dumitru Ceara 

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

Re: [ovs-dev] [PATCH 1/4] netdev-dpdk: Introduce per rxq/txq Vhost-user statistics

[David Marchand]

On Thu, Dec 9, 2021 at 3:51 PM Maxime Coquelin
 wrote:
> On 12/7/21 21:37, David Marchand wrote:
> >> +struct netdev_dpdk_vhost_q_stats {
> >> +uint64_t bytes;
> >> +uint64_t packets;
> >> +uint64_t errors;
> >> +};
> >> +
> >>   /* Custom software stats for dpdk ports */
> >>   struct netdev_dpdk_sw_stats {
> >>   /* No. of retries when unable to transmit. */
> >> @@ -206,6 +213,10 @@ struct netdev_dpdk_sw_stats {
> >>   uint64_t rx_qos_drops;
> >>   /* Packet drops in HWOL processing. */
> >>   uint64_t tx_invalid_hwol_drops;
> >> +/* Per-queue Vhost Tx stats */
> >> +struct netdev_dpdk_vhost_q_stats *txq;
> >> +/* Per-queue Vhost Rx stats */
> >> +struct netdev_dpdk_vhost_q_stats *rxq;
> > |Here, we add "driver" specific stats, while netdev_dpdk_sw_stats struct
> > carries OVS "own" stats. This netdev_dpdk_sw_stats struct is converted
> > by netdev_dpdk_get_sw_custom_stats and there is a small framework on
> > adding custom OVS stats (using some macros "trick"). I'd rather leave
> > netdev_dpdk_sw_stats struct untouched for consistency. Pointers to vhost
> > specific stats can be added to the netdev_dpdk struct (we have some
> > spare space after the pointer to netdev_dpdk_sw_stats). |
>
> I checked with pahole, and the comment stating "/*36 pad bytes here.*/"
> is outdated. Since it is difficult to maintain it given it needs to be
> updated when struct netdev_stats is modified, I will just remove it in
> next revision.

Yeah... I don't like those comments which become obsolete too easily.
+1


-- 
David Marchand

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

Re: [ovs-dev] [PATCH 1/4] netdev-dpdk: Introduce per rxq/txq Vhost-user statistics

[Maxime Coquelin]

On 12/7/21 21:37, David Marchand wrote:

+struct netdev_dpdk_vhost_q_stats {
+uint64_t bytes;
+uint64_t packets;
+uint64_t errors;
+};
+
  /* Custom software stats for dpdk ports */
  struct netdev_dpdk_sw_stats {
  /* No. of retries when unable to transmit. */
@@ -206,6 +213,10 @@ struct netdev_dpdk_sw_stats {
  uint64_t rx_qos_drops;
  /* Packet drops in HWOL processing. */
  uint64_t tx_invalid_hwol_drops;
+/* Per-queue Vhost Tx stats */
+struct netdev_dpdk_vhost_q_stats *txq;
+/* Per-queue Vhost Rx stats */
+struct netdev_dpdk_vhost_q_stats *rxq;
|Here, we add "driver" specific stats, while netdev_dpdk_sw_stats struct 
carries OVS "own" stats. This netdev_dpdk_sw_stats struct is converted 
by netdev_dpdk_get_sw_custom_stats and there is a small framework on 
adding custom OVS stats (using some macros "trick"). I'd rather leave 
netdev_dpdk_sw_stats struct untouched for consistency. Pointers to vhost 
specific stats can be added to the netdev_dpdk struct (we have some 
spare space after the pointer to netdev_dpdk_sw_stats). |


I checked with pahole, and the comment stating "/*36 pad bytes here.*/"
is outdated. Since it is difficult to maintain it given it needs to be
updated when struct netdev_stats is modified, I will just remove it in
next revision.

Maxime

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

Re: [ovs-dev] [PATCH v3] Revert "odp-util: Always report ODP_FIT_TOO_LITTLE for IGMP."

[Aaron Conole]

Mike Pattrick  writes:

> On Wed, 2021-12-01 at 14:46 +0100, Eelco Chaudron wrote:
>> 
>> On 29 Nov 2021, at 22:02, Aaron Conole wrote:
>> 
>> > This reverts commit c645550bb249 ("odp-util: Always report
>> > ODP_FIT_TOO_LITTLE for IGMP.")
>> > 
>> > Always forcing a slow path action can result in some over-broad
>> > flows which swallow all traffic and force them to userspace, as reported
>> > in the thread at
>> > https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387706.html
>> > and at
>> > https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387689.html
>> > 
>> > Revert the ODP_FIT_TOO_LITTLE return for IGMP specifically.
>> > Additionally, remove the userspace wc mask to prevent revalidator from
>> > cycling flows.
>> > 
>> > Fixes: c645550bb249 ("odp-util: Always report ODP_FIT_TOO_LITTLE for 
>> > IGMP.")
>> > Signed-off-by: Aaron Conole 
>> > Acked-by: Eelco Chaudron 
>> 
>> Thanks Aaron for adding the tests!
>> 
>> Acked-by: Eelco Chaudron 
>
> Hello Aaron,
>
> Looks good but I'm having some issues with the "igmp flood for non-snoop 
> enabled" test.
>
> I find that it fails for me a small percentage of the time, The
> following snippet will reproduce this issue:
>
> for i in $(seq 1 100); do make check TESTSUITEFLAGS="2379"; if [[ $?
> -ne 0 ]]; then echo "Test $i failed"; break; fi; done
>
>
> The diff shows:
>
> -recirc_id(0),in_port(1),eth(src=aa:55:aa:55:00:01,dst=01:01:00:0c:29:a0),eth_type(0x0800),ipv4(frag=no),
> actions:100,2
> -recirc_id(0),in_port(2),eth(src=01:01:00:0c:29:a0,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(frag=no),
> actions:1
> +recirc_id(0),in_port(2),eth(src=aa:55:aa:55:00:01,dst=01:01:00:0c:29:a0),eth_type(0x0800),ipv4(frag=no),
> actions:100,1
> +recirc_id(0),in_port(1),eth(src=01:01:00:0c:29:a0,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(frag=no),
> actions:2
>
>
> And the logs show:
>> 2021-12-06T20:32:20.519Z|00053|bridge|INFO|bridge br0: added interface p1 on 
>> port 2
>> 2021-12-06T20:32:20.519Z|00054|bridge|INFO|bridge br0: added interface p0 on 
>> port 1
>
>
> So it seems like the proper port numbers are being assigned, but the
> flow seems to have incorrect port numbers. Not sure what's going on
> there, but I also found that splitting the the "ovs-vsctl add-port"
> invocations into two commands fixed the issue.

That is very strange - I don't see such behavior on the systems I've
tested with (x2 on each system):

  Fedora 35
  RHEL8

And the logs are completely baffling (for example, we are using
netdev/dummy-receive so it should always have the correct port numbers).

Such behavior shouldn't occur, and concerns me. Can you describe your
setup?

> Cheers,
> M
>
>> 
>> ___
>> 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 1/1] docs: add notes on using the Tap Poll Mode Driver

[Ilya Maximets]

On 11/26/21 07:50, Nobuhiro MIKI wrote:
> In Tap PMD, file descriptors for rxq and txq are paired by dup(2) [1].
> Here, rxq and txq are created for the number of PMD threads +1 each. One
> additional fd will be created for keepalive [2]. For example, if the
> number of PMD threads is 4, the number of fd is 5 for rxq, 5 for txq, and
> 1 for keepalive, so a total of 11 fd will be created.
> 
> The user can specify any value as rxq from the CLI, but it must bespecified
> here as rxq = PMD threads count +1. If you forget to specify rxq, or if you
> specify a small value, there will be a queue that will not be watched, and
> some packets will fail to be received.

Hmm.  If DPDK creates more rx queues than it was requested by the application,
that sounds like a DPDK bug to me that should be fixed in DPDK.

Briefly reading the tap_setup_queue() function in the driver, I don't see
it allocating something that wasn't requested.  i.e.  it doesn't seem to
dup all the tx fds, it only dups ones that correspond to the requested
rx queue id.  But if kernel actually sends packets to these tx descriptors
expecting the application to receive them, that's should be fixed in the
driver, e.g. by prohibiting the n_rxq != n_txq configuration.

Changing the n_rxq configuration every time the number of threads changed
in OVS doesn't seem like a viable solution.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1 1/1] dpdk: Update to use DPDK v21.11.

[David Marchand]

On Thu, Dec 9, 2021 at 1:27 PM Ian Stokes  wrote:
>
> This commit adds support for DPDK v21.11, it includes the following
> changes.
>
> 1. ci: Install python elftools for DPDK 21.02.
> 2. ci: Update meson requirement for DPDK 21.05.
> 3. netdev-dpdk: Fix build with 21.05.
> 4. ci: Compile DPDK in non developer mode.
>
>http://patchwork.ozlabs.org/project/openvswitch/list/?series=242480=*
>
> 5. netdev-dpdk: Remove access to DPDK internals.
> 6. netdev-dpdk: Remove unused attribute from rte_flow rule.
> 7. netdev-dpdk: Fix mbuf macros namespace with 21.11-rc1.
> 8. netdev-dpdk: Fix vhost namespace with 21.11-rc2.
>
>http://patchwork.ozlabs.org/project/openvswitch/list/?series=271040=*
>
> In addition documentaion and DPDK unit tests were also updated in this
> commit for use with DPDK v21.11.
>
> For credit all authors of the original commits to 'dpdk-latest' with the above
> changes have been added as co-authors for this commit.
>
> Signed-off-by: David Marchand 
> Co-authored-by: David Marchand 
> Reviewed-by: Maxime Coquelin 
> Signed-off-by: Ian Stokes 
>
> ---
> RFC -> V1
> * Add telemetry warning to OVS DPDK unit tests.
> * Update TSO documentation link to 21.11.
> * Update fedora spec to use 21.11.
> ---
>  .ci/linux-build.sh   |   6 +-
>  .ci/linux-prepare.sh |   4 +-
>  Documentation/faq/releases.rst   |   2 +-
>  Documentation/intro/install/dpdk.rst |  16 +-
>  Documentation/topics/dpdk/phy.rst|   8 +-
>  Documentation/topics/dpdk/vdev.rst   |   2 +-
>  Documentation/topics/dpdk/vhost-user.rst |   2 +-
>  Documentation/topics/testing.rst |   2 +-
>  Documentation/topics/userspace-tso.rst   |   2 +-
>  NEWS |   1 +
>  lib/dp-packet.h  |  26 +-
>  lib/netdev-dpdk.c| 115 +++
>  rhel/openvswitch-fedora.spec | 515 
> +++

You should update rhel/openvswitch-fedora.spec.in not the generated
rhel/openvswitch-fedora.spec.


>  tests/system-dpdk.at |  16 +-
>  14 files changed, 624 insertions(+), 93 deletions(-)
>  create mode 100644 rhel/openvswitch-fedora.spec
>

[snip]


> diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
> index e0e750fde..d0f6ee394 100644
> --- a/tests/system-dpdk.at
> +++ b/tests/system-dpdk.at
> @@ -15,7 +15,8 @@ AT_CHECK([grep "DPDK Enabled - initialized" 
> ovs-vswitchd.log], [], [stdout])
>  OVS_VSWITCHD_STOP(["/Global register is changed during/d
>  /EAL:   Invalid NUMA socket, default to 0/d
>  /EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
> clock cycles !/d
> -/EAL: No free hugepages reported in hugepages-1048576kB/d"])
> +/EAL: No free hugepages reported in hugepages-1048576kB/d
> +/TELEMETRY: No legacy callbacks, legacy socket not created/d"])
>  AT_CLEANUP
>  dnl 
> --
>
> @@ -37,12 +38,12 @@ sleep 2
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("/does not exist. The Open vSwitch kernel module is 
> probably not loaded./d
> +OVS_VSWITCHD_STOP(["/does not exist. The Open vSwitch kernel module is 
> probably not loaded./d
>  /Failed to enable flow control/d
>  /Global register is changed during/d
>  /EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
> clock cycles !/d
>  /EAL: No free hugepages reported in hugepages-1048576kB/d
> -")
> +/TELEMETRY: No legacy callbacks, legacy socket not created/d"])

This log is displayed when DPDK is compiled without libjansson (in
this case, the metrics library register no "legacy" callbacks, hence
the log..).
Ok for waiving it for now.
I'll submit a patch on DPDK side to lower it to INFO level because I
find it useless.


Rest lgtm.

Thanks Ian!


-- 
David Marchand

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

[ovs-dev] [PATCH v3 ovn] ovn-trace: honor ct state in execute_ct_lb

[Lorenzo Bianconi]

When performing CT_LB action in ovn-trace, take into account current
connection tracking state provided by the user.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1980702
Fixes: 8accd26cb2 ("OVN: add CT_LB action to ovn-trace")
Signed-off-by: Lorenzo Bianconi 
---
Changes since v2:
- always run next_ct_state() in execute_ct_lb()

Changes since v1:
- use next_ct_state() and do not access ct state directly
---
 tests/ovn-northd.at   | 58 ---
 utilities/ovn-trace.c |  6 -
 2 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index c4424ab14..839ab62e7 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2916,7 +2916,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb /* default (use --ct to customize) */ {
+output("lsp2");
+};
 };
 };
 ])
@@ -2927,7 +2932,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb /* default (use --ct to customize) */ {
+output("lsp2");
+};
 };
 };
 ])
@@ -2944,7 +2954,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb /* default (use --ct to customize) */ {
+output("lsp2");
+};
 };
 };
 ])
@@ -2955,7 +2970,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb /* default (use --ct to customize) */ {
+output("lsp2");
+};
 };
 };
 ])
@@ -3052,7 +3072,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb /* default (use --ct to customize) */ {
+output("lsp2");
+};
 };
 };
 ])
@@ -3063,7 +3088,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb /* default (use --ct to customize) */ {
+output("lsp2");
+};
 };
 };
 ])
@@ -3080,7 +3110,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb /* default (use --ct to customize) */ {
+output("lsp2");
+};
 };
 };
 ])
@@ -3091,7 +3126,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+ 

Re: [ovs-dev] [PATCH] system-dpdk: Improve Vhost-user ping tests reliability

[Ilya Maximets]

On 11/29/21 16:07, Aaron Conole wrote:
> Maxime Coquelin  writes:
> 
>> Instead of waiting 10 seconds for testpmd to start, this
>> patch makes use of OVS_WAIT_UNTIL() macro to wait for
>> the Virtio device readiness notification in ovs-vswitchd
>> logs.
>>
>> Signed-off-by: Maxime Coquelin 
>> ---
> 
> Acked-by: Aaron Conole 

Thanks!  Applied.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] netdev-vport : Fix userspace tunnel ioctl(SIOCGIFINDEX) info logs.

[Ilya Maximets]

On 12/8/21 03:57, lin huang wrote:
> From: linhuang 
> 
> Userspace tunnel doesn't have a valid device in the kernel. So
> get_ifindex() function (ioctl) always get error during
> adding a port, deleting a port or updating a port status.
> 
> The info log is
> "2021-08-29T09:17:39.830Z|00059|netdev_linux|INFO|ioctl(SIOCGIFINDEX)
> on vxlan_sys_4789 device failed: No such device"
> 
> If there are a lot of userspace tunnel ports on a bridge, the
> iface_refresh_netdev_status() function will spend a lot of time.
> 
> So ignore userspace tunnel port ioctl(SIOCGIFINDEX) operation, just
> return -ENODEV.
> 
> Signed-off-by: Lin Huang 
> Test-by: Mike Pattrick 
> ---
>  lib/netdev-vport.c | 6 ++
>  vswitchd/bridge.c  | 2 ++
>  2 files changed, 8 insertions(+)
> 

Applied.  Thanks!

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH 1/1] dpdk: Update to use DPDK v21.11.

[Stokes, Ian]

> > On Tue, Nov 30, 2021 at 4:54 PM Ian Stokes  wrote:
> > >
> > > This commit adds support for DPDK v21.11, it includes the following
> > > changes.
> > >
> > > 1. ci: Install python elftools for DPDK 21.02.
> > > 2. ci: Update meson requirement for DPDK 21.05.
> > > 3. netdev-dpdk: Fix build with 21.05.
> > > 4. ci: Compile DPDK in non developer mode.
> > >
> > >
> >
> http://patchwork.ozlabs.org/project/openvswitch/list/?series=242480=*
> > >
> > > 5. netdev-dpdk: Remove access to DPDK internals.
> > > 6. netdev-dpdk: Remove unused attribute from rte_flow rule.
> > > 7. netdev-dpdk: Fix mbuf macros namespace with 21.11-rc1.
> > > 8. netdev-dpdk: Fix vhost namespace with 21.11-rc2.
> > >
> > >
> >
> http://patchwork.ozlabs.org/project/openvswitch/list/?series=271040=*
> > >
> > > For credit all authors of the original commits to 'dpdk-latest' with the 
> > > above
> > > changes have been added as co-authors for this commit.
> > >
> > > Signed-off-by: David Marchand 
> > > Co-authored-by: David Marchand 
> > > Signed-off-by: Ian Stokes 
> > > ---
> > >  .ci/linux-build.sh   |   6 +-
> > >  .ci/linux-prepare.sh |   4 +-
> > >  Documentation/faq/releases.rst   |   2 +-
> > >  Documentation/intro/install/dpdk.rst |  16 ++---
> > >  Documentation/topics/dpdk/phy.rst|   8 +--
> > >  Documentation/topics/dpdk/vdev.rst   |   2 +-
> > >  Documentation/topics/dpdk/vhost-user.rst |   2 +-
> > >  Documentation/topics/testing.rst |   2 +-
> >
> > I did a quick pass and caught some small things to fix:
> >
> > - should we list 21.11.x for 2.17.x in Documentation/faq/releases.rst table?
> 
> Good point, in the past we've held off on adding this until the 2.17 branch is
> actually created so I left this out for the moment.
> >
> >
> > - there is one reference to 20.11 documentation in
> > Documentation/topics/userspace-tso.rst:__
> > https://doc.dpdk.org/guides-20.11/nics/overview.html
> >
> Good catch, will fix.
> 
> >
> > - Fedora spec still references 20.11:
> > rhel/openvswitch-fedora.spec.in:BuildRequires: dpdk-devel >= 20.11
> Happy to update this, I think in the past its been carried out by the RH 
> folks.
> 
> >
> >
> > The rest is the same than dpdk-latest branch (with the experimental
> > api build check kept in dpdk-latest only).
> > So lgtm, and with those small things from above fixed, feel free to add:
> 
> So the one thing I spotted was the dpdk unit tests now fail, it seems there 
> is a
> new log we need to track from testpmd I'm guessing to ensure they work, will
> roll this change into the v1 of this patch along with the changes above.
> 
> >
> > Reviewed-by: David Marchand 
> >
> 

Hi David,

Apologies for the delay, very busy this week with the conference etc. I've sent 
a new v1 with the changes above.

http://patchwork.ozlabs.org/project/openvswitch/list/?series=275984

We hit some issues with the unit tests (separate to the changes we discussed) 
which caused some delay, however they seem unrelated to the update itself to 
21.11 and more related to some of the platforms we were testing on so shouldn't 
block 21.11 update, apologies for the delay while investigating this.

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

[ovs-dev] [PATCH v1 1/1] dpdk: Update to use DPDK v21.11.

[Ian Stokes]

This commit adds support for DPDK v21.11, it includes the following
changes.

1. ci: Install python elftools for DPDK 21.02.
2. ci: Update meson requirement for DPDK 21.05.
3. netdev-dpdk: Fix build with 21.05.
4. ci: Compile DPDK in non developer mode.

   http://patchwork.ozlabs.org/project/openvswitch/list/?series=242480=*

5. netdev-dpdk: Remove access to DPDK internals.
6. netdev-dpdk: Remove unused attribute from rte_flow rule.
7. netdev-dpdk: Fix mbuf macros namespace with 21.11-rc1.
8. netdev-dpdk: Fix vhost namespace with 21.11-rc2.

   http://patchwork.ozlabs.org/project/openvswitch/list/?series=271040=*

In addition documentaion and DPDK unit tests were also updated in this
commit for use with DPDK v21.11.

For credit all authors of the original commits to 'dpdk-latest' with the above
changes have been added as co-authors for this commit.

Signed-off-by: David Marchand 
Co-authored-by: David Marchand 
Reviewed-by: Maxime Coquelin 
Signed-off-by: Ian Stokes 

---
RFC -> V1
* Add telemetry warning to OVS DPDK unit tests.
* Update TSO documentation link to 21.11.
* Update fedora spec to use 21.11.
---
 .ci/linux-build.sh   |   6 +-
 .ci/linux-prepare.sh |   4 +-
 Documentation/faq/releases.rst   |   2 +-
 Documentation/intro/install/dpdk.rst |  16 +-
 Documentation/topics/dpdk/phy.rst|   8 +-
 Documentation/topics/dpdk/vdev.rst   |   2 +-
 Documentation/topics/dpdk/vhost-user.rst |   2 +-
 Documentation/topics/testing.rst |   2 +-
 Documentation/topics/userspace-tso.rst   |   2 +-
 NEWS |   1 +
 lib/dp-packet.h  |  26 +-
 lib/netdev-dpdk.c| 115 +++
 rhel/openvswitch-fedora.spec | 515 +++
 tests/system-dpdk.at |  16 +-
 14 files changed, 624 insertions(+), 93 deletions(-)
 create mode 100644 rhel/openvswitch-fedora.spec

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 863f02388..ff6ae4b10 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -159,6 +159,10 @@ function install_dpdk()
 # Disable building DPDK unit tests. Not needed for OVS build or tests.
 DPDK_OPTS="$DPDK_OPTS -Dtests=false"
 
+# Disable DPDK developer mode, this results in less build checks and less
+# meson verbose outputs.
+DPDK_OPTS="$DPDK_OPTS -Ddeveloper_mode=disabled"
+
 # Install DPDK using prefix.
 DPDK_OPTS="$DPDK_OPTS --prefix=$(pwd)/build"
 
@@ -216,7 +220,7 @@ fi
 
 if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
 if [ -z "$DPDK_VER" ]; then
-DPDK_VER="20.11.1"
+DPDK_VER="21.11"
 fi
 install_dpdk $DPDK_VER
 if [ "$CC" = "clang" ]; then
diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
index 360c0a68e..b3addf404 100755
--- a/.ci/linux-prepare.sh
+++ b/.ci/linux-prepare.sh
@@ -21,8 +21,8 @@ make -j4 HAVE_LLVM= HAVE_SQLITE= install
 cd ..
 
 pip3 install --disable-pip-version-check --user \
-flake8 hacking sphinx wheel setuptools
-pip3 install --user  'meson==0.47.1'
+flake8 hacking sphinx wheel setuptools pyelftools
+pip3 install --user  'meson==0.49.2'
 
 if [ "$M32" ]; then
 # Installing 32-bit libraries.
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 64bc577e0..59d55202d 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -225,7 +225,7 @@ Q: Are all the DPDK releases that OVS versions work with 
maintained?
 The latest information about DPDK stable and LTS releases can be found
 at `DPDK stable`_.
 
-.. _DPDK stable: http://doc.dpdk.org/guides-20.11/contributing/stable.html
+.. _DPDK stable: http://doc.dpdk.org/guides-21.11/contributing/stable.html
 
 Q: I get an error like this when I configure Open vSwitch:
 
diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index d554409fc..d9f44055d 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -42,7 +42,7 @@ Build requirements
 In addition to the requirements described in :doc:`general`, building Open
 vSwitch with DPDK will require the following:
 
-- DPDK 20.11.1
+- DPDK 21.11
 
 - A `DPDK supported NIC`_
 
@@ -59,8 +59,8 @@ vSwitch with DPDK will require the following:
 
 Detailed system requirements can be found at `DPDK requirements`_.
 
-.. _DPDK supported NIC: https://doc.dpdk.org/guides-20.11/nics/index.html
-.. _DPDK requirements: 
https://doc.dpdk.org/guides-20.11/linux_gsg/sys_reqs.html
+.. _DPDK supported NIC: https://doc.dpdk.org/guides-21.11/nics/index.html
+.. _DPDK requirements: 
https://doc.dpdk.org/guides-21.11/linux_gsg/sys_reqs.html
 
 .. _dpdk-install:
 
@@ -73,9 +73,9 @@ Install DPDK
 #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
 
$ cd /usr/src/
-   $ wget https://fast.dpdk.org/rel/dpdk-20.11.1.tar.xz
-   $ tar xf dpdk-20.11.1.tar.xz
-   $ 

Re: [ovs-dev] [PATCH net v2 0/3] net/sched: Fix ct zone matching for invalid conntrack state

[Marcelo Ricardo Leitner]

On Thu, Dec 09, 2021 at 09:57:31AM +0200, Paul Blakey wrote:
> Changelog:
>   1->2:
> Cover letter wording
> Added blamed CCs

Thanks.

> 
> Paul Blakey (3):
>   net/sched: Extend qdisc control block with tc control block
>   net/sched: flow_dissector: Fix matching on zone id for invalid conns
>   net: openvswitch: Fix matching zone id for invalid conns arriving from tc

I keep getting surprised by how much metadata we have on CT other than
skb->_nfct. :-)

Reviewed-by: Marcelo Ricardo Leitner 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 ovn] ovn-trace: honor ct state in execute_ct_lb

[Lorenzo Bianconi]

> On 12/9/21 12:16, Lorenzo Bianconi wrote:
> > When performing CT_LB action in ovn-trace, take into account current
> > connection tracking state provided by the user.
> > 
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1980702
> > Fixes: 8accd26cb2 ("OVN: add CT_LB action to ovn-trace")
> > Signed-off-by: Lorenzo Bianconi 
> > ---
> > Changes since v1:
> > - use next_ct_state() and do not access ct state directly
> > ---
> 
> Hi Lorenzo,
> 
> Thanks for v2, I have one more comment below.
> 
> Regards,
> Dumitru
> 
> >  tests/ovn-northd.at   | 56 ---
> >  utilities/ovn-trace.c |  8 ++-
> >  2 files changed, 55 insertions(+), 9 deletions(-)
> > 
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index c4424ab14..8c14bc76a 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -2916,7 +2916,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> > --minimal ls "${flow}"], [0], [dn
> >  # 
> > tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
> >  ct_lb {
> >  ct_lb {
> > -output("lsp2");
> > +reg0[[6]] = 0;
> > +*** chk_lb_hairpin_reply action not implemented;
> > +reg0[[12]] = 0;
> > +ct_lb {
> > +output("lsp2");
> > +};
> >  };
> >  };
> >  ])
> > @@ -2927,7 +2932,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> > --minimal ls "${flow}"], [0], [dn
> >  # 
> > udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
> >  ct_lb {
> >  ct_lb {
> > -output("lsp2");
> > +reg0[[6]] = 0;
> > +*** chk_lb_hairpin_reply action not implemented;
> > +reg0[[12]] = 0;
> > +ct_lb {
> > +output("lsp2");
> > +};
> >  };
> >  };
> >  ])
> > @@ -2944,7 +2954,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> > --minimal ls "${flow}"], [0], [dn
> >  # 
> > tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
> >  ct_lb {
> >  ct_lb {
> > -output("lsp2");
> > +reg0[[6]] = 0;
> > +*** chk_lb_hairpin_reply action not implemented;
> > +reg0[[12]] = 0;
> > +ct_lb {
> > +output("lsp2");
> > +};
> >  };
> >  };
> >  ])
> > @@ -2955,7 +2970,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> > --minimal ls "${flow}"], [0], [dn
> >  # 
> > udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
> >  ct_lb {
> >  ct_lb {
> > -output("lsp2");
> > +reg0[[6]] = 0;
> > +*** chk_lb_hairpin_reply action not implemented;
> > +reg0[[12]] = 0;
> > +ct_lb {
> > +output("lsp2");
> > +};
> >  };
> >  };
> >  ])
> > @@ -3052,7 +3072,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> > --minimal ls "${flow}"], [0], [dn
> >  # 
> > tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
> >  ct_lb {
> >  ct_lb {
> > -output("lsp2");
> > +reg0[[6]] = 0;
> > +*** chk_lb_hairpin_reply action not implemented;
> > +reg0[[12]] = 0;
> > +ct_lb {
> > +output("lsp2");
> > +};
> >  };
> >  };
> >  ])
> > @@ -3063,7 +3088,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> > --minimal ls "${flow}"], [0], [dn
> >  # 
> > udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
> >  ct_lb {
> >  ct_lb {
> > -output("lsp2");
> > +reg0[[6]] = 0;
> > +*** chk_lb_hairpin_reply action not implemented;
> > +reg0[[12]] = 0;
> > +ct_lb {
> > +output("lsp2");
> > +};
> >  };
> >  };
> >  ])
> > @@ -3080,7 +3110,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> > --minimal ls "${flow}"], [0], [dn
> >  # 
> > tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
> >  ct_lb {
> >  ct_lb {
> > -output("lsp2");
> > +reg0[[6]] = 0;
> > +*** chk_lb_hairpin_reply action not implemented;
> > +reg0[[12]] = 0;
> > +ct_lb {
> > +output("lsp2");
> > +};
> >  };
> >  };
> >  ])
> > @@ -3091,7 +3126,12 @@ 

Re: [ovs-dev] [PATCH v2 ovn] ovn-trace: honor ct state in execute_ct_lb

[Dumitru Ceara]

On 12/9/21 12:16, Lorenzo Bianconi wrote:
> When performing CT_LB action in ovn-trace, take into account current
> connection tracking state provided by the user.
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1980702
> Fixes: 8accd26cb2 ("OVN: add CT_LB action to ovn-trace")
> Signed-off-by: Lorenzo Bianconi 
> ---
> Changes since v1:
> - use next_ct_state() and do not access ct state directly
> ---

Hi Lorenzo,

Thanks for v2, I have one more comment below.

Regards,
Dumitru

>  tests/ovn-northd.at   | 56 ---
>  utilities/ovn-trace.c |  8 ++-
>  2 files changed, 55 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index c4424ab14..8c14bc76a 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -2916,7 +2916,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> --minimal ls "${flow}"], [0], [dn
>  # 
> tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
>  ct_lb {
>  ct_lb {
> -output("lsp2");
> +reg0[[6]] = 0;
> +*** chk_lb_hairpin_reply action not implemented;
> +reg0[[12]] = 0;
> +ct_lb {
> +output("lsp2");
> +};
>  };
>  };
>  ])
> @@ -2927,7 +2932,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> --minimal ls "${flow}"], [0], [dn
>  # 
> udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
>  ct_lb {
>  ct_lb {
> -output("lsp2");
> +reg0[[6]] = 0;
> +*** chk_lb_hairpin_reply action not implemented;
> +reg0[[12]] = 0;
> +ct_lb {
> +output("lsp2");
> +};
>  };
>  };
>  ])
> @@ -2944,7 +2954,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> --minimal ls "${flow}"], [0], [dn
>  # 
> tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
>  ct_lb {
>  ct_lb {
> -output("lsp2");
> +reg0[[6]] = 0;
> +*** chk_lb_hairpin_reply action not implemented;
> +reg0[[12]] = 0;
> +ct_lb {
> +output("lsp2");
> +};
>  };
>  };
>  ])
> @@ -2955,7 +2970,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> --minimal ls "${flow}"], [0], [dn
>  # 
> udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
>  ct_lb {
>  ct_lb {
> -output("lsp2");
> +reg0[[6]] = 0;
> +*** chk_lb_hairpin_reply action not implemented;
> +reg0[[12]] = 0;
> +ct_lb {
> +output("lsp2");
> +};
>  };
>  };
>  ])
> @@ -3052,7 +3072,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> --minimal ls "${flow}"], [0], [dn
>  # 
> tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
>  ct_lb {
>  ct_lb {
> -output("lsp2");
> +reg0[[6]] = 0;
> +*** chk_lb_hairpin_reply action not implemented;
> +reg0[[12]] = 0;
> +ct_lb {
> +output("lsp2");
> +};
>  };
>  };
>  ])
> @@ -3063,7 +3088,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> --minimal ls "${flow}"], [0], [dn
>  # 
> udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
>  ct_lb {
>  ct_lb {
> -output("lsp2");
> +reg0[[6]] = 0;
> +*** chk_lb_hairpin_reply action not implemented;
> +reg0[[12]] = 0;
> +ct_lb {
> +output("lsp2");
> +};
>  };
>  };
>  ])
> @@ -3080,7 +3110,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> --minimal ls "${flow}"], [0], [dn
>  # 
> tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
>  ct_lb {
>  ct_lb {
> -output("lsp2");
> +reg0[[6]] = 0;
> +*** chk_lb_hairpin_reply action not implemented;
> +reg0[[12]] = 0;
> +ct_lb {
> +output("lsp2");
> +};
>  };
>  };
>  ])
> @@ -3091,7 +3126,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new 
> --minimal ls "${flow}"], [0], [dn
>  # 
> udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
>  ct_lb {
>  

[ovs-dev] [PATCH v2 ovn] ovn-trace: honor ct state in execute_ct_lb

[Lorenzo Bianconi]

When performing CT_LB action in ovn-trace, take into account current
connection tracking state provided by the user.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1980702
Fixes: 8accd26cb2 ("OVN: add CT_LB action to ovn-trace")
Signed-off-by: Lorenzo Bianconi 
---
Changes since v1:
- use next_ct_state() and do not access ct state directly
---
 tests/ovn-northd.at   | 56 ---
 utilities/ovn-trace.c |  8 ++-
 2 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index c4424ab14..8c14bc76a 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2916,7 +2916,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb {
+output("lsp2");
+};
 };
 };
 ])
@@ -2927,7 +2932,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb {
+output("lsp2");
+};
 };
 };
 ])
@@ -2944,7 +2954,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb {
+output("lsp2");
+};
 };
 };
 ])
@@ -2955,7 +2970,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb {
+output("lsp2");
+};
 };
 };
 ])
@@ -3052,7 +3072,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb {
+output("lsp2");
+};
 };
 };
 ])
@@ -3063,7 +3088,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb {
+output("lsp2");
+};
 };
 };
 ])
@@ -3080,7 +3110,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
tcp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb {
+output("lsp2");
+};
 };
 };
 ])
@@ -3091,7 +3126,12 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal 
ls "${flow}"], [0], [dn
 # 
udp,reg14=0x${lsp1_inport},vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
 ct_lb {
 ct_lb {
-output("lsp2");
+reg0[[6]] = 0;
+*** chk_lb_hairpin_reply action not implemented;
+reg0[[12]] = 0;
+ct_lb {
+output("lsp2");
+};
 };
 };
 ])
diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
index 617ad834c..e99202da6 100644
--- a/utilities/ovn-trace.c
+++ b/utilities/ovn-trace.c
@@ -2351,6 +2351,7 @@ 

Re: [ovs-dev] [PATCH ovn 3/3] northd: support mix of stateless ACL with lower priority than stateful

[Dumitru Ceara]

On 12/1/21 13:56, Vladislav Odintsov wrote:
> It was unsupported - to have a mix of stateless ACLs with lower than
> stateful priority at the same time with stateful (related) ACLs.
> 
> This patch changes stateless ACLs behaviour, but this change should not
> be harmful for users. Likely nobody mixed rules in the described manner,
> so this change should be okay.
> 
> Signed-off-by: Vladislav Odintsov 
> ---

Hi Vladislav,

We also need to update the documentation here:
https://github.com/ovn-org/ovn/blob/a2e7f69d6684766dadfb0a1eb455e123f6bd200a/ovn-nb.xml#L1934

Should this be a "Fixes: 3187b9fef1 ("ovn-northd: introduce new
allow-stateless ACL verb")"?  CC-ing Ihar and Han for more input on this
matter.

I guess this should give more context on the original implementation:
https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/383513.html

However, I think there are some issues with your approach; using the
example Han gave there (adding it here for clarity):

  All traffic from A to B (egress) is stateless:
  ACL1: from-lport 100 'inport == "A" && ip.dst == B' allow-stateless
  
  So does the return traffic:
  ACL2: to-lport 100 'outport == "A" && ip.src == B' allow-stateless
  
  Now we need the traffic initiated from A to B's TCP port 80 to be
  stateful:
  ACL3: from-lport 200 'inport == "A" && ip.dst == B && tcp.dst == 80' 
allow-related

My topology consists of one logical switch and two ports:

  ovn-nbctl ls-add ls
  ovn-nbctl lsp-add ls vm1
  ovn-nbctl lsp-set-addresses vm1 00:00:00:00:00:01

  ip netns add vm1
  ovs-vsctl add-port br-int vm1 -- set interface vm1 type=internal
  ip link set vm1 netns vm1
  ip netns exec vm1 ip link set vm1 address 00:00:00:00:00:01
  ip netns exec vm1 ip addr add 42.42.42.2/24 dev vm1
  ip netns exec vm1 ip link set vm1 up
  ovs-vsctl set Interface vm1 external_ids:iface-id=vm1

  ovn-nbctl lsp-add ls vm2
  ovn-nbctl lsp-set-addresses vm2 00:00:00:00:00:02

  ip netns add vm2
  ovs-vsctl add-port br-int vm2 -- set interface vm2 type=internal
  ip link set vm2 netns vm2
  ip netns exec vm2 ip link set vm2 address 00:00:00:00:00:02
  ip netns exec vm2 ip addr add 42.42.42.3/24 dev vm2
  ip netns exec vm2 ip link set vm2 up
  ovs-vsctl set Interface vm2 external_ids:iface-id=vm2

  ovn-nbctl acl-add ls from-lport 100 'inport == "vm1" && ip4.dst == 
42.42.42.3' allow-stateless
  ovn-nbctl acl-add ls to-lport 100 'outport == "vm1" && ip4.src == 42.42.42.3' 
allow-stateless
  ovn-nbctl acl-add ls from-lport 200 'inport == "vm1" && ip4.dst == 42.42.42.3 
&& tcp.dst == 80' allow-related

I'd expect TCP sessions to always be established with this config and
that is the case without your patch, sessions can be established fine
between A:X -> B:80:
  tcp  6 431998 ESTABLISHED src=42.42.42.2 dst=42.42.42.3 sport=52994 
dport=80 src=42.42.42.3 dst=42.42.42.2 sport=80 dport=52994 [ASSURED] mark=0 
secctx=system_u:object_r:unlabeled_t:s0 zone=1 use=1

While, when applying your patch, the connection is not fully established:
  tcp  6 19 SYN_RECV src=42.42.42.2 dst=42.42.42.3 sport=60730 dport=8080 
src=42.42.42.3 dst=42.42.42.2 sport=8080 dport=60730 mark=0 
secctx=system_u:object_r:unlabeled_t:s0 zone=1 use=1
  tcp  6 79 SYN_SENT src=42.42.42.2 dst=42.42.42.3 sport=60730 dport=8080 
[UNREPLIED] src=42.42.42.3 dst=42.42.42.2 sport=8080 dport=60730 mark=0 
secctx=system_u:object_r:unlabeled_t:s0 zone=3 use=1

Zone 1 is vm1's conntrack zone and zone 3 corresponds to vm2.

>  northd/northd.c | 92 +
>  northd/ovn-northd.8.xml |  4 +-
>  tests/ovn-northd.at | 48 -
>  3 files changed, 87 insertions(+), 57 deletions(-)
> 
> diff --git a/northd/northd.c b/northd/northd.c
> index 2efc4bb1f..1831e6e79 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -592,6 +592,7 @@ struct ovn_datapath {
>  uint32_t port_key_hint;
>  
>  bool has_stateful_acl;
> +bool has_stateless_acl;
>  bool has_lb_vip;
>  bool has_unknown;
>  bool has_acls;
> @@ -5416,15 +5417,26 @@ ls_get_acl_flags(struct ovn_datapath *od)
>  {
>  od->has_acls = false;
>  od->has_stateful_acl = false;
> +od->has_stateless_acl = false;
>  
>  if (od->nbs->n_acls) {
>  od->has_acls = true;
>  
>  for (size_t i = 0; i < od->nbs->n_acls; i++) {
>  struct nbrec_acl *acl = od->nbs->acls[i];
> -if (!strcmp(acl->action, "allow-related")) {
> +if (!od->has_stateful_acl &&
> +!strcmp(acl->action, "allow-related")) {
>  od->has_stateful_acl = true;
> -return;
> +if (od->has_stateless_acl) {
> +return;
> +}
> +}
> +if (!od->has_stateless_acl &&
> +!strcmp(acl->action, "allow-stateless")) {
> +od->has_stateless_acl = true;
> +if (od->has_stateful_acl) {
> +return;
> +   

Re: [ovs-dev] [PATCH 1/4] netdev-dpdk: Introduce per rxq/txq Vhost-user statistics

[Maxime Coquelin]

On 12/8/21 21:25, David Marchand wrote:

On Wed, Nov 24, 2021 at 10:24 PM Maxime Coquelin
 wrote:

@@ -2385,6 +2408,7 @@ netdev_dpdk_vhost_update_rx_counters(struct netdev_dpdk 
*dev,
  }

  stats->rx_bytes += packet_size;
+q_stats->bytes += packet_size;
  }

  if (OVS_UNLIKELY(qos_drops)) {
@@ -2437,7 +2461,7 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
  }

  rte_spinlock_lock(>stats_lock);
-netdev_dpdk_vhost_update_rx_counters(dev, batch->packets,
+netdev_dpdk_vhost_update_rx_counters(dev, qid, batch->packets,


After some testing, instead of qid here, we should use rxq->queue_id :-)
The tx part is ok.


Oops. Yes, it needs to be rxq->queue_id.




   nb_rx, qos_drops);
  rte_spinlock_unlock(>stats_lock);



[snip]


With this fix, I start a vm with a 4 rxq vhost port.
This vhost port has a virtio counterpart bound to the vm virtio-net kmod.

After a while, I can see some stats for rx and tx packets:
# ovs-vsctl get interface vhost4 statistics | sed -e 's#[{}]##g' -e
's#, #\n#g' | grep -v '=0$'
rx_1_to_64_packets=15
rx_256_to_511_packets=25
rx_65_to_127_packets=35
rx_bytes=12185
rx_packets=75
rx_q1_bytes=242
rx_q1_packets=3
rx_q2_bytes=242
rx_q2_packets=3
rx_q3_bytes=11701
rx_q3_packets=69
tx_bytes=2414
tx_packets=20
tx_q0_bytes=2414
tx_q0_packets=20

All good, and per q stats nicely sums to the same value than the "global" stats.

I then stop the vm and look at stats:
# ovs-vsctl get interface vhost4 statistics | sed -e 's#[{}]##g' -e
's#, #\n#g' | grep -v '=0$'
rx_1_to_64_packets=15
rx_256_to_511_packets=26
rx_65_to_127_packets=38
rx_bytes=12776
rx_packets=79
tx_bytes=2414
tx_dropped=1
tx_packets=20
tx_q0_bytes=2414
tx_q0_packets=20


So we end up with misalignment of "global" stats and per q stats in
this situation.
Can we do something about it?

I am unsure about resetting all stats to 0, but it could be the
simpler solution...


I would vote for resetting all counters on number of queues changes,
both per-queue and global, as discussed off-list. That's the only way to
keep consistency between global and per-queue counters.

Maxime





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

Re: [ovs-dev] [PATCH ovn v4] controller/pinctrl: improve packet-in debuggability

[Mohammad Heib]

Great, thanks!

On Thu, Dec 9, 2021 at 1:21 AM Numan Siddique  wrote:

> On Wed, Dec 8, 2021 at 9:00 AM Mohammad Heib  wrote:
> >
> > Improve packet-in debuggability within pinctrl module
> > by printing basic details about each received packet-in
> > message, those messages will be printed to the logs only
> > when DBG log level is enabled.
> >
> > Also, add two coverage counters that will indicate the total
> > packet-in messages that were received and the number of times
> > that the pinctrl main thread was notified to handle a change
> > in the local DBs, those counters can be used by the user as
> > an indicator to enable the DBG logs level and see more details
> > about the received packet-in in the logs.
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1821965
> > Signed-off-by: Mohammad Heib 
>
> Thanks for v4.
>
> Applied to the main branch.
>
> Numan
>
> > ---
> >  controller/pinctrl.c  | 39 ++
> >  include/ovn/actions.h |  1 +
> >  lib/actions.c | 44 +++
> >  tests/ovn.at  |  8 
> >  4 files changed, 92 insertions(+)
> >
> > diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> > index 0d443c150..4ce16ac74 100644
> > --- a/controller/pinctrl.c
> > +++ b/controller/pinctrl.c
> > @@ -364,6 +364,8 @@ COVERAGE_DEFINE(pinctrl_drop_put_mac_binding);
> >  COVERAGE_DEFINE(pinctrl_drop_buffered_packets_map);
> >  COVERAGE_DEFINE(pinctrl_drop_controller_event);
> >  COVERAGE_DEFINE(pinctrl_drop_put_vport_binding);
> > +COVERAGE_DEFINE(pinctrl_notify_main_thread);
> > +COVERAGE_DEFINE(pinctrl_total_pin_pkts);
> >
> >  struct empty_lb_backends_event {
> >  struct hmap_node hmap_node;
> > @@ -3268,6 +3270,41 @@ process_packet_in(struct rconn *swconn, const
> struct ofp_header *msg)
> >   ntohl(ah->opcode));
> >  break;
> >  }
> > +
> > +
> > +if (VLOG_IS_DBG_ENABLED()) {
> > +struct ds pin_str = DS_EMPTY_INITIALIZER;
> > +char * opc_str = ovnact_op_to_string(ntohl(ah->opcode));
> > +
> > +ds_put_format(_str,
> > +"pinctrl received  packet-in | opcode=%s",
> > +opc_str);
> > +
> > +ds_put_format(_str, "| OF_Table_ID=%u", pin.table_id);
> > +ds_put_format(_str, "| OF_Cookie_ID=0x%"PRIx64,
> > +ntohll(pin.cookie));
> > +
> > +if (pin.flow_metadata.flow.in_port.ofp_port) {
> > +ds_put_format(_str, "| in-port=%u",
> > +pin.flow_metadata.flow.in_port.ofp_port);
> > +}
> > +
> > +ds_put_format(_str, "| src-mac="ETH_ADDR_FMT",",
> > +ETH_ADDR_ARGS(headers.dl_src));
> > +ds_put_format(_str, " dst-mac="ETH_ADDR_FMT,
> > +ETH_ADDR_ARGS(headers.dl_dst));
> > +if (headers.dl_type != htons(ETH_TYPE_IPV6)) {
> > +ds_put_format(_str, "| src-ip="IP_FMT",",
> > +IP_ARGS(headers.nw_src));
> > +ds_put_format(_str, " dst-ip="IP_FMT,
> > +IP_ARGS(headers.nw_dst));
> > +}
> > +
> > +VLOG_DBG("%s \n", ds_cstr(_str));
> > +ds_destroy(_str);
> > +free(opc_str);
> > +}
> > +
> >  }
> >
> >  /* Called with in the pinctrl_handler thread context. */
> > @@ -3285,6 +3322,7 @@ pinctrl_recv(struct rconn *swconn, const struct
> ofp_header *oh,
> >  config.miss_send_len = UINT16_MAX;
> >  set_switch_config(swconn, );
> >  } else if (type == OFPTYPE_PACKET_IN) {
> > +COVERAGE_INC(pinctrl_total_pin_pkts);
> >  process_packet_in(swconn, oh);
> >  } else {
> >  if (VLOG_IS_DBG_ENABLED()) {
> > @@ -3309,6 +3347,7 @@ notify_pinctrl_handler(void)
> >  static void
> >  notify_pinctrl_main(void)
> >  {
> > +COVERAGE_INC(pinctrl_notify_main_thread);
> >  seq_change(pinctrl_main_seq);
> >  }
> >
> > diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> > index ede5eb93c..cdef5fb03 100644
> > --- a/include/ovn/actions.h
> > +++ b/include/ovn/actions.h
> > @@ -806,5 +806,6 @@ void ovnacts_encode(const struct ovnact[], size_t
> ovnacts_len,
> >  struct ofpbuf *ofpacts);
> >
> >  void ovnacts_free(struct ovnact[], size_t ovnacts_len);
> > +char *ovnact_op_to_string(uint32_t);
> >
> >  #endif /* ovn/actions.h */
> > diff --git a/lib/actions.c b/lib/actions.c
> > index 6b9a426ae..da00ee349 100644
> > --- a/lib/actions.c
> > +++ b/lib/actions.c
> > @@ -4315,3 +4315,47 @@ ovnacts_free(struct ovnact *ovnacts, size_t
> ovnacts_len)
> >  }
> >  }
> >  }
> > +
> > +/* Return ovn action opcode string representation.
> > + * The returned memory is dynamically allocated
> > + * and the caller must free it using free().
> > + */
> > +
> > +char *
> > +ovnact_op_to_string(uint32_t ovnact_opc)
> > +{
> > +switch (ovnact_opc) {
> > +#define ACTION_OPCODES 

Re: [ovs-dev] [PATCH ovn v3] controller/pinctrl: improve packet-in debuggability

[Mohammad Heib]

On 12/8/21 5:26 PM, Ilya Maximets wrote:

On 12/8/21 15:11, Mohammad Heib wrote:

On 12/6/21 4:08 PM, Ilya Maximets wrote:

On 12/6/21 15:01, Ilya Maximets wrote:

On 12/6/21 14:35, Ilya Maximets wrote:

On 12/6/21 10:09, Mohammad Heib wrote:

Improve packet-in debuggability within pinctrl module
by printing basic details about each received packet-in
message, those messages will be printed to the logs only
when DBG log level is enabled.

Also, add two coverage counters that will indicate the total
packet-in messages that were received and the number of times
that the pinctrl main thread was notified to handle a change
in the local DBs, those counters can be used by the user as
an indicator to enable the DBG logs level and see more details
about the received packet-in in the logs.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1821965
Signed-off-by: Mohammad Heib 
---
  controller/pinctrl.c  |  39 ++
  include/ovn/actions.h |   1 +
  lib/actions.c | 123 ++
  tests/ovn.at  |   8 +++
  4 files changed, 171 insertions(+)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 0d443c150..4ce16ac74 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -364,6 +364,8 @@ COVERAGE_DEFINE(pinctrl_drop_put_mac_binding);
  COVERAGE_DEFINE(pinctrl_drop_buffered_packets_map);
  COVERAGE_DEFINE(pinctrl_drop_controller_event);
  COVERAGE_DEFINE(pinctrl_drop_put_vport_binding);
+COVERAGE_DEFINE(pinctrl_notify_main_thread);
+COVERAGE_DEFINE(pinctrl_total_pin_pkts);
  
  struct empty_lb_backends_event {

  struct hmap_node hmap_node;
@@ -3268,6 +3270,41 @@ process_packet_in(struct rconn *swconn, const struct 
ofp_header *msg)
   ntohl(ah->opcode));
  break;
  }
+
+
+if (VLOG_IS_DBG_ENABLED()) {
+struct ds pin_str = DS_EMPTY_INITIALIZER;
+char * opc_str = ovnact_op_to_string(ntohl(ah->opcode));
+
+ds_put_format(_str,
+"pinctrl received  packet-in | opcode=%s",
+opc_str);
+
+ds_put_format(_str, "| OF_Table_ID=%u", pin.table_id);
+ds_put_format(_str, "| OF_Cookie_ID=0x%"PRIx64,
+ntohll(pin.cookie));
+
+if (pin.flow_metadata.flow.in_port.ofp_port) {
+ds_put_format(_str, "| in-port=%u",
+pin.flow_metadata.flow.in_port.ofp_port);
+}
+
+ds_put_format(_str, "| src-mac="ETH_ADDR_FMT",",
+ETH_ADDR_ARGS(headers.dl_src));
+ds_put_format(_str, " dst-mac="ETH_ADDR_FMT,
+ETH_ADDR_ARGS(headers.dl_dst));
+if (headers.dl_type != htons(ETH_TYPE_IPV6)) {
+ds_put_format(_str, "| src-ip="IP_FMT",",
+IP_ARGS(headers.nw_src));
+ds_put_format(_str, " dst-ip="IP_FMT,
+IP_ARGS(headers.nw_dst));
+}
+
+VLOG_DBG("%s \n", ds_cstr(_str));
+ds_destroy(_str);
+free(opc_str);
+}
+
  }
  
  /* Called with in the pinctrl_handler thread context. */

@@ -3285,6 +3322,7 @@ pinctrl_recv(struct rconn *swconn, const struct 
ofp_header *oh,
  config.miss_send_len = UINT16_MAX;
  set_switch_config(swconn, );
  } else if (type == OFPTYPE_PACKET_IN) {
+COVERAGE_INC(pinctrl_total_pin_pkts);
  process_packet_in(swconn, oh);
  } else {
  if (VLOG_IS_DBG_ENABLED()) {
@@ -3309,6 +3347,7 @@ notify_pinctrl_handler(void)
  static void
  notify_pinctrl_main(void)
  {
+COVERAGE_INC(pinctrl_notify_main_thread);
  seq_change(pinctrl_main_seq);
  }
  
diff --git a/include/ovn/actions.h b/include/ovn/actions.h

index ede5eb93c..cdef5fb03 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -806,5 +806,6 @@ void ovnacts_encode(const struct ovnact[], size_t 
ovnacts_len,
  struct ofpbuf *ofpacts);
  
  void ovnacts_free(struct ovnact[], size_t ovnacts_len);

+char *ovnact_op_to_string(uint32_t);
  
  #endif /* ovn/actions.h */

diff --git a/lib/actions.c b/lib/actions.c
index 6b9a426ae..a0ff0cb6a 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -4315,3 +4315,126 @@ ovnacts_free(struct ovnact *ovnacts, size_t ovnacts_len)
  }
  }
  }
+
+/* Return ovn action opcode string representation.
+ * The returned memory is dynamically allocated
+ * and the caller must free it using free().
+ */
+
+char *
+ovnact_op_to_string(uint32_t ovnact_opc)
+{
+struct ds opc_str = DS_EMPTY_INITIALIZER;
+
+switch (ovnact_opc) {
+case ACTION_OPCODE_ARP:
+ds_put_cstr(_str, "ARP");
+break;
+case ACTION_OPCODE_IGMP:
+ds_put_cstr(_str, "IGMP");
+break;
+
+case ACTION_OPCODE_PUT_ARP:
+ds_put_cstr(_str, "PUT_ARP");
+break;
+
+case ACTION_OPCODE_PUT_DHCP_OPTS:
+ds_put_cstr(_str, "PUT_DHCP_OPTS");
+break;
+
+case ACTION_OPCODE_ND_NA:
+

Re: [ovs-dev] [PATCH 0/4] dpif-netdev: Hash-based Tx packet steering

[Maxime Coquelin]

Hi Ilya,

On 12/8/21 00:23, Ilya Maximets wrote:

On 11/24/21 22:23, Maxime Coquelin wrote:

This series introduces a new HXPS Tx mode alognside existing
XPS and static modes. The goal is to provide a mode where all
the transmit queues are used, whatever the number of PMD
threads. This may be used with Vhost-user ports, where the
guest application driving the Virtio device expects packets
to be distributed on all the queues.

As a preliminary step, in order to be able to validate the
feature at OVS level, the first patch introduces per-queue
basic statistics for Vhost-user ports. This patch is
complementary to David's patch [0] adding per-queue
statistics to DPDK ports using xstats.

The series also introduces two DPDK tests for Vhost-user
multiqueue, with and without HXPS enabled.

Maxime Coquelin (4):
   netdev-dpdk: Introduce per rxq/txq Vhost-user statistics
   dpif-netdev: Introduce Tx queue mode
   dpif-netdev: Add HXPS Tx queue mode
   system-dpdk: Add tests for HXPS

  Documentation/automake.mk   |   1 +
  Documentation/topics/dpdk/hxps.rst  |  51 ++
  Documentation/topics/dpdk/index.rst |   1 +
  lib/dpif-netdev.c   |  95 ++
  lib/netdev-dpdk.c   | 143 ++--
  tests/system-dpdk.at| 135 ++
  6 files changed, 399 insertions(+), 27 deletions(-)
  create mode 100644 Documentation/topics/dpdk/hxps.rst



Hi, Maxime.  Thanks for working on this.
I agree that this feature might be very useful for some deployments.

I didn't read the code any carefully, just glanced at it.  But I have
a couple of high level comments:

1. I don't think that the test for the actual XPS mode implementation
should be part of a system-dpdk testsuite, as it's not actually
related to DPDK and doesn't require any system HW/ports/non-OVS
applications running in order to test it.

So, I think, that we should be able to do practically the same
testing, but with dummy interfaces, with a test placed in the
dpif-netdev.at or pmd.at (probably, latter).

To achieve that you'll need per-queue stats in netdev-dummy, but
implementation of these will be practically the same or even a
bit simpler as you did for vhost-user ports.

Per-queue stats for vhost-user ports might be good to have in
general, so that patch along with some simple test in system-dpdk.at
for them could be split from this patch set and sent separately.
Or dropped, if you think they are not valuable (?).


OK, I will work on adding per-queue stats to netdev-dummy. I agree it is
better to decouple the testing of this feature from DPDK, and it will
make the test being executed more often.


2. The test itself doesn't need a packet generator script, AFAICT.
You may just send some number of packets via dummy port changing
source or destination udp/tcp port affecting the packet hash this
way.  For example, see the SEND_TCP_BOND_PKTS macro in the
tests/ofproto-dpif.at and how bonding rebalancing tests are using it.


Thanks for the pointer on SEND_TCP_BOND_PKTS, I agree this is better
this way. Also, using fuzzing as I did may have the drawback of seldomly
generate packets with hashes that would make them all ending into a
single queue.


3. Might be better instead of introduction of a specialized config
knob (other_config:hxps=true), to have a multi-choice knob like
other_config:xps-mode with 2 options 'default' and 'hash', where
'default' is a current way of tx queue distribution and it will
be a default value.  'hash' will be a new mode that uses packet
hash to choose the tx queue (what's implemented in this patch set).


Ok, makes sense.


What do you think?


Those are valid points, thanks for your suggestions.
Maxime


Best regards, Ilya Maximets.



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