[ovs-dev] [PATCH ovn] ovn-controller: Add command debug/dump-lflow-conj-ids.

2021-12-02 Thread Han Zhou
Add the debug command in case lflow conjunction ids mapping needs to be
checked during trouble shooting. Also use the dump function in test
cases.

An example output:

$ ovn-appctl -t ovn-controller debug/dump-lflow-conj-ids
Conjunction IDs allocations:
lflow: 4445d4a0-3ca3-4385-a677-c0199d65ea4d, start: 1145427104, n: 1
lflow: 711b0d24-52bc-41a4-9af2-5cdea2d5167d, start: 1897598244, n: 1
lflow: 6e712ecb-f863-4a0c-a7c4-367d59914d35, start: 1852911307, n: 1

Total 3 IDs used.

Suggested-by: Numan Siddique 
Signed-off-by: Han Zhou 
---
 controller/lflow-conj-ids.c  | 29 +
 controller/lflow-conj-ids.h  |  2 ++
 controller/ovn-controller.c  | 15 +
 controller/test-lflow-conj-ids.c |  5 +
 tests/ovn-lflow-conj-ids.at  | 37 
 5 files changed, 88 insertions(+)

diff --git a/controller/lflow-conj-ids.c b/controller/lflow-conj-ids.c
index fc33c25e5..bfe63862a 100644
--- a/controller/lflow-conj-ids.c
+++ b/controller/lflow-conj-ids.c
@@ -195,6 +195,35 @@ void lflow_conj_ids_clear(struct conj_ids *conj_ids) {
 lflow_conj_ids_init(conj_ids);
 }
 
+void
+lflow_conj_ids_dump(struct conj_ids *conj_ids, struct ds *out_data)
+{
+struct lflow_conj_node *lflow_conj;
+size_t count = 0;
+
+ds_put_cstr(out_data, "Conjunction IDs allocations:\n");
+HMAP_FOR_EACH (lflow_conj, hmap_node, _ids->lflow_conj_ids) {
+bool has_conflict =
+(lflow_conj->start_conj_id != lflow_conj->lflow_uuid.parts[0]);
+ds_put_format(out_data, "lflow: "UUID_FMT", start: %"PRIu32
+  ", n: %"PRIu32"%s\n",
+  UUID_ARGS(_conj->lflow_uuid),
+  lflow_conj->start_conj_id,
+  lflow_conj->n_conjs,
+  has_conflict ? " (*)" : "");
+count += lflow_conj->n_conjs;
+}
+
+ds_put_cstr(out_data, "---\n");
+ds_put_format(out_data, "Total %"PRIuSIZE" IDs used.\n", count);
+
+size_t allocated = hmap_count(_ids->conj_id_allocations);
+if (count != allocated) {
+ds_put_format(out_data, "WARNING: mismatch - %"PRIuSIZE" allocated\n",
+  allocated);
+}
+}
+
 /* Insert n_conjs conjuntion ids starting from start_conj_id into the conj_ids,
  * assuming the ids are confirmed to be available. */
 static void
diff --git a/controller/lflow-conj-ids.h b/controller/lflow-conj-ids.h
index d333fa8d5..6da0a612c 100644
--- a/controller/lflow-conj-ids.h
+++ b/controller/lflow-conj-ids.h
@@ -17,6 +17,7 @@
 #ifndef LFLOW_CONJ_IDS_H
 #define LFLOW_CONJ_IDS_H 1
 
+#include "openvswitch/dynamic-string.h"
 #include "openvswitch/hmap.h"
 #include "uuid.h"
 
@@ -37,5 +38,6 @@ void lflow_conj_ids_free(struct conj_ids *, const struct uuid 
*lflow_uuid);
 void lflow_conj_ids_init(struct conj_ids *);
 void lflow_conj_ids_destroy(struct conj_ids *);
 void lflow_conj_ids_clear(struct conj_ids *);
+void lflow_conj_ids_dump(struct conj_ids *, struct ds *out_data);
 
 #endif /* controller/lflow-conj-ids.h */
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 26593bc0d..5fc90a34a 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -89,6 +89,7 @@ static unixctl_cb_func debug_pause_execution;
 static unixctl_cb_func debug_resume_execution;
 static unixctl_cb_func debug_status_execution;
 static unixctl_cb_func debug_dump_local_bindings;
+static unixctl_cb_func debug_dump_conj_ids;
 static unixctl_cb_func lflow_cache_flush_cmd;
 static unixctl_cb_func lflow_cache_show_stats_cmd;
 static unixctl_cb_func debug_delay_nb_cfg_report;
@@ -3428,6 +3429,10 @@ main(int argc, char *argv[])
  debug_dump_local_bindings,
  _data->lbinding_data);
 
+unixctl_command_register("debug/dump-conj-ids", "", 0, 0,
+ debug_dump_conj_ids,
+ _output_data->conj_ids);
+
 unsigned int ovs_cond_seqno = UINT_MAX;
 unsigned int ovnsb_cond_seqno = UINT_MAX;
 unsigned int ovnsb_expected_cond_seqno = UINT_MAX;
@@ -4256,3 +4261,13 @@ debug_dump_local_bindings(struct unixctl_conn *conn, int 
argc OVS_UNUSED,
 unixctl_command_reply(conn, ds_cstr(_data));
 ds_destroy(_data);
 }
+
+static void
+debug_dump_conj_ids(struct unixctl_conn *conn, int argc OVS_UNUSED,
+const char *argv[] OVS_UNUSED, void *conj_ids)
+{
+struct ds conj_ids_dump = DS_EMPTY_INITIALIZER;
+lflow_conj_ids_dump(conj_ids, _ids_dump);
+unixctl_command_reply(conn, ds_cstr(_ids_dump));
+ds_destroy(_ids_dump);
+}
diff --git a/controller/test-lflow-conj-ids.c b/controller/test-lflow-conj-ids.c
index 1273f9a4c..55eb3c7b6 100644
--- a/controller/test-lflow-conj-ids.c
+++ b/controller/test-lflow-conj-ids.c
@@ -106,6 +106,11 @@ test_conj_ids_operations(struct ovs_cmdl_context *ctx)
 goto done;
 }
 }
+struct ds conj_ids_dump = DS_EMPTY_INITIALIZER;
+   

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

2021-12-02 Thread Pravin Shelar
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)) ||
> > > +   dest_options_header_count >= 2) {
> > > +   *ext_hdrs |= OFPIEH12_UNSEQ;
> > > +   }
> > > +   *ext_hdrs |= OFPIEH12_ESP;
> > > +   break;
> > you need to check_header() before looking into each extension header.
>
> Could you elaborate why I need to add check_header(),
> skb_header_pointer() is doing sanitization.

I mean check_header() would allow you to read the header without
copying the bits, it is used in ovs flow extraction so its usual
check.
___
dev 

Re: [ovs-dev] [PATCH 1/1] python: Add cooperative_yield() API method to Idl.

2021-12-02 Thread Mike Pattrick
Hello Terry,

On Thu, Dec 2, 2021 at 3:52 PM Terry Wilson  wrote:
>
> On Thu, Dec 2, 2021 at 2:38 PM 0-day Robot  wrote:
> >
> > Bleep bloop.  Greetings Terry Wilson, 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: Failed to merge in the changes.
>
> Not quite sure what the problem is here. I made the patch against a
> fresh checkout of branch-2.15. I also downloaded the patch from
> patchwork and applied it to a fresh checkout of branch-2.15 with no
> errors.

I believe it's expecting a subject like:

[PATCH branch-2.15 1/1] python: Add cooperative_yield() API method to Idl.


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

Cheers,
M

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


[ovs-dev] [PATCH 2/2] netdev-dpdk: Expose per rxq/txq basic statistics.

2021-12-02 Thread David Marchand
When troubleshooting multiqueue setups, having per queue statistics helps
checking packets repartition in rx and tx queues.

Per queue statistics are exported by most DPDK drivers (with capability
RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS).
OVS only filters DPDK statistics, there is nothing to request in DPDK API.
So the only change is to extend the filter on xstats.

Querying statistics with
$ ovs-vsctl get interface dpdk0 statistics |
  sed -e 's#[{}]##g' -e 's#, #\n#g'

and comparing gives:
@@ -13,7 +13,12 @@
 rx_phy_crc_errors=0
 rx_phy_in_range_len_errors=0
 rx_phy_symbol_errors=0
+rx_q0_bytes=0
 rx_q0_errors=0
+rx_q0_packets=0
+rx_q1_bytes=0
 rx_q1_errors=0
+rx_q1_packets=0
 rx_wqe_errors=0
 tx_broadcast_packets=0
 tx_bytes=0
@@ -27,3 +32,13 @@
 tx_pp_rearm_queue_errors=0
 tx_pp_timestamp_future_errors=0
 tx_pp_timestamp_past_errors=0
+tx_q0_bytes=0
+tx_q0_packets=0
+tx_q1_bytes=0
+tx_q1_packets=0
+tx_q2_bytes=0
+tx_q2_packets=0
+tx_q3_bytes=0
+tx_q3_packets=0
+tx_q4_bytes=0
+tx_q4_packets=0

Signed-off-by: David Marchand 
Reviewed-by: Maxime Coquelin 
---
Changes since RFC:
- dropped regex and used simpler string manipulations,

---
 lib/netdev-dpdk.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 51bb41551b..1e6079d544 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1582,6 +1582,16 @@ netdev_dpdk_get_xstat_name(struct netdev_dpdk *dev, 
uint64_t id)
 return dev->rte_xstats_names[id].name;
 }
 
+static bool
+is_queue_stat(const char *s)
+{
+uint16_t tmp;
+
+return (s[0] == 'r' || s[0] == 't') &&
+(ovs_scan(s + 1, "x_q%"SCNu16"_packets", ) ||
+ ovs_scan(s + 1, "x_q%"SCNu16"_bytes", ));
+}
+
 static void
 netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
 OVS_REQUIRES(dev->mutex)
@@ -1632,9 +1642,10 @@ netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
 id = rte_xstats[i].id;
 name = netdev_dpdk_get_xstat_name(dev, id);
 
-/* We need to filter out everything except dropped, error and
- * management counters. */
-if (string_ends_with(name, "_errors") ||
+/* For custom stats, we filter out everything except per rxq/txq basic
+ * stats, and dropped, error and management counters. */
+if (is_queue_stat(name) ||
+string_ends_with(name, "_errors") ||
 strstr(name, "_management_") ||
 string_ends_with(name, "_dropped")) {
 
-- 
2.23.0

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


[ovs-dev] [PATCH 1/2] netdev-dpdk: Fix statistics when changing Rx/Tx queues count.

2021-12-02 Thread David Marchand
When changing number of Rx or Tx queues, per queue basic stats can be
renumbered in DPDK ethdev layer [1].

OVS maintains an internal xstats IDs cache that was refreshed when a
cached id was not valid anymore (in netdev_dpdk_get_custom_stats) or if
a new DPDK port was created.
This did not handle changes of Rx/Tx queues count.

For example, with a mlx5 port:
$ ovs-vsctl set interface dpdk0 options:n_rxq=2
$ ovs-vsctl get interface dpdk0 statistics |
  sed -e 's#[{}]##g' -e 's#, #\n#g' |
  grep rx_q._errors
rx_q0_errors=0

Move the cache filling after reconfiguring and starting the port.
There is no need to flush the cache in netdev_dpdk_get_custom_stats.

While at it, the xstats code can be cleaned up:
- remove wrong or Lapalissade comments,
- don't check x*alloc return value,
- expect that consecutive calls to xstats API return the same number of
  elements,
- only write to dev-> when all DPDK calls succeeded,
- add missing lock annotations to netdev_dpdk_clear_xstats and
  netdev_dpdk_get_xstat_name,

1: https://git.dpdk.org/dpdk/tree/lib/librte_ethdev/rte_ethdev.c?h=v20.11#n2696

Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2021-November/389456.html
Signed-off-by: David Marchand 
---
 lib/netdev-dpdk.c | 160 ++
 1 file changed, 62 insertions(+), 98 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ca92c947a2..51bb41551b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -540,6 +540,7 @@ static void netdev_dpdk_vhost_destruct(struct netdev 
*netdev);
 
 static int netdev_dpdk_get_sw_custom_stats(const struct netdev *,
struct netdev_custom_stats *);
+static void netdev_dpdk_configure_xstats(struct netdev_dpdk *dev);
 static void netdev_dpdk_clear_xstats(struct netdev_dpdk *dev);
 
 int netdev_dpdk_get_vid(const struct netdev_dpdk *dev);
@@ -1161,6 +1162,8 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 }
 dev->started = true;
 
+netdev_dpdk_configure_xstats(dev);
+
 rte_eth_promiscuous_enable(dev->port_id);
 rte_eth_allmulticast_enable(dev->port_id);
 
@@ -1559,23 +1562,19 @@ netdev_dpdk_dealloc(struct netdev *netdev)
 
 static void
 netdev_dpdk_clear_xstats(struct netdev_dpdk *dev)
+OVS_REQUIRES(dev->mutex)
 {
-/* If statistics are already allocated, we have to
- * reconfigure, as port_id could have been changed. */
-if (dev->rte_xstats_names) {
-free(dev->rte_xstats_names);
-dev->rte_xstats_names = NULL;
-dev->rte_xstats_names_size = 0;
-}
-if (dev->rte_xstats_ids) {
-free(dev->rte_xstats_ids);
-dev->rte_xstats_ids = NULL;
-dev->rte_xstats_ids_size = 0;
-}
+free(dev->rte_xstats_names);
+dev->rte_xstats_names = NULL;
+dev->rte_xstats_names_size = 0;
+free(dev->rte_xstats_ids);
+dev->rte_xstats_ids = NULL;
+dev->rte_xstats_ids_size = 0;
 }
 
-static const char*
+static const char *
 netdev_dpdk_get_xstat_name(struct netdev_dpdk *dev, uint64_t id)
+OVS_REQUIRES(dev->mutex)
 {
 if (id >= dev->rte_xstats_names_size) {
 return "UNKNOWN";
@@ -1583,101 +1582,70 @@ netdev_dpdk_get_xstat_name(struct netdev_dpdk *dev, 
uint64_t id)
 return dev->rte_xstats_names[id].name;
 }
 
-static bool
+static void
 netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
 OVS_REQUIRES(dev->mutex)
 {
+struct rte_eth_xstat_name *rte_xstats_names = NULL;
+struct rte_eth_xstat *rte_xstats = NULL;
+int rte_xstats_names_size;
 int rte_xstats_len;
-bool ret;
-struct rte_eth_xstat *rte_xstats;
-uint64_t id;
-int xstats_no;
 const char *name;
+uint64_t id;
 
-/* Retrieving all XSTATS names. If something will go wrong
- * or amount of counters will be equal 0, rte_xstats_names
- * buffer will be marked as NULL, and any further xstats
- * query won't be performed (e.g. during netdev_dpdk_get_stats
- * execution). */
+netdev_dpdk_clear_xstats(dev);
 
-ret = false;
-rte_xstats = NULL;
+rte_xstats_names_size = rte_eth_xstats_get_names(dev->port_id, NULL, 0);
+if (rte_xstats_names_size < 0) {
+VLOG_WARN("Cannot get XSTATS names for port: "DPDK_PORT_ID_FMT,
+  dev->port_id);
+goto out;
+}
 
-if (dev->rte_xstats_names == NULL || dev->rte_xstats_ids == NULL) {
-dev->rte_xstats_names_size =
-rte_eth_xstats_get_names(dev->port_id, NULL, 0);
+rte_xstats_names = xcalloc(rte_xstats_names_size,
+   sizeof *rte_xstats_names);
+rte_xstats_len = rte_eth_xstats_get_names(dev->port_id,
+  rte_xstats_names,
+  rte_xstats_names_size);
+if (rte_xstats_len < 0 || rte_xstats_len != rte_xstats_names_size) {
+VLOG_WARN("Cannot get XSTATS names for port: "DPDK_PORT_ID_FMT,
+  dev->port_id);
+goto 

Re: [ovs-dev] [PATCH 1/1] python: Add cooperative_yield() API method to Idl.

2021-12-02 Thread Terry Wilson
On Thu, Dec 2, 2021 at 2:38 PM 0-day Robot  wrote:
>
> Bleep bloop.  Greetings Terry Wilson, 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: Failed to merge in the changes.

Not quite sure what the problem is here. I made the patch against a
fresh checkout of branch-2.15. I also downloaded the patch from
patchwork and applied it to a fresh checkout of branch-2.15 with no
errors.

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


Re: [ovs-dev] [PATCH 1/1] python: Add cooperative_yield() API method to Idl.

2021-12-02 Thread 0-day Robot
Bleep bloop.  Greetings Terry Wilson, 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: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 python: Add cooperative_yield() API method to Idl.
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 1/1] python: Add cooperative_yield() API method to Idl.

2021-12-02 Thread Terry Wilson
When using eventlet monkey_patch()'d code, greenthreads can be
blocked on connection for several seconds while the database
contents are parsed. Eventlet recommends adding a sleep(0) call
to cooperatively yield in cpu-bound code. asyncio code has
asyncio.sleep(0). This patch adds an API  method that defaults to
doing nothing, but can be overridden to yield as needed.

Signed-off-by: Terry Wilson 
(cherry picked from commit d28c5ca57650d6866453d0adb9a2e048cda21a86)
---
 NEWS |  6 ++
 python/ovs/db/idl.py | 11 +++
 2 files changed, 17 insertions(+)

diff --git a/NEWS b/NEWS
index d0f0cc8d8..a8ab1aa8b 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,11 @@
 v2.15.3 - xx xxx 
 -
+   - The Python Idl class now has a cooperative_yield() method that can be
+ overridden by an application that uses eventlet / gevent / asyncio with
+ the desired yield method (e.g. {eventlet,gevent,asyncio}.sleep(0)) to
+ prevent the application from being blocked for a long time while
+ processing database updates.
+
 
 v2.15.2 - 21 Oct 2021
 -
diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index 3ca47f96b..7ecaeee6d 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -491,6 +491,15 @@ class Idl(object):
 :type updates:  Row
 """
 
+def cooperative_yield(self):
+"""Hook for cooperatively yielding to eventlet/gevent/asyncio/etc.
+
+When a block of code is going to spend a lot of time cpu-bound without
+doing any I/O, it can cause greenthread/coroutine libraries to block.
+This call should be added to code where this can happen, but defaults
+to doing nothing to avoid overhead where it is not needed.
+"""
+
 def __send_cond_change(self, table, cond):
 monitor_cond_change = {table.name: [{"where": cond}]}
 old_uuid = str(self.uuid)
@@ -656,6 +665,8 @@ class Idl(object):
   'is not an object'
   % (table_name, uuid_string))
 
+self.cooperative_yield()
+
 if version == OVSDB_UPDATE2:
 changes = self.__process_update2(table, uuid, row_update)
 if changes:
-- 
2.31.1

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


[ovs-dev] [PATCH 0/1] python: Backport cooperative_yield addition

2021-12-02 Thread Terry Wilson
This is a backport of the cooperative yield addition. By default it
does nothing without a user of the library overriding the method,
but it does allow fixing some issues for greenthread users. If
possible, I'd like to have it down to 2.13. It's simple enough, it
should backport pretty cleanly excpet the NEWS file.

Terry Wilson (1):
  python: Add cooperative_yield() API method to Idl.

 NEWS |  6 ++
 python/ovs/db/idl.py | 11 +++
 2 files changed, 17 insertions(+)

-- 
2.31.1

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


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

2021-12-02 Thread Cpp Code
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)) 
> > ||
> > +   dest_options_header_count >= 2) {
> > +   *ext_hdrs |= OFPIEH12_UNSEQ;
> > +   }
> > +   *ext_hdrs |= OFPIEH12_ESP;
> > +   break;
> you need to check_header() before looking into each extension header.

Could you elaborate why I need to add check_header(),
skb_header_pointer() is doing sanitization.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] net: openvswitch: Remove redundant if statements

2021-12-02 Thread Xu Wang
The 'if (dev)' statement already move into dev_{put , hold}, so remove
redundant if statements.

Signed-off-by: Xu Wang 
---
 net/openvswitch/vport-netdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
index 8e1a88f13622..c1ad6699b1f8 100644
--- a/net/openvswitch/vport-netdev.c
+++ b/net/openvswitch/vport-netdev.c
@@ -137,8 +137,7 @@ static void vport_netdev_free(struct rcu_head *rcu)
 {
struct vport *vport = container_of(rcu, struct vport, rcu);
 
-   if (vport->dev)
-   dev_put(vport->dev);
+   dev_put(vport->dev);
ovs_vport_free(vport);
 }
 
-- 
2.25.1

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


Re: [ovs-dev] [PATCH v2 6/8] dpif-netdev: Add configure to enable autovalidator at build time.

2021-12-02 Thread 0-day Robot
Bleep bloop.  Greetings Emma Finn, 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.


checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Emma Finn 
Lines checked: 78, Warnings: 1, Errors: 0


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


Re: [ovs-dev] [PATCH v2 5/8] pmd.at: Add test-cases for ovs-actions commands.

2021-12-02 Thread 0-day Robot
Bleep bloop.  Greetings Emma Finn, 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.


checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Emma Finn 
Lines checked: 51, Warnings: 1, Errors: 0


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 v2 8/8] odp-execute: Add ISA implementation of pop_vlan action.

2021-12-02 Thread Emma Finn
This commit adds the AVX512 implementation of the pop_vlan action.
The implementation here is auto-validated by the miniflow
extract autovalidator, hence its correctness can be easily
tested and verified.

Signed-off-by: Emma Finn 

---
v2:
- Refactor to fix build warnings
---
 lib/odp-execute-avx512.c  | 77 ++-
 lib/odp-execute-private.c |  2 +-
 lib/odp-execute-private.h |  2 +-
 3 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c
index 0765b8e3d..7a21a60b1 100644
--- a/lib/odp-execute-avx512.c
+++ b/lib/odp-execute-avx512.c
@@ -14,6 +14,11 @@
  * limitations under the License.
  */
 
+#ifdef __x86_64__
+/* Sparse cannot handle the AVX512 instructions. */
+#if !defined(__CHECKER__)
+
+
 #include 
 #include 
 
@@ -25,6 +30,71 @@
 
 #include "immintrin.h"
 
+VLOG_DEFINE_THIS_MODULE(odp_execute_avx512);
+BUILD_ASSERT_DECL(offsetof(struct dp_packet, l2_5_ofs) +
+  MEMBER_SIZEOF(struct dp_packet, l2_5_ofs) ==
+  offsetof(struct dp_packet, l3_ofs));
+
+BUILD_ASSERT_DECL(offsetof(struct dp_packet, l3_ofs) +
+   MEMBER_SIZEOF(struct dp_packet, l3_ofs) ==
+   offsetof(struct dp_packet, l4_ofs));
+
+static inline void ALWAYS_INLINE
+avx512_dp_packet_resize_l2(struct dp_packet *b, int increment)
+{
+/* update packet size/data pointers */
+dp_packet_set_data(b, (char *) dp_packet_data(b) - increment);
+dp_packet_set_size(b, dp_packet_size(b) + increment);
+
+/* Increment u16 packet offset values */
+const __m128i v_zeros = _mm_setzero_si128();
+const __m128i v_u16_max = _mm_cmpeq_epi16(v_zeros, v_zeros);
+
+/* Only these lanes can be incremented for push-VLAN action. */
+const uint8_t k_lanes = 0b1110;
+__m128i v_offset = _mm_set1_epi16(VLAN_HEADER_LEN);
+
+/* Load packet and compare with UINT16_MAX */
+void *adjust_ptr = >l2_pad_size;
+__m128i v_adjust_src = _mm_loadu_si128(adjust_ptr);
+__mmask8 k_cmp = _mm_mask_cmpneq_epu16_mask(k_lanes, v_adjust_src,
+v_u16_max);
+
+/* Add VLAN_HEADER_LEN using compare mask, store results. */
+__m128i v_adjust_wip = _mm_mask_sub_epi16(v_adjust_src, k_cmp,
+  v_adjust_src, v_offset);
+_mm_storeu_si128(adjust_ptr, v_adjust_wip);
+
+}
+
+static inline void ALWAYS_INLINE
+avx512_eth_pop_vlan(struct dp_packet *packet)
+{
+struct vlan_eth_header *veh = dp_packet_eth(packet);
+
+if (veh && dp_packet_size(packet) >= sizeof *veh &&
+eth_type_vlan(veh->veth_type)) {
+
+__m128i v_ether = _mm_loadu_si128((void *) veh);
+__m128i v_realign = _mm_alignr_epi8(v_ether, _mm_setzero_si128(),
+16 - VLAN_HEADER_LEN);
+_mm_storeu_si128((void *) veh, v_realign);
+avx512_dp_packet_resize_l2(packet, -VLAN_HEADER_LEN);
+
+}
+}
+
+static void
+action_avx512_pop_vlan(void *dp OVS_UNUSED, struct dp_packet_batch *batch,
+   const struct nlattr *a OVS_UNUSED,
+   bool should_steal OVS_UNUSED)
+{
+struct dp_packet *packet;
+
+DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
+avx512_eth_pop_vlan(packet);
+}
+}
 
 /* Probe functions to check ISA requirements. */
 static int32_t
@@ -62,8 +132,13 @@ action_avx512_probe(void)
 
 
 int32_t
-action_avx512_init(void)
+action_avx512_init(struct odp_execute_action_impl *self)
 {
 avx512_isa_probe(0);
+self->funcs[OVS_ACTION_ATTR_POP_VLAN] = action_avx512_pop_vlan;
+
 return 0;
 }
+
+#endif
+#endif
diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c
index 35f07c10e..f4959c2c3 100644
--- a/lib/odp-execute-private.c
+++ b/lib/odp-execute-private.c
@@ -52,7 +52,7 @@ static struct odp_execute_action_impl action_impls[] = {
 .available = 1,
 .name = "avx512",
 .probe = action_avx512_probe,
-.init_func = NULL,
+.init_func = action_avx512_init,
 },
 #endif
 };
diff --git a/lib/odp-execute-private.h b/lib/odp-execute-private.h
index 4c09bee63..5ba2868bf 100644
--- a/lib/odp-execute-private.h
+++ b/lib/odp-execute-private.h
@@ -102,7 +102,7 @@ int32_t odp_execute_action_set(const char *name,
 int32_t odp_action_scalar_init(struct odp_execute_action_impl *self);
 
 /* Init function for the optimized with AVX512 actions. */
-int32_t action_avx512_init(void);
+int32_t action_avx512_init(struct odp_execute_action_impl *self);
 
 /* Probe function to check ISA requirements. */
 int32_t action_avx512_probe(void);
-- 
2.25.1

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


[ovs-dev] [PATCH v2 6/8] dpif-netdev: Add configure to enable autovalidator at build time.

2021-12-02 Thread Emma Finn
From: Kumar Amber 

This commit adds a new command to allow the user to enable
autovalidatior by default at build time thus allowing for
runnig unit test by default.

 $ ./configure --enable-actions-default-autovalidator

Signed-off-by: Kumar Amber 
Signed-off-by: Emma Finn 
---
 acinclude.m4  | 17 +
 configure.ac  |  1 +
 lib/odp-execute.c |  4 
 3 files changed, 22 insertions(+)

diff --git a/acinclude.m4 b/acinclude.m4
index 8ab690f47..d878ea4e7 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -14,6 +14,23 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+dnl Set OVS Actions Autovalidator as default action at compile time?
+dnl This enables automatically running all unit tests with all actions
+dnl implementations.
+AC_DEFUN([OVS_CHECK_ACTIONS_AUTOVALIDATOR], [
+  AC_ARG_ENABLE([actions-default-autovalidator],
+[AC_HELP_STRING([--enable-actions-default-autovalidator], 
[Enable actions autovalidator as default ovs actions implementation.])],
+[autovalidator=yes],[autovalidator=no])
+  AC_MSG_CHECKING([whether actions Autovalidator is default implementation])
+  if test "$autovalidator" != yes; then
+AC_MSG_RESULT([no])
+  else
+OVS_CFLAGS="$OVS_CFLAGS -DACTIONS_AUTOVALIDATOR_DEFAULT"
+AC_MSG_RESULT([yes])
+  fi
+])
+
+
 dnl Set OVS MFEX Autovalidator as default miniflow extract at compile time?
 dnl This enables automatically running all unit tests with all MFEX
 dnl implementations.
diff --git a/configure.ac b/configure.ac
index eaa9bf7ee..bfd0a9aff 100644
--- a/configure.ac
+++ b/configure.ac
@@ -185,6 +185,7 @@ OVS_CTAGS_IDENTIFIERS
 OVS_CHECK_DPCLS_AUTOVALIDATOR
 OVS_CHECK_DPIF_AVX512_DEFAULT
 OVS_CHECK_MFEX_AUTOVALIDATOR
+OVS_CHECK_ACTIONS_AUTOVALIDATOR
 OVS_CHECK_AVX512
 
 AC_ARG_VAR(KARCH, [Kernel Architecture String])
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index ab051aecc..1bc9fae09 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -865,7 +865,11 @@ odp_execute_init(void)
 static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
 if (ovsthread_once_start()) {
 odp_execute_action_init();
+#ifdef ACTIONS_AUTOVALIDATOR_DEFAULT
+odp_actions_impl_set("autovalidator");
+#else
 odp_actions_impl_set("scalar");
+#endif
 ovsthread_once_done();
 }
 }
-- 
2.25.1

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


[ovs-dev] [PATCH v2 7/8] odp-execute: Add ISA implementation of actions.

2021-12-02 Thread Emma Finn
This commit adds the AVX512 implementation of the action functionality.

Usage:
  $ ovs-appctl dpif-netdev/action-impl-set avx512

Signed-off-by: Emma Finn 
---
 lib/automake.mk   |  4 ++-
 lib/dpdk.c|  1 +
 lib/odp-execute-avx512.c  | 69 +++
 lib/odp-execute-private.c |  9 +
 lib/odp-execute-private.h |  9 +
 5 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 lib/odp-execute-avx512.c

diff --git a/lib/automake.mk b/lib/automake.mk
index 16087031f..34c03da45 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -32,6 +32,7 @@ lib_libopenvswitch_la_LIBADD += lib/libopenvswitchavx512.la
 lib_libopenvswitchavx512_la_CFLAGS = \
-mavx512f \
-mavx512bw \
+   -mavx512vl \
-mavx512dq \
-mbmi \
-mbmi2 \
@@ -40,7 +41,8 @@ lib_libopenvswitchavx512_la_CFLAGS = \
 lib_libopenvswitchavx512_la_SOURCES = \
lib/dpif-netdev-lookup-avx512-gather.c \
lib/dpif-netdev-extract-avx512.c \
-   lib/dpif-netdev-avx512.c
+   lib/dpif-netdev-avx512.c \
+   lib/odp-execute-avx512.c
 lib_libopenvswitchavx512_la_LDFLAGS = \
-static
 endif
diff --git a/lib/dpdk.c b/lib/dpdk.c
index b2ef31cd2..825e2daad 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -630,6 +630,7 @@ dpdk_get_cpu_has_isa(const char *arch, const char *feature)
 CHECK_CPU_FEATURE(feature, "avx512vbmi", RTE_CPUFLAG_AVX512VBMI);
 CHECK_CPU_FEATURE(feature, "avx512vpopcntdq", RTE_CPUFLAG_AVX512VPOPCNTDQ);
 CHECK_CPU_FEATURE(feature, "bmi2", RTE_CPUFLAG_BMI2);
+CHECK_CPU_FEATURE(feature, "avx512vl", RTE_CPUFLAG_AVX512VL);
 #endif
 
 VLOG_WARN("Unknown CPU arch,feature: %s,%s. Returning not supported.\n",
diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c
new file mode 100644
index 0..0765b8e3d
--- /dev/null
+++ b/lib/odp-execute-avx512.c
@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2021 Intel.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+
+#include "dpdk.h"
+#include "odp-execute-private.h"
+#include "odp-netlink.h"
+#include "dp-packet.h"
+#include "openvswitch/vlog.h"
+
+#include "immintrin.h"
+
+
+/* Probe functions to check ISA requirements. */
+static int32_t
+avx512_isa_probe(uint32_t needs_vbmi)
+{
+static const char *isa_required[] = {
+"avx512f",
+"avx512bw",
+"bmi2",
+"avx512vl"
+};
+
+int32_t ret = 0;
+for (uint32_t i = 0; i < ARRAY_SIZE(isa_required); i++) {
+if (!dpdk_get_cpu_has_isa("x86_64", isa_required[i])) {
+ret = -ENOTSUP;
+}
+}
+
+if (needs_vbmi) {
+if (!dpdk_get_cpu_has_isa("x86_64", "avx512vbmi")) {
+ret = -ENOTSUP;
+}
+}
+
+return ret;
+}
+
+int32_t
+action_avx512_probe(void)
+{
+const uint32_t needs_vbmi = 0;
+return avx512_isa_probe(needs_vbmi);
+}
+
+
+int32_t
+action_avx512_init(void)
+{
+avx512_isa_probe(0);
+return 0;
+}
diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c
index a6ebc8a65..35f07c10e 100644
--- a/lib/odp-execute-private.c
+++ b/lib/odp-execute-private.c
@@ -46,6 +46,15 @@ static struct odp_execute_action_impl action_impls[] = {
 .probe = NULL,
 .init_func = action_autoval_init,
 },
+
+#if (__x86_64__ && HAVE_AVX512F && HAVE_LD_AVX512_GOOD && __SSE4_2__)
+[ACTION_IMPL_AVX512] = {
+.available = 1,
+.name = "avx512",
+.probe = action_avx512_probe,
+.init_func = NULL,
+},
+#endif
 };
 
 static void
diff --git a/lib/odp-execute-private.h b/lib/odp-execute-private.h
index d49714bd2..4c09bee63 100644
--- a/lib/odp-execute-private.h
+++ b/lib/odp-execute-private.h
@@ -73,6 +73,9 @@ enum odp_execute_action_impl_idx {
  * Do not change the autovalidator position in this list without updating
  * the define below.
  */
+#if (__x86_64__ && HAVE_AVX512F && HAVE_LD_AVX512_GOOD && __SSE4_2__)
+ACTION_IMPL_AVX512,
+#endif
 
 ACTION_IMPL_MAX,
 };
@@ -98,4 +101,10 @@ int32_t odp_execute_action_set(const char *name,
  */
 int32_t odp_action_scalar_init(struct odp_execute_action_impl *self);
 
+/* Init function for the optimized with AVX512 actions. */
+int32_t action_avx512_init(void);
+
+/* Probe function to check ISA requirements. */
+int32_t action_avx512_probe(void);
+
 #endif /* ODP_EXTRACT_PRIVATE */
-- 
2.25.1

___
dev 

[ovs-dev] [PATCH v2 5/8] pmd.at: Add test-cases for ovs-actions commands.

2021-12-02 Thread Emma Finn
From: Kumar Amber 

Added separate test-case for ovs-actions get/set commands:
1023: PMD - ovs-actions configuration

The above added tests are to test the commands which are used
to either get or set the ovs-actions function pointers to
various different implementations like AVX512 or auto-validator
based on different CPU ISA supported.

Signed-off-by: Kumar Amber 
Signed-off-by: Emma Finn 
---
 tests/pmd.at | 20 
 1 file changed, 20 insertions(+)

diff --git a/tests/pmd.at b/tests/pmd.at
index c875a744f..4384652ff 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -1160,3 +1160,23 @@ ovs-appctl: ovs-vswitchd: server returned an error
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([PMD - ovs-actions configuration])
+OVS_VSWITCHD_START([], [], [], [--dummy-numa 0,0])
+AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dummy-pmd])
+
+AT_CHECK([ovs-vsctl show], [], [stdout])
+AT_CHECK([ovs-appctl dpif-netdev/action-impl-get | grep "scalar"], [], [dnl
+  scalar (available: True, active: True)
+])
+
+AT_CHECK([ovs-appctl dpif-netdev/action-impl-get | grep "autovalidator"], [], 
[dnl
+  autovalidator (available: True, active: False)
+])
+
+AT_CHECK([ovs-appctl dpif-netdev/action-impl-set scalar], [0], [dnl
+action implementation set to scalar.
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
\ No newline at end of file
-- 
2.25.1

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


[ovs-dev] [PATCH v2 3/8] odp-execute: Add auto validation function for actions.

2021-12-02 Thread Emma Finn
This commit introduced the auto-validation function which
allows users to compare the batch of packets obtained from
different action implementations against the linear
action implementation.

The autovalidator function can be triggered at runtime using the
following command:

$ ovs-appctl dpif-netdev/action-impl-set autovalidator

Signed-off-by: Emma Finn 
---
 lib/dp-packet.c   | 23 +
 lib/dp-packet.h   |  5 ++
 lib/odp-execute-private.c | 99 +++
 lib/odp-execute-private.h |  3 ++
 4 files changed, 130 insertions(+)

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 72f6d09ac..1e4ff35ef 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -506,3 +506,26 @@ dp_packet_resize_l2(struct dp_packet *b, int increment)
 dp_packet_adjust_layer_offset(>l2_5_ofs, increment);
 return dp_packet_data(b);
 }
+
+bool
+dp_packet_compare_and_log(struct dp_packet *good, struct dp_packet *test,
+  struct ds *err_str)
+{
+if ((good->l2_pad_size != test->l2_pad_size) ||
+(good->l2_5_ofs != test->l2_5_ofs) ||
+(good->l3_ofs != test->l3_ofs) ||
+(good->l4_ofs != test->l4_ofs)) {
+ds_put_format(err_str, "Autovalidation packet offsets failed"
+  "\n");
+ds_put_format(err_str, "Good offsets: l2_pad_size %u,"
+  " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
+  good->l2_pad_size, good->l2_5_ofs,
+  good->l3_ofs, good->l4_ofs);
+ds_put_format(err_str, "Test offsets: l2_pad_size %u,"
+  " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
+  test->l2_pad_size, test->l2_5_ofs,
+  test->l3_ofs, test->l4_ofs);
+return false;
+}
+return true;
+}
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 3dc582fbf..cc4c0d6da 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -234,6 +234,11 @@ void *dp_packet_steal_data(struct dp_packet *);
 static inline bool dp_packet_equal(const struct dp_packet *,
const struct dp_packet *);
 
+
+bool dp_packet_compare_and_log(struct dp_packet *good,
+   struct dp_packet *test,
+   struct ds *err_str);
+
 
 /* Frees memory that 'b' points to, as well as 'b' itself. */
 static inline void
diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c
index 5233eb909..880c91c16 100644
--- a/lib/odp-execute-private.c
+++ b/lib/odp-execute-private.c
@@ -29,6 +29,7 @@
 
 int32_t action_autoval_init(struct odp_execute_action_impl *self);
 VLOG_DEFINE_THIS_MODULE(odp_execute_private);
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
 
 static struct odp_execute_action_impl action_impls[] = {
 [ACTION_IMPL_SCALAR] = {
@@ -37,6 +38,13 @@ static struct odp_execute_action_impl action_impls[] = {
 .probe = NULL,
 .init_func = odp_action_scalar_init,
 },
+
+[ACTION_IMPL_AUTOVALIDATOR] = {
+.available = 1,
+.name = "autovalidator",
+.probe = NULL,
+.init_func = action_autoval_init,
+},
 };
 
 static void
@@ -82,3 +90,94 @@ odp_execute_action_init(void)
 }
 }
 }
+
+/* Init sequence required to be scalar first to pick up the default scalar
+* implementations, allowing over-riding of the optimized functions later.
+*/
+BUILD_ASSERT_DECL(ACTION_IMPL_SCALAR == 0);
+BUILD_ASSERT_DECL(ACTION_IMPL_AUTOVALIDATOR == 1);
+
+/* Loop over packets, and validate each one for the given action. */
+static void
+action_autoval_generic(void *dp OVS_UNUSED, struct dp_packet_batch *batch,
+   const struct nlattr *a, bool should_steal)
+{
+uint32_t failed = 0;
+
+int type = nl_attr_type(a);
+enum ovs_action_attr attr_type = (enum ovs_action_attr) type;
+
+struct odp_execute_action_impl *scalar = _impls[ACTION_IMPL_SCALAR];
+
+struct dp_packet_batch good_batch;
+dp_packet_batch_clone(_batch, batch);
+
+scalar->funcs[attr_type](NULL, _batch, a, should_steal);
+
+for (uint32_t impl = ACTION_IMPL_BEGIN; impl < ACTION_IMPL_MAX; impl++) {
+/* Clone original batch and execute implementation under test. */
+struct dp_packet_batch test_batch;
+dp_packet_batch_clone(_batch, batch);
+action_impls[impl].funcs[attr_type](NULL, _batch, a,
+should_steal);
+
+/* Loop over implementations, checking each one. */
+for (uint32_t pidx = 0; pidx < batch->count; pidx++) {
+struct dp_packet *good_pkt = good_batch.packets[pidx];
+struct dp_packet *test_pkt = test_batch.packets[pidx];
+
+struct ds log_msg = DS_EMPTY_INITIALIZER;
+
+/* Compare packet length and payload contents. */
+bool eq = dp_packet_equal(good_pkt, test_pkt);
+
+if (!eq) {
+   

[ovs-dev] [PATCH v2 4/8] odp-execute: Add command to switch action implementation.

2021-12-02 Thread Emma Finn
This commit adds a new command to allow the user to switch
the active action implementation at runtime. A probe function
is executed before switching the implementation, to ensure
the CPU is capable of running the ISA required.

Usage:
  $ ovs-appctl dpif-netdev/action-impl-set scalar

This commit also adds a new command to retrieve the list of available
action implementations. This can be used by to check what implementations
of actions are available and what implementation is active during runtime.

Usage:
   $ ovs-appctl dpif-netdev/action-impl-get

Signed-off-by: Emma Finn 

---
v2:
- Refactor to fix build warnings
---
 lib/dpif-netdev.c | 39 +++
 lib/odp-execute-private.c | 31 +++
 lib/odp-execute.c | 12 
 lib/odp-execute.h |  5 +
 4 files changed, 87 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 277e0d6c3..9684bbbc4 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -59,6 +59,7 @@
 #include "netdev-vport.h"
 #include "netlink.h"
 #include "odp-execute.h"
+#include "odp-execute-private.h"
 #include "odp-util.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/list.h"
@@ -1310,6 +1311,38 @@ error:
 ds_destroy();
 }
 
+static void
+action_impl_set(struct unixctl_conn *conn, int argc OVS_UNUSED,
+const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
+{
+struct ds reply = DS_EMPTY_INITIALIZER;
+
+int32_t err = odp_actions_impl_set(argv[1]);
+if (err) {
+ds_put_format(, "action implementation %s not found.\n",
+  argv[1]);
+const char *reply_str = ds_cstr();
+unixctl_command_reply_error(conn, reply_str);
+VLOG_ERR("%s", reply_str);
+ds_destroy();
+return;
+}
+
+ds_put_format(, "action implementation set to %s.\n", argv[1]);
+unixctl_command_reply(conn, ds_cstr());
+ds_destroy();
+}
+
+static void
+action_impl_get(struct unixctl_conn *conn, int argc OVS_UNUSED,
+const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
+{
+struct ds reply = DS_EMPTY_INITIALIZER;
+odp_execute_action_get();
+unixctl_command_reply(conn, ds_cstr());
+ds_destroy();
+}
+
 static void
 dpif_netdev_pmd_rebalance(struct unixctl_conn *conn, int argc,
   const char *argv[], void *aux OVS_UNUSED)
@@ -1547,6 +1580,12 @@ dpif_netdev_init(void)
 unixctl_command_register("dpif-netdev/miniflow-parser-get", "",
  0, 0, dpif_miniflow_extract_impl_get,
  NULL);
+unixctl_command_register("dpif-netdev/action-impl-set", "name",
+ 1, 1, action_impl_set,
+ NULL);
+unixctl_command_register("dpif-netdev/action-impl-get", "",
+ 0, 0, action_impl_get,
+ NULL);
 return 0;
 }
 
diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c
index 880c91c16..a6ebc8a65 100644
--- a/lib/odp-execute-private.c
+++ b/lib/odp-execute-private.c
@@ -30,6 +30,7 @@
 int32_t action_autoval_init(struct odp_execute_action_impl *self);
 VLOG_DEFINE_THIS_MODULE(odp_execute_private);
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+static uint32_t active_action_impl_index;
 
 static struct odp_execute_action_impl action_impls[] = {
 [ACTION_IMPL_SCALAR] = {
@@ -57,6 +58,36 @@ action_impl_copy_funcs(struct odp_execute_action_impl *to,
 }
 }
 
+void
+odp_execute_action_get(struct ds *string)
+{
+uint32_t i;
+
+ds_put_cstr(string, "Available Actions implementations:\n");
+for (i = 0; i < ACTION_IMPL_MAX; i++) {
+ds_put_format(string, "  %s (available: %s, active: %s)\n",
+  action_impls[i].name,
+  action_impls[i].available ? "True" : "False",
+  i == active_action_impl_index ? "True" : "False");
+}
+}
+
+int32_t
+odp_execute_action_set(const char *name,
+   struct odp_execute_action_impl *active)
+{
+uint32_t i;
+for (i = 0; i < ACTION_IMPL_MAX; i++) {
+/* string compare, and set ptrs *atomically*. */
+if (strcmp(action_impls[i].name, name) == 0) {
+action_impl_copy_funcs(active, _impls[i]);
+active_action_impl_index = i;
+return 0;
+}
+}
+return -1;
+}
+
 void
 odp_execute_action_init(void)
 {
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 31966aaa7..ab051aecc 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -865,10 +865,22 @@ odp_execute_init(void)
 static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
 if (ovsthread_once_start()) {
 odp_execute_action_init();
+odp_actions_impl_set("scalar");
 ovsthread_once_done();
 }
 }
 
+int32_t
+odp_actions_impl_set(const char *name)
+{
+
+int err = 

[ovs-dev] [PATCH v2 0/8] Actions Infrastructure + Optimizations

2021-12-02 Thread Emma Finn
---
v2:
- Fix the CI build issues.
---

This patchset introduces actions infrastructure changes
which allows the user to choose between different action
implementations based on CPU ISA by using different commands.
The Infrastructure also provides a way to check the correctness of
the ISA optimized action version against the scalar
version.
This patchset also introduces an optimized version of the pop_vlan
action.

Emma Finn (6):
  odp-execute: Add function pointers to odp-execute for different action
implementations.
  odp-execute: Add function pointer for pop_vlan action.
  odp-execute: Add auto validation function for actions.
  odp-execute: Add command to switch action implementation.
  odp-execute: Add ISA implementation of actions.
  odp-execute: Add ISA implementation of pop_vlan action.

Kumar Amber (2):
  pmd.at: Add test-cases for ovs-actions commands.
  dpif-netdev: Add configure to enable autovalidator at build time.

 acinclude.m4  |  17 +++
 configure.ac  |   1 +
 lib/automake.mk   |   6 +-
 lib/dp-packet.c   |  23 
 lib/dp-packet.h   |   5 +
 lib/dpdk.c|   1 +
 lib/dpif-netdev.c |  41 +++
 lib/odp-execute-avx512.c  | 144 
 lib/odp-execute-private.c | 223 ++
 lib/odp-execute-private.h | 110 +++
 lib/odp-execute.c |  84 --
 lib/odp-execute.h |   9 ++
 tests/pmd.at  |  20 
 13 files changed, 672 insertions(+), 12 deletions(-)
 create mode 100644 lib/odp-execute-avx512.c
 create mode 100644 lib/odp-execute-private.c
 create mode 100644 lib/odp-execute-private.h

-- 
2.25.1

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


[ovs-dev] [PATCH v2 1/8] odp-execute: Add function pointers to odp-execute for different action implementations.

2021-12-02 Thread Emma Finn
This commit introduces the initial infrastructure required to allow
different implementations for OvS actions. The patch introduces action
function pointers which allows user to switch between different action
implementations available. This will allow for more performance and flexibility
so the user can choose the action implementation to best suite their use case.

Signed-off-by: Emma Finn 

---
v2:
- Removed unused variable warning
---
 lib/automake.mk   |  2 +
 lib/dpif-netdev.c |  2 +
 lib/odp-execute-private.c | 84 +
 lib/odp-execute-private.h | 98 +++
 lib/odp-execute.c | 39 ++--
 lib/odp-execute.h |  4 ++
 6 files changed, 224 insertions(+), 5 deletions(-)
 create mode 100644 lib/odp-execute-private.c
 create mode 100644 lib/odp-execute-private.h

diff --git a/lib/automake.mk b/lib/automake.mk
index 46f869a33..16087031f 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -201,6 +201,8 @@ lib_libopenvswitch_la_SOURCES = \
lib/nx-match.h \
lib/object-collection.c \
lib/object-collection.h \
+   lib/odp-execute-private.c \
+   lib/odp-execute-private.h \
lib/odp-execute.c \
lib/odp-execute.h \
lib/odp-util.c \
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 69d7ec26e..277e0d6c3 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1598,6 +1598,8 @@ create_dpif_netdev(struct dp_netdev *dp)
 dpif->dp = dp;
 dpif->last_port_seq = seq_read(dp->port_seq);
 
+odp_execute_init();
+
 return >dpif;
 }
 
diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c
new file mode 100644
index 0..6441c491c
--- /dev/null
+++ b/lib/odp-execute-private.c
@@ -0,0 +1,84 @@
+/*
+ * Copyright (c) 2021 Intel.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "dpdk.h"
+
+#include "openvswitch/vlog.h"
+#include "odp-execute-private.h"
+#include "odp-netlink.h"
+#include "dp-packet.h"
+#include "odp-util.h"
+
+
+int32_t action_autoval_init(struct odp_execute_action_impl *self);
+VLOG_DEFINE_THIS_MODULE(odp_execute_private);
+
+static struct odp_execute_action_impl action_impls[] = {
+[ACTION_IMPL_SCALAR] = {
+.available = 1,
+.name = "scalar",
+.probe = NULL,
+.init_func = NULL,
+},
+};
+
+static void
+action_impl_copy_funcs(struct odp_execute_action_impl *to,
+   const struct odp_execute_action_impl *from)
+{
+for (uint32_t i = 0; i < __OVS_KEY_ATTR_MAX; i++) {
+atomic_uintptr_t *func = (void *) >funcs[i];
+atomic_store_relaxed(func, (uintptr_t) from->funcs[i]);
+}
+}
+
+void
+odp_execute_action_init(void)
+{
+/* Call probe on each impl, and cache the result. */
+for (int i = 0; i < ACTION_IMPL_MAX; i++) {
+bool avail = true;
+if (action_impls[i].probe) {
+/* Return zero is success, non-zero means error. */
+avail = (action_impls[i].probe() == 0);
+}
+VLOG_INFO("Action implementation %s (available: %s)\n",
+  action_impls[i].name, avail ? "available" : "not available");
+action_impls[i].available = avail;
+}
+
+uint32_t i;
+for (i = 0; i < ACTION_IMPL_MAX; i++) {
+/* Each impl's function array is initialized to reflect the scalar
+ * implementation. This simplifies adding optimized implementations,
+ * as the autovalidator can always compare all actions.
+ *
+ * Below copies the scalar functions to all other implementations.
+ */
+if (i != ACTION_IMPL_SCALAR) {
+action_impl_copy_funcs(_impls[i],
+   _impls[ACTION_IMPL_SCALAR]);
+}
+
+if (action_impls[i].init_func) {
+action_impls[i].init_func(_impls[i]);
+}
+}
+}
diff --git a/lib/odp-execute-private.h b/lib/odp-execute-private.h
new file mode 100644
index 0..c2e86bbee
--- /dev/null
+++ b/lib/odp-execute-private.h
@@ -0,0 +1,98 @@
+/*
+ * Copyright (c) 2021 Intel.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, 

[ovs-dev] [PATCH v2 2/8] odp-execute: Add function pointer for pop_vlan action.

2021-12-02 Thread Emma Finn
This commit removes the pop_vlan action from the large switch
and creates a separate function for batched processing. A function
pointer is also added to call the new batched function for the pop_vlan
action.

Signed-off-by: Emma Finn 

---
v2:
- Refactor to fix build warnings
---
 lib/odp-execute-private.c |  2 +-
 lib/odp-execute.c | 29 +++--
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c
index 6441c491c..5233eb909 100644
--- a/lib/odp-execute-private.c
+++ b/lib/odp-execute-private.c
@@ -35,7 +35,7 @@ static struct odp_execute_action_impl action_impls[] = {
 .available = 1,
 .name = "scalar",
 .probe = NULL,
-.init_func = NULL,
+.init_func = odp_action_scalar_init,
 },
 };
 
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 49dfa2a74..31966aaa7 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -831,6 +831,28 @@ requires_datapath_assistance(const struct nlattr *a)
 return false;
 }
 
+static void
+action_pop_vlan(void *dp OVS_UNUSED, struct dp_packet_batch *batch,
+const struct nlattr *a OVS_UNUSED,
+bool should_steal OVS_UNUSED)
+{
+struct dp_packet *packet;
+DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
+eth_pop_vlan(packet);
+}
+}
+
+/* Implementation of the scalar actions impl init function. Build up the
+ * array of func ptrs here.
+ */
+int32_t
+odp_action_scalar_init(struct odp_execute_action_impl *self)
+{
+self->funcs[OVS_ACTION_ATTR_POP_VLAN] = action_pop_vlan;
+
+return 0;
+}
+
 /* The active function pointers on the datapath. ISA optimized implementations
  * are enabled by plugging them into this static arary, which is consulted when
  * applying actions on the datapath.
@@ -962,12 +984,6 @@ odp_execute_actions(void *dp, struct dp_packet_batch 
*batch, bool steal,
 break;
 }
 
-case OVS_ACTION_ATTR_POP_VLAN:
-DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
-eth_pop_vlan(packet);
-}
-break;
-
 case OVS_ACTION_ATTR_PUSH_MPLS: {
 const struct ovs_action_push_mpls *mpls = nl_attr_get(a);
 
@@ -1100,6 +1116,7 @@ odp_execute_actions(void *dp, struct dp_packet_batch 
*batch, bool steal,
 }
 case OVS_ACTION_ATTR_OUTPUT:
 case OVS_ACTION_ATTR_LB_OUTPUT:
+case OVS_ACTION_ATTR_POP_VLAN:
 case OVS_ACTION_ATTR_TUNNEL_PUSH:
 case OVS_ACTION_ATTR_TUNNEL_POP:
 case OVS_ACTION_ATTR_USERSPACE:
-- 
2.25.1

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


Re: [ovs-dev] [PATCH] datapath: handle DNAT tuple collision

2021-12-02 Thread Odintsov Vladislav
Thanks for the backport from upstream!
May I request backport patch [1] and this patch down to supported branches?

Thanks.

1: 
https://github.com/openvswitch/ovs/commit/6a101a6c8372570a30e0f8edb558c8a69cc80e7d

Regards,
Vladislav Odintsov

On 12 Oct 2021, at 22:30, Ilya Maximets 
mailto:i.maxim...@ovn.org>> wrote:

On 9/9/21 21:59, Paolo Valerio wrote:
Dumitru Ceara mailto:dce...@redhat.com>> writes:

Upstream commit:
   commit 8aa7b526dc0b5dbf40c1b834d76a667ad672a410
   Author: Dumitru Ceara mailto:dce...@redhat.com>>
   Date:   Wed Oct 7 17:48:03 2020 +0200

   openvswitch: handle DNAT tuple collision

   With multiple DNAT rules it's possible that after destination
   translation the resulting tuples collide.

   For example, two openvswitch flows:
   nw_dst=10.0.0.10,tp_dst=10, actions=ct(commit,table=2,nat(dst=20.0.0.1:20))
   nw_dst=10.0.0.20,tp_dst=10, actions=ct(commit,table=2,nat(dst=20.0.0.1:20))

   Assuming two TCP clients initiating the following connections:
   10.0.0.10:5000->10.0.0.10:10
   10.0.0.10:5000->10.0.0.20:10

   Both tuples would translate to 10.0.0.10:5000->20.0.0.1:20 causing
   nf_conntrack_confirm() to fail because of tuple collision.

   Netfilter handles this case by allocating a null binding for SNAT at
   egress by default.  Perform the same operation in openvswitch for DNAT
   if no explicit SNAT is requested by the user and allocate a null binding
   for SNAT for packets in the "original" direction.

   Reported-at: https://bugzilla.redhat.com/1877128
   Suggested-by: Florian Westphal mailto:f...@strlen.de>>
   Fixes: 05752523e565 ("openvswitch: Interface with NAT.")
   Signed-off-by: Dumitru Ceara mailto:dce...@redhat.com>>
   Signed-off-by: Jakub Kicinski mailto:k...@kernel.org>>

Fixes: f8f97cdce9ad ("datapath: Interface with NAT.")
Signed-off-by: Dumitru Ceara mailto:dce...@redhat.com>>
---

Acked-by: Paolo Valerio mailto:pvale...@redhat.com>>

Thanks!  Applied.

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 v3] dpif-netdev: Call cpuid for x86 isa availability.

2021-12-02 Thread David Marchand
On Tue, Nov 30, 2021 at 5:53 PM Van Haaren, Harry
 wrote:
> > Resolve isa availability in constructors by using a simplified query
> > based on cpuid API that comes from the compiler.
>
> Using constructors instead of an init() time call is interesting, but may not 
> be what we
> always want. For "vswitchd" it is a useful startup feature, however other 
> binaries/tools
> such as "ovs-vsctl" or "ovs-appctl" do not require CPUID-based ISA detection 
> at all.
> As per this patch, every launch of "ovs-vsctl" (or other tooling/binaries) 
> will cause the
> constructors to run.
>
> I would like to add some VLOG_* info/logging to the CPU ISA detection, it may 
> be useful
> to understand the system if in future debug of CPU ISA implementations is 
> required.
> (This is how the constructor-running was identified, lots of printf() at 
> tooling startup!)

I can look at adding logs in dpif as a preparation patch.
The current situation where we have logs at init is not user friendly:
for a user that wants to use feature X and enters the right ovs-*ctl
commands, it is hard to make the relation with "cpu has feature Y" in
OVS logs that could be days old.


Moving this code in some init function will add an init order dependency.
This detection code is really small, stateless and can be done as
early as possible.
Otoh, with constructors, components in OVS can get them without caring
if init was run (let's say some day we need to know about those
features in utils).


> > +
> > +enum x86_reg {
> > +EAX,
> > +EBX,
> > +ECX,
> > +EDX,
> > +};
> > +#define X86_LEAF_MASK 0x8000
> > +#define X86_EXT_FEATURES_LEAF 0x0007
> > +static bool x86_has_isa(uint32_t leaf, enum x86_reg reg, uint32_t bit)
> > +{
> > +uint32_t maxleaf = __get_cpuid_max(leaf & X86_LEAF_MASK, NULL);
> > +uint32_t regs[4];
> > +
> > +if (maxleaf < leaf) {
> > +return false;
>
> This is a programming error, not a runtime error correct? We're asking for a
> leaf that has not been supported in OVS. Presumably the programmer intended
> to ask for a feature that OVS has support for. So a unique/identifiable error 
> return

A ovs_assert() is better in this case.


-- 
David Marchand

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


Re: [ovs-dev] [PATCH v3 2/4] system-dpdk: Use dummy-pmd port for packet injection.

2021-12-02 Thread David Marchand
On Thu, Dec 2, 2021 at 2:56 PM Van Haaren, Harry
 wrote:
>
> > -Original Message-
> > From: dev  On Behalf Of David Marchand
> > Sent: Thursday, December 2, 2021 12:21 PM
> > To: Amber, Kumar 
> > Cc: d...@openvswitch.org; i.maxim...@ovn.org; f...@sysclose.org;
> > maxime.coque...@redhat.com
> > Subject: Re: [ovs-dev] [PATCH v3 2/4] system-dpdk: Use dummy-pmd port for
> > packet injection.
> >
> > On Wed, Dec 1, 2021 at 3:52 PM Amber, Kumar 
> > wrote:
> > > > diff --git a/tests/genpkts.py b/tests/genpkts.py new file mode 100755 
> > > > index
> > > > 00..f64f786ccb
> > > > --- /dev/null
> > > > +++ b/tests/genpkts.py
> > > > @@ -0,0 +1,56 @@
> > > > +#!/usr/bin/env python3
> > > > +
> > > > +import sys
> > > > +
> > > > +from scapy.all import RandMAC, RandIP, RandIP6, RandShort, fuzz from
> > > > +scapy.all import IPv6, Dot1Q, IP, Ether, UDP, TCP
> > > > +
> > > > +if len(sys.argv) < 2:
> > > > +print('usage: {} packets_count [fuzz]'.format(sys.argv[0]))
> > > > +sys.exit(1)
> > > > +
> > > > +tmpl = []
> > > > +
> > > > +if len(sys.argv) == 2:
> > > > +eth = Ether(dst='ff:ff:ff:ff:ff:ff')
> > > > +vlan = eth / Dot1Q(vlan=1)
> > > > +p = eth / IP() / TCP(sport=20, dport=80, flags='SA', window=8192)
> > > > +tmpl += [p.build().hex()]
> > > > +p = eth / IP() / UDP(sport=53, dport=53)
> > > > +tmpl += [p.build().hex()]
> > > > +p = eth / IP() / TCP(sport=20, dport=80, flags='S', window=8192)
> > > > +tmpl += [p.build().hex()]
> > > > +p = eth / IP() / UDP(sport=53, dport=53)
> > > > +tmpl += [p.build().hex()]
> > > > +p = vlan / IP() / UDP(sport=53, dport=53)
> > > > +tmpl += [p.build().hex()]
> > > > +p = vlan / IP() / TCP(sport=20, dport=80, flags='S', window=8192)
> > > > +tmpl += [p.build().hex()]
> > >
> > > Hardcoding the values here is not preferable as we wanted to test the
> > optimized implementations
> > > with various values contained inside the header.
> >
> > Those hardcoded values comes from the pcap file that was previously used.
> > If you want to add more protocols, it is easier with this patch as you
> > only need to update some python script rather than rewrite a pcap
> > file.
>
> The PCAP was written by a script, which generated random MAC addresses;
>   previously:  eth = Ether(src=RandMAC(), dst=RandMAC())

We have 3 MFEX tests.

OVS-DPDK - MFEX Autovalidator
OVS-DPDK - MFEX Autovalidator fuzzy
OVS-DPDK - MFEX Configuration

In the first and 3rd ones, a pcap file tests/pcap/mfex_test.pcap
(committed to OVS sources) was used before this patch.
The hunk above from my patch is about replacing this "hardcoded" pcap
binary file with a python script that generates hardcoded packets.

For the 2nd test, if you look at the patch, you'll notice that the
fuzzy part is handled like before and generate random packets.


>
> I do not see the value add here, and I'm concerned that when regressions in 
> these methods are pointed
> out that these are not acted on, but instead we are told "this hardcoded 
> method is better". It is not.

Read what I wrote as: "hardcoded python" is better than "hardcoded
pcap" binary file.


> > > > +on_exit "pkill -f -x -9 '$PYTHON3 $srcdir/genpkts.py -1 fuzz'"
> > > > +($PYTHON3 $srcdir/genpkts.py -1 fuzz | while read pkt; do
> > > > + ovs-appctl netdev-dummy/receive p1 "$pkt" || break
> > > > + done) &
> > > > +
> > > >  AT_CHECK([ovs-appctl dpif-netdev/miniflow-parser-set autovalidator], 
> > > > [0],
> > [dnl
> > > > Miniflow extract implementation set to autovalidator.
> > > >  ])
> > > >
> > > > -OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics | grep -oP
> > > > 'rx_packets=\s*\K\d+'` -ge 10])
> > >
> > > We should increase the packet count to at-least 10x the current number to
> > have a proper fuzzy testing and we have measured it would only take 10~ to 
> > 15~
> > sec more. The current runtime did not catch issues when we purposely broke 
> > the
> > implementation and by allowing to run for 1 packets, it did catch the 
> > induced
> > error.
> >
> > For me, the fuzzy testing does not have its place in a CI, because it
> > is not reproducible.
> > I let it in place and just made sure it would not reach the timeout.
>
> Disagree here, Fuzzing is *ideal* for CI, as it tests different inputs 
> continuously,
> and each CI run improves the confidence in the system. A large number of open
> source projects are actively doing large-scale fuzzing in CI instances.
> e.g.; https://google.github.io/clusterfuzz/
>
> If the fuzzing autotests fails in CI, it still flags that there is *an 
> issue*. In our
> case with AutoValidators for DPCLS and MFEX, it even prints a whole debug log
> of "good" miniflow, as well as "bad" results of the optimized implementation.
> These VLOG_ERR results would be hugely helpful in identifying & debugging.
>
> I do not understand the motivation for disabling/limiting fuzzing in CI.

Fuzzing is sure a cool thing when run on an identified 

Re: [ovs-dev] [PATCH v3 2/4] system-dpdk: Use dummy-pmd port for packet injection.

2021-12-02 Thread Van Haaren, Harry
> -Original Message-
> From: dev  On Behalf Of David Marchand
> Sent: Thursday, December 2, 2021 12:21 PM
> To: Amber, Kumar 
> Cc: d...@openvswitch.org; i.maxim...@ovn.org; f...@sysclose.org;
> maxime.coque...@redhat.com
> Subject: Re: [ovs-dev] [PATCH v3 2/4] system-dpdk: Use dummy-pmd port for
> packet injection.
> 
> On Wed, Dec 1, 2021 at 3:52 PM Amber, Kumar 
> wrote:
> > > diff --git a/tests/genpkts.py b/tests/genpkts.py new file mode 100755 
> > > index
> > > 00..f64f786ccb
> > > --- /dev/null
> > > +++ b/tests/genpkts.py
> > > @@ -0,0 +1,56 @@
> > > +#!/usr/bin/env python3
> > > +
> > > +import sys
> > > +
> > > +from scapy.all import RandMAC, RandIP, RandIP6, RandShort, fuzz from
> > > +scapy.all import IPv6, Dot1Q, IP, Ether, UDP, TCP
> > > +
> > > +if len(sys.argv) < 2:
> > > +print('usage: {} packets_count [fuzz]'.format(sys.argv[0]))
> > > +sys.exit(1)
> > > +
> > > +tmpl = []
> > > +
> > > +if len(sys.argv) == 2:
> > > +eth = Ether(dst='ff:ff:ff:ff:ff:ff')
> > > +vlan = eth / Dot1Q(vlan=1)
> > > +p = eth / IP() / TCP(sport=20, dport=80, flags='SA', window=8192)
> > > +tmpl += [p.build().hex()]
> > > +p = eth / IP() / UDP(sport=53, dport=53)
> > > +tmpl += [p.build().hex()]
> > > +p = eth / IP() / TCP(sport=20, dport=80, flags='S', window=8192)
> > > +tmpl += [p.build().hex()]
> > > +p = eth / IP() / UDP(sport=53, dport=53)
> > > +tmpl += [p.build().hex()]
> > > +p = vlan / IP() / UDP(sport=53, dport=53)
> > > +tmpl += [p.build().hex()]
> > > +p = vlan / IP() / TCP(sport=20, dport=80, flags='S', window=8192)
> > > +tmpl += [p.build().hex()]
> >
> > Hardcoding the values here is not preferable as we wanted to test the
> optimized implementations
> > with various values contained inside the header.
> 
> Those hardcoded values comes from the pcap file that was previously used.
> If you want to add more protocols, it is easier with this patch as you
> only need to update some python script rather than rewrite a pcap
> file.

The PCAP was written by a script, which generated random MAC addresses;
  previously:  eth = Ether(src=RandMAC(), dst=RandMAC())

The same issue exists here with TCP sport/dport, which were previously randomly
generated, and are now being hard-coded to a small set of specific values;
  previously:tcp = TCP(dport=RandShort(), sport=RandShort())

Amber correctly points out that hard-coding these values is a regression in 
testing.
Re-writing a PCAP file is easy, please refer to the current "mfex_fuzzy.py" 
script.
Updating hex ethernet addresses manually, or TCP ports manually instead of
automatically generating them is a step backwards.

I do not see the value add here, and I'm concerned that when regressions in 
these methods are pointed
out that these are not acted on, but instead we are told "this hardcoded method 
is better". It is not.


> > > +on_exit "pkill -f -x -9 '$PYTHON3 $srcdir/genpkts.py -1 fuzz'"
> > > +($PYTHON3 $srcdir/genpkts.py -1 fuzz | while read pkt; do
> > > + ovs-appctl netdev-dummy/receive p1 "$pkt" || break
> > > + done) &
> > > +
> > >  AT_CHECK([ovs-appctl dpif-netdev/miniflow-parser-set autovalidator], [0],
> [dnl
> > > Miniflow extract implementation set to autovalidator.
> > >  ])
> > >
> > > -OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics | grep -oP
> > > 'rx_packets=\s*\K\d+'` -ge 10])
> >
> > We should increase the packet count to at-least 10x the current number to
> have a proper fuzzy testing and we have measured it would only take 10~ to 15~
> sec more. The current runtime did not catch issues when we purposely broke the
> implementation and by allowing to run for 1 packets, it did catch the 
> induced
> error.
> 
> For me, the fuzzy testing does not have its place in a CI, because it
> is not reproducible.
> I let it in place and just made sure it would not reach the timeout.

Disagree here, Fuzzing is *ideal* for CI, as it tests different inputs 
continuously,
and each CI run improves the confidence in the system. A large number of open
source projects are actively doing large-scale fuzzing in CI instances.
e.g.; https://google.github.io/clusterfuzz/

If the fuzzing autotests fails in CI, it still flags that there is *an issue*. 
In our
case with AutoValidators for DPCLS and MFEX, it even prints a whole debug log
of "good" miniflow, as well as "bad" results of the optimized implementation.
These VLOG_ERR results would be hugely helpful in identifying & debugging.

I do not understand the motivation for disabling/limiting fuzzing in CI.


(Side note; there are methods to get reproducible PRNG built into unit tests,
see this project for example, where exactly that is achieved by printing the 
"seed"
value of the PRNG before test, allowing developers to reproduce the exact run:
https://nemequ.github.io/munit/#prng)


> On the system I used, this test takes 5s with 1k and timeouts with 10k.
> So I guess the 10s/15s evaluation is 

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

2021-12-02 Thread David Marchand
On Thu, Dec 2, 2021 at 1:26 PM Stokes, Ian  wrote:
> > 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.

I caught one issue depending on hugepage sizes availability when
working on system-dpdk ut but I had dropped this hunk and forgot when
I sent the patches for the master branch.
I suspect this is the one you hit.

This is due to c69150679891 ("eal/linux: improve no hugepages logging").

You'll want to update those warning logs in system-dpdk.at, like:
-/EAL: No free hugepages reported in hugepages-1048576kB/d"])
+/EAL: No free .* hugepages reported/d"])

This will likely conflict with my patchset on system-dpdk, but this is
easy to fix.


-- 
David Marchand

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


Re: [ovs-dev] [PATCH] net: openvswitch: Remove redundant if statements

2021-12-02 Thread patchwork-bot+netdevbpf
Hello:

This patch was applied to netdev/net-next.git (master)
by David S. Miller :

On Thu,  2 Dec 2021 07:51:48 + you wrote:
> The 'if (dev)' statement already move into dev_{put , hold}, so remove
> redundant if statements.
> 
> Signed-off-by: Xu Wang 
> ---
>  net/openvswitch/vport-netdev.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Here is the summary with links:
  - net: openvswitch: Remove redundant if statements
https://git.kernel.org/netdev/net-next/c/98fa41d62760

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html


___
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.

2021-12-02 Thread Ilya Maximets
On 12/2/21 13:37, David Marchand wrote:
> On Thu, Dec 2, 2021 at 1:18 PM Ilya Maximets  wrote:
>>
>> On 12/2/21 12:47, David Marchand wrote:
>>> 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?
>>
>> This will be done as part of "Prepare for 2.17.0." patch while preparing
>> for the actual release along with update for the kernel support list.
> 
> I was not sure because of 8d04161534e1 ("faq: Update OVS/DPDK version
> table for OVS 2.15.").

Yeah.  Before 2.16 this kind of documentation updates were more or less
random as we always forgot to do that.

> But that's better to do this kind of update once, when preparing the release.

Keeping them to the release patches gives some stability and makes it harder
to forget to update the documentation, IMO.

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


[ovs-dev] [PATCH v2] tc: Fix incorrect TC rule for decap+encap datapath flow

2021-12-02 Thread Roi Dayan via dev
A datapath flow generated for traffic from vxlan port to another vxlan port
looks like this:

tunnel(tun_id=0x65,src=10.10.11.3,dst=10.10.11.2,ttl=0/0,tp_dst=4789,flags(+key)),...,in_port(vxlan_sys_4789),...,
 
actions:set(tunnel(tun_id=0x66,src=10.10.12.2,dst=10.10.12.3,tp_dst=4789,flags(key))),vxlan_sys_4789

The generated TC rule with explicit tunnel key unset action added after
tunnel key set action, which is wrong.

filter protocol ip pref 7 flower chain 0 handle 0x1
  dst_mac fa:16:3e:2a:4e:23
  eth_type ipv4
  ip_tos 0x0/3
  enc_dst_ip 10.10.11.2
  enc_src_ip 10.10.11.3
  enc_key_id 101
  enc_dst_port 4789
  ip_flags nofrag
  not_in_hw
action order 1: tunnel_key  set
src_ip 10.10.12.2
dst_ip 10.10.12.3
key_id 102
dst_port 4789
nocsum pipe
 index 1 ref 1 bind 1 installed 568 sec used 0 sec
Action statistics:
Sent 46620 bytes 555 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0

action order 2: tunnel_key  unset pipe
 index 2 ref 1 bind 1 installed 568 sec used 0 sec
Action statistics:
Sent 46620 bytes 555 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0

action order 3: mirred (Egress Redirect to device vxlan_sys_4789) stolen
index 1 ref 1 bind 1 installed 568 sec used 0 sec
Action statistics:
Sent 46620 bytes 555 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
cookie e0c82bfd504b701428b00db6b08db3b2

Fix it by also adding the the tunnel key unset action before the tunnel
key set action and not only before output port.

Fixes: 7c53bd7839d8 ("tc: Move tunnel_key unset action before output ports")
Signed-off-by: Roi Dayan 
Reviewed-by: Paul Blakey 
---

Notes:
v2
- fix a mistake in the reviewed-by tag email domain.

 lib/tc.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/lib/tc.c b/lib/tc.c
index 38a1dfc0ebc8..adb2d3182ad4 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -2545,6 +2545,17 @@ nl_msg_put_flower_rewrite_pedits(struct ofpbuf *request,
 return 0;
 }
 
+static void
+nl_msg_put_flower_acts_release(struct ofpbuf *request, uint16_t act_index)
+{
+size_t act_offset;
+
+act_offset = nl_msg_start_nested(request, act_index);
+nl_msg_put_act_tunnel_key_release(request);
+nl_msg_put_act_flags(request);
+nl_msg_end_nested(request, act_offset);
+}
+
 static int
 nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower)
 {
@@ -2579,6 +2590,11 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct 
tc_flower *flower)
 }
 break;
 case TC_ACT_ENCAP: {
+if (!released && flower->tunnel) {
+nl_msg_put_flower_acts_release(request, act_index++);
+released = true;
+}
+
 act_offset = nl_msg_start_nested(request, act_index++);
 nl_msg_put_act_tunnel_key_set(request, 
action->encap.id_present,
   action->encap.id,
@@ -2636,10 +2652,7 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct 
tc_flower *flower)
 break;
 case TC_ACT_OUTPUT: {
 if (!released && flower->tunnel) {
-act_offset = nl_msg_start_nested(request, act_index++);
-nl_msg_put_act_tunnel_key_release(request);
-nl_msg_put_act_flags(request);
-nl_msg_end_nested(request, act_offset);
+nl_msg_put_flower_acts_release(request, act_index++);
 released = true;
 }
 
-- 
2.8.0

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


Re: [ovs-dev] [PATCH] tc: Fix incorrect TC rule for decap+encap datapath flow

2021-12-02 Thread Roi Dayan via dev




On 2021-12-02 2:36 PM, Roi Dayan wrote:

A datapath flow generated for traffic from vxlan port to another vxlan port
looks like this:

tunnel(tun_id=0x65,src=10.10.11.3,dst=10.10.11.2,ttl=0/0,tp_dst=4789,flags(+key)),...,in_port(vxlan_sys_4789),...,
 
actions:set(tunnel(tun_id=0x66,src=10.10.12.2,dst=10.10.12.3,tp_dst=4789,flags(key))),vxlan_sys_4789

The generated TC rule with explicit tunnel key unset action added after
tunnel key set action, which is wrong.

filter protocol ip pref 7 flower chain 0 handle 0x1
   dst_mac fa:16:3e:2a:4e:23
   eth_type ipv4
   ip_tos 0x0/3
   enc_dst_ip 10.10.11.2
   enc_src_ip 10.10.11.3
   enc_key_id 101
   enc_dst_port 4789
   ip_flags nofrag
   not_in_hw
 action order 1: tunnel_key  set
 src_ip 10.10.12.2
 dst_ip 10.10.12.3
 key_id 102
 dst_port 4789
 nocsum pipe
  index 1 ref 1 bind 1 installed 568 sec used 0 sec
 Action statistics:
 Sent 46620 bytes 555 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0

 action order 2: tunnel_key  unset pipe
  index 2 ref 1 bind 1 installed 568 sec used 0 sec
 Action statistics:
 Sent 46620 bytes 555 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0

 action order 3: mirred (Egress Redirect to device vxlan_sys_4789) 
stolen
 index 1 ref 1 bind 1 installed 568 sec used 0 sec
 Action statistics:
 Sent 46620 bytes 555 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
 cookie e0c82bfd504b701428b00db6b08db3b2

Fix it by also adding the the tunnel key unset action before the tunnel
key set action and not only before output port.

Fixes: 7c53bd7839d8 ("tc: Move tunnel_key unset action before output ports")
Signed-off-by: Roi Dayan 
Reviewed-by: Paul Blakey 


sorry I used incorrect email domain with the reviewed-by tag.
i'll v2 fixing only the email domain.


---
  lib/tc.c | 21 +
  1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/lib/tc.c b/lib/tc.c
index 38a1dfc0ebc8..adb2d3182ad4 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -2545,6 +2545,17 @@ nl_msg_put_flower_rewrite_pedits(struct ofpbuf *request,
  return 0;
  }
  
+static void

+nl_msg_put_flower_acts_release(struct ofpbuf *request, uint16_t act_index)
+{
+size_t act_offset;
+
+act_offset = nl_msg_start_nested(request, act_index);
+nl_msg_put_act_tunnel_key_release(request);
+nl_msg_put_act_flags(request);
+nl_msg_end_nested(request, act_offset);
+}
+
  static int
  nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower)
  {
@@ -2579,6 +2590,11 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct 
tc_flower *flower)
  }
  break;
  case TC_ACT_ENCAP: {
+if (!released && flower->tunnel) {
+nl_msg_put_flower_acts_release(request, act_index++);
+released = true;
+}
+
  act_offset = nl_msg_start_nested(request, act_index++);
  nl_msg_put_act_tunnel_key_set(request, 
action->encap.id_present,
action->encap.id,
@@ -2636,10 +2652,7 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct 
tc_flower *flower)
  break;
  case TC_ACT_OUTPUT: {
  if (!released && flower->tunnel) {
-act_offset = nl_msg_start_nested(request, act_index++);
-nl_msg_put_act_tunnel_key_release(request);
-nl_msg_put_act_flags(request);
-nl_msg_end_nested(request, act_offset);
+nl_msg_put_flower_acts_release(request, act_index++);
  released = true;
  }
  


___
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.

2021-12-02 Thread David Marchand
On Thu, Dec 2, 2021 at 1:18 PM Ilya Maximets  wrote:
>
> On 12/2/21 12:47, David Marchand wrote:
> > 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?
>
> This will be done as part of "Prepare for 2.17.0." patch while preparing
> for the actual release along with update for the kernel support list.

I was not sure because of 8d04161534e1 ("faq: Update OVS/DPDK version
table for OVS 2.15.").
But that's better to do this kind of update once, when preparing the release.

Thanks Ilya.


-- 
David Marchand

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


[ovs-dev] [PATCH] tc: Fix incorrect TC rule for decap+encap datapath flow

2021-12-02 Thread Roi Dayan via dev
A datapath flow generated for traffic from vxlan port to another vxlan port
looks like this:

tunnel(tun_id=0x65,src=10.10.11.3,dst=10.10.11.2,ttl=0/0,tp_dst=4789,flags(+key)),...,in_port(vxlan_sys_4789),...,
 
actions:set(tunnel(tun_id=0x66,src=10.10.12.2,dst=10.10.12.3,tp_dst=4789,flags(key))),vxlan_sys_4789

The generated TC rule with explicit tunnel key unset action added after
tunnel key set action, which is wrong.

filter protocol ip pref 7 flower chain 0 handle 0x1
  dst_mac fa:16:3e:2a:4e:23
  eth_type ipv4
  ip_tos 0x0/3
  enc_dst_ip 10.10.11.2
  enc_src_ip 10.10.11.3
  enc_key_id 101
  enc_dst_port 4789
  ip_flags nofrag
  not_in_hw
action order 1: tunnel_key  set
src_ip 10.10.12.2
dst_ip 10.10.12.3
key_id 102
dst_port 4789
nocsum pipe
 index 1 ref 1 bind 1 installed 568 sec used 0 sec
Action statistics:
Sent 46620 bytes 555 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0

action order 2: tunnel_key  unset pipe
 index 2 ref 1 bind 1 installed 568 sec used 0 sec
Action statistics:
Sent 46620 bytes 555 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0

action order 3: mirred (Egress Redirect to device vxlan_sys_4789) stolen
index 1 ref 1 bind 1 installed 568 sec used 0 sec
Action statistics:
Sent 46620 bytes 555 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
cookie e0c82bfd504b701428b00db6b08db3b2

Fix it by also adding the the tunnel key unset action before the tunnel
key set action and not only before output port.

Fixes: 7c53bd7839d8 ("tc: Move tunnel_key unset action before output ports")
Signed-off-by: Roi Dayan 
Reviewed-by: Paul Blakey 
---
 lib/tc.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/lib/tc.c b/lib/tc.c
index 38a1dfc0ebc8..adb2d3182ad4 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -2545,6 +2545,17 @@ nl_msg_put_flower_rewrite_pedits(struct ofpbuf *request,
 return 0;
 }
 
+static void
+nl_msg_put_flower_acts_release(struct ofpbuf *request, uint16_t act_index)
+{
+size_t act_offset;
+
+act_offset = nl_msg_start_nested(request, act_index);
+nl_msg_put_act_tunnel_key_release(request);
+nl_msg_put_act_flags(request);
+nl_msg_end_nested(request, act_offset);
+}
+
 static int
 nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower)
 {
@@ -2579,6 +2590,11 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct 
tc_flower *flower)
 }
 break;
 case TC_ACT_ENCAP: {
+if (!released && flower->tunnel) {
+nl_msg_put_flower_acts_release(request, act_index++);
+released = true;
+}
+
 act_offset = nl_msg_start_nested(request, act_index++);
 nl_msg_put_act_tunnel_key_set(request, 
action->encap.id_present,
   action->encap.id,
@@ -2636,10 +2652,7 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct 
tc_flower *flower)
 break;
 case TC_ACT_OUTPUT: {
 if (!released && flower->tunnel) {
-act_offset = nl_msg_start_nested(request, act_index++);
-nl_msg_put_act_tunnel_key_release(request);
-nl_msg_put_act_flags(request);
-nl_msg_end_nested(request, act_offset);
+nl_msg_put_flower_acts_release(request, act_index++);
 released = true;
 }
 
-- 
2.8.0

___
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.

2021-12-02 Thread 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 
> 

Thanks for the review.

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


Re: [ovs-dev] [PATCH v3 2/4] system-dpdk: Use dummy-pmd port for packet injection.

2021-12-02 Thread David Marchand
On Wed, Dec 1, 2021 at 3:52 PM Amber, Kumar  wrote:
> > diff --git a/tests/genpkts.py b/tests/genpkts.py new file mode 100755 index
> > 00..f64f786ccb
> > --- /dev/null
> > +++ b/tests/genpkts.py
> > @@ -0,0 +1,56 @@
> > +#!/usr/bin/env python3
> > +
> > +import sys
> > +
> > +from scapy.all import RandMAC, RandIP, RandIP6, RandShort, fuzz from
> > +scapy.all import IPv6, Dot1Q, IP, Ether, UDP, TCP
> > +
> > +if len(sys.argv) < 2:
> > +print('usage: {} packets_count [fuzz]'.format(sys.argv[0]))
> > +sys.exit(1)
> > +
> > +tmpl = []
> > +
> > +if len(sys.argv) == 2:
> > +eth = Ether(dst='ff:ff:ff:ff:ff:ff')
> > +vlan = eth / Dot1Q(vlan=1)
> > +p = eth / IP() / TCP(sport=20, dport=80, flags='SA', window=8192)
> > +tmpl += [p.build().hex()]
> > +p = eth / IP() / UDP(sport=53, dport=53)
> > +tmpl += [p.build().hex()]
> > +p = eth / IP() / TCP(sport=20, dport=80, flags='S', window=8192)
> > +tmpl += [p.build().hex()]
> > +p = eth / IP() / UDP(sport=53, dport=53)
> > +tmpl += [p.build().hex()]
> > +p = vlan / IP() / UDP(sport=53, dport=53)
> > +tmpl += [p.build().hex()]
> > +p = vlan / IP() / TCP(sport=20, dport=80, flags='S', window=8192)
> > +tmpl += [p.build().hex()]
>
> Hardcoding the values here is not preferable as we wanted to test the 
> optimized implementations
> with various values contained inside the header.

Those hardcoded values comes from the pcap file that was previously used.
If you want to add more protocols, it is easier with this patch as you
only need to update some python script rather than rewrite a pcap
file.


> > +on_exit "pkill -f -x -9 '$PYTHON3 $srcdir/genpkts.py -1 fuzz'"
> > +($PYTHON3 $srcdir/genpkts.py -1 fuzz | while read pkt; do
> > + ovs-appctl netdev-dummy/receive p1 "$pkt" || break
> > + done) &
> > +
> >  AT_CHECK([ovs-appctl dpif-netdev/miniflow-parser-set autovalidator], [0], 
> > [dnl
> > Miniflow extract implementation set to autovalidator.
> >  ])
> >
> > -OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics | grep -oP
> > 'rx_packets=\s*\K\d+'` -ge 10])
>
> We should increase the packet count to at-least 10x the current number to 
> have a proper fuzzy testing and we have measured it would only take 10~ to 
> 15~ sec more. The current runtime did not catch issues when we purposely 
> broke the implementation and by allowing to run for 1 packets, it did 
> catch the induced error.

For me, the fuzzy testing does not have its place in a CI, because it
is not reproducible.
I let it in place and just made sure it would not reach the timeout.


On the system I used, this test takes 5s with 1k and timeouts with 10k.
So I guess the 10s/15s evaluation is dependent on the system.

I prefer to stick to current value.


Thanks.

-- 
David Marchand

___
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.

2021-12-02 Thread Ilya Maximets
On 12/2/21 12:47, David Marchand wrote:
> 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?

This will be done as part of "Prepare for 2.17.0." patch while preparing
for the actual release along with update for the kernel support list.

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.

2021-12-02 Thread David Marchand
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?


- there is one reference to 20.11 documentation in
Documentation/topics/userspace-tso.rst:__
https://doc.dpdk.org/guides-20.11/nics/overview.html


- Fedora spec still references 20.11:
rhel/openvswitch-fedora.spec.in:BuildRequires: dpdk-devel >= 20.11


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:

Reviewed-by: David Marchand 

Thanks Ian.


-- 
David Marchand

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


Re: [ovs-dev] [PATCH v1 8/8] odp-execute: Add ISA implementation of pop_vlan action.

2021-12-02 Thread 0-day Robot
Bleep bloop.  Greetings Emma Finn, 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.


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


Re: [ovs-dev] [PATCH v1 7/8] odp-execute: Add ISA implementation of actions.

2021-12-02 Thread 0-day Robot
Bleep bloop.  Greetings Emma Finn, 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.


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


Re: [ovs-dev] [PATCH v1 6/8] dpif-netdev: Add configure to enable autovalidator at build time.

2021-12-02 Thread 0-day Robot
Bleep bloop.  Greetings Emma Finn, 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.


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


Re: [ovs-dev] [PATCH v1 5/8] pmd.at: Add test-cases for ovs-actions commands.

2021-12-02 Thread 0-day Robot
Bleep bloop.  Greetings Emma Finn, 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.


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


Re: [ovs-dev] [PATCH v1 4/8] odp-execute: Add command to switch action implementation.

2021-12-02 Thread 0-day Robot
Bleep bloop.  Greetings Emma Finn, 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.


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


Re: [ovs-dev] [PATCH v1 3/8] odp-execute: Add auto validation function for actions.

2021-12-02 Thread 0-day Robot
Bleep bloop.  Greetings Emma Finn, 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.


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


Re: [ovs-dev] [PATCH v1 2/8] odp-execute: Add function pointer for pop_vlan action.

2021-12-02 Thread 0-day Robot
Bleep bloop.  Greetings Emma Finn, 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.


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


Re: [ovs-dev] [PATCH v1 1/8] odp-execute: Add function pointers to odp-execute for different action implementations.

2021-12-02 Thread 0-day Robot
Bleep bloop.  Greetings Emma Finn, 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.


build:
mv lib/nx-match.inc.tmp lib/nx-match.inc
depbase=`echo lib/nx-match.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/nx-match.lo -MD -MP -MF $depbase.Tpo -c -o lib/nx-match.lo 
lib/nx-match.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/nx-match.lo -MD -MP -MF lib/.deps/nx-match.Tpo 
-c lib/nx-match.c -o lib/nx-match.o
depbase=`echo lib/object-collection.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/object-collection.lo -MD -MP -MF $depbase.Tpo -c -o 
lib/object-collection.lo lib/object-collection.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/object-collection.lo -MD -MP -MF 
lib/.deps/object-collection.Tpo -c lib/object-collection.c -o 
lib/object-collection.o
depbase=`echo lib/odp-execute-private.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/odp-execute-private.lo -MD -MP -MF $depbase.Tpo -c -o 
lib/odp-execute-private.lo lib/odp-execute-private.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/odp-execute-private.lo -MD -MP -MF 
lib/.deps/odp-execute-private.Tpo -c lib/odp-execute-private.c -o 
lib/odp-execute-private.o
lib/odp-execute-private.c:32:31: error: 'rl' defined but not used 
[-Werror=unused-variable]
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
   ^
lib/odp-execute-private.c:43:17: error: 'active_action_impl_index' defined but 
not used [-Werror=unused-variable]
 static uint32_t active_action_impl_index;
 ^
cc1: all warnings being treated as errors
make[2]: *** [lib/odp-execute-private.lo] Error 1
make[2]: Leaving directory 
`/var/lib/jenkins/jobs/0day_robot_upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory 
`/var/lib/jenkins/jobs/0day_robot_upstream_build_from_pw/workspace'
make: *** [all] Error 2


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 v1 8/8] odp-execute: Add ISA implementation of pop_vlan action.

2021-12-02 Thread Emma Finn
This commit adds the AVX512 implementation of the pop_vlan action.
The implementation here is auto-validated by the miniflow
extract autovalidator, hence its correctness can be easily
tested and verified.

Signed-off-by: Emma Finn 
---
 lib/odp-execute-avx512.c | 76 
 1 file changed, 76 insertions(+)

diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c
index c46638e3f..9885c68e6 100644
--- a/lib/odp-execute-avx512.c
+++ b/lib/odp-execute-avx512.c
@@ -14,6 +14,11 @@
  * limitations under the License.
  */
 
+#ifdef __x86_64__
+/* Sparse cannot handle the AVX512 instructions. */
+#if !defined(__CHECKER__)
+
+
 #include 
 #include 
 
@@ -21,9 +26,75 @@
 #include "odp-netlink.h"
 #include "dp-packet.h"
 #include "openvswitch/vlog.h"
+#include "dpdk.h"
 
 #include "immintrin.h"
 
+VLOG_DEFINE_THIS_MODULE(odp_execute_avx512);
+BUILD_ASSERT_DECL(offsetof(struct dp_packet, l2_5_ofs) +
+  MEMBER_SIZEOF(struct dp_packet, l2_5_ofs) ==
+  offsetof(struct dp_packet, l3_ofs));
+
+BUILD_ASSERT_DECL(offsetof(struct dp_packet, l3_ofs) +
+   MEMBER_SIZEOF(struct dp_packet, l3_ofs) ==
+   offsetof(struct dp_packet, l4_ofs));
+
+static inline void ALWAYS_INLINE
+avx512_dp_packet_resize_l2(struct dp_packet *b, int increment)
+{
+/* update packet size/data pointers */
+dp_packet_set_data(b, (char *) dp_packet_data(b) - increment);
+dp_packet_set_size(b, dp_packet_size(b) + increment);
+
+/* Increment u16 packet offset values */
+const __m128i v_zeros = _mm_setzero_si128();
+const __m128i v_u16_max = _mm_cmpeq_epi16(v_zeros, v_zeros);
+
+/* Only these lanes can be incremented for push-VLAN action. */
+const uint8_t k_lanes = 0b1110;
+__m128i v_offset = _mm_set1_epi16(VLAN_HEADER_LEN);
+
+/* Load packet and compare with UINT16_MAX */
+void *adjust_ptr = >l2_pad_size;
+__m128i v_adjust_src = _mm_loadu_si128(adjust_ptr);
+__mmask8 k_cmp = _mm_mask_cmpneq_epu16_mask(k_lanes, v_adjust_src,
+v_u16_max);
+
+/* Add VLAN_HEADER_LEN using compare mask, store results. */
+__m128i v_adjust_wip = _mm_mask_sub_epi16(v_adjust_src, k_cmp,
+  v_adjust_src, v_offset);
+_mm_storeu_si128(adjust_ptr, v_adjust_wip);
+
+}
+
+static inline void ALWAYS_INLINE
+avx512_eth_pop_vlan(struct dp_packet *packet)
+{
+struct vlan_eth_header *veh = dp_packet_eth(packet);
+
+if (veh && dp_packet_size(packet) >= sizeof *veh &&
+eth_type_vlan(veh->veth_type)) {
+
+__m128i v_ether = _mm_loadu_si128((void *) veh);
+__m128i v_realign = _mm_alignr_epi8(v_ether, _mm_setzero_si128(),
+16 - VLAN_HEADER_LEN);
+_mm_storeu_si128((void *) veh, v_realign);
+avx512_dp_packet_resize_l2(packet, -VLAN_HEADER_LEN);
+
+}
+}
+
+static void
+action_avx512_pop_vlan(void *dp OVS_UNUSED, struct dp_packet_batch *batch,
+   const struct nlattr *a OVS_UNUSED,
+   bool should_steal OVS_UNUSED)
+{
+struct dp_packet *packet;
+
+DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
+avx512_eth_pop_vlan(packet);
+}
+}
 
 /* Probe functions to check ISA requirements. */
 static int32_t
@@ -64,5 +135,10 @@ int32_t
 action_avx512_init(struct odp_execute_action_impl *self)
 {
 avx512_isa_probe(0);
+self->funcs[OVS_ACTION_ATTR_POP_VLAN] = action_avx512_pop_vlan;
+
 return 0;
 }
+
+#endif
+#endif
-- 
2.25.1

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


[ovs-dev] [PATCH v1 7/8] odp-execute: Add ISA implementation of actions.

2021-12-02 Thread Emma Finn
This commit adds the AVX512 implementation of the action functionality.

Usage:
  $ ovs-appctl dpif-netdev/action-impl-set avx512

Signed-off-by: Emma Finn 
---
 lib/automake.mk   |  4 ++-
 lib/dpdk.c|  1 +
 lib/odp-execute-avx512.c  | 68 +++
 lib/odp-execute-private.c |  9 ++
 lib/odp-execute-private.h |  9 ++
 5 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 lib/odp-execute-avx512.c

diff --git a/lib/automake.mk b/lib/automake.mk
index 16087031f..34c03da45 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -32,6 +32,7 @@ lib_libopenvswitch_la_LIBADD += lib/libopenvswitchavx512.la
 lib_libopenvswitchavx512_la_CFLAGS = \
-mavx512f \
-mavx512bw \
+   -mavx512vl \
-mavx512dq \
-mbmi \
-mbmi2 \
@@ -40,7 +41,8 @@ lib_libopenvswitchavx512_la_CFLAGS = \
 lib_libopenvswitchavx512_la_SOURCES = \
lib/dpif-netdev-lookup-avx512-gather.c \
lib/dpif-netdev-extract-avx512.c \
-   lib/dpif-netdev-avx512.c
+   lib/dpif-netdev-avx512.c \
+   lib/odp-execute-avx512.c
 lib_libopenvswitchavx512_la_LDFLAGS = \
-static
 endif
diff --git a/lib/dpdk.c b/lib/dpdk.c
index b2ef31cd2..825e2daad 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -630,6 +630,7 @@ dpdk_get_cpu_has_isa(const char *arch, const char *feature)
 CHECK_CPU_FEATURE(feature, "avx512vbmi", RTE_CPUFLAG_AVX512VBMI);
 CHECK_CPU_FEATURE(feature, "avx512vpopcntdq", RTE_CPUFLAG_AVX512VPOPCNTDQ);
 CHECK_CPU_FEATURE(feature, "bmi2", RTE_CPUFLAG_BMI2);
+CHECK_CPU_FEATURE(feature, "avx512vl", RTE_CPUFLAG_AVX512VL);
 #endif
 
 VLOG_WARN("Unknown CPU arch,feature: %s,%s. Returning not supported.\n",
diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c
new file mode 100644
index 0..c46638e3f
--- /dev/null
+++ b/lib/odp-execute-avx512.c
@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) 2021 Intel.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+
+#include "odp-execute-private.h"
+#include "odp-netlink.h"
+#include "dp-packet.h"
+#include "openvswitch/vlog.h"
+
+#include "immintrin.h"
+
+
+/* Probe functions to check ISA requirements. */
+static int32_t
+avx512_isa_probe(uint32_t needs_vbmi)
+{
+static const char *isa_required[] = {
+"avx512f",
+"avx512bw",
+"bmi2",
+"avx512vl"
+};
+
+int32_t ret = 0;
+for (uint32_t i = 0; i < ARRAY_SIZE(isa_required); i++) {
+if (!dpdk_get_cpu_has_isa("x86_64", isa_required[i])) {
+ret = -ENOTSUP;
+}
+}
+
+if (needs_vbmi) {
+if (!dpdk_get_cpu_has_isa("x86_64", "avx512vbmi")) {
+ret = -ENOTSUP;
+}
+}
+
+return ret;
+}
+
+int32_t
+action_avx512_probe(void)
+{
+const uint32_t needs_vbmi = 0;
+return avx512_isa_probe(needs_vbmi);
+}
+
+
+int32_t
+action_avx512_init(struct odp_execute_action_impl *self)
+{
+avx512_isa_probe(0);
+return 0;
+}
diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c
index 3d1176cdd..bdb8d1e1e 100644
--- a/lib/odp-execute-private.c
+++ b/lib/odp-execute-private.c
@@ -45,6 +45,15 @@ static struct odp_execute_action_impl action_impls[] = {
 .probe = NULL,
 .init_func = action_autoval_init,
 },
+
+#if (__x86_64__ && HAVE_AVX512F && HAVE_LD_AVX512_GOOD && __SSE4_2__)
+[ACTION_IMPL_AVX512] = {
+.available = 1,
+.name = "avx512",
+.probe = action_avx512_probe,
+.init_func = action_avx512_init,
+},
+#endif
 };
 
 static uint32_t active_action_impl_index;
diff --git a/lib/odp-execute-private.h b/lib/odp-execute-private.h
index d49714bd2..5ba2868bf 100644
--- a/lib/odp-execute-private.h
+++ b/lib/odp-execute-private.h
@@ -73,6 +73,9 @@ enum odp_execute_action_impl_idx {
  * Do not change the autovalidator position in this list without updating
  * the define below.
  */
+#if (__x86_64__ && HAVE_AVX512F && HAVE_LD_AVX512_GOOD && __SSE4_2__)
+ACTION_IMPL_AVX512,
+#endif
 
 ACTION_IMPL_MAX,
 };
@@ -98,4 +101,10 @@ int32_t odp_execute_action_set(const char *name,
  */
 int32_t odp_action_scalar_init(struct odp_execute_action_impl *self);
 
+/* Init function for the optimized with AVX512 actions. */
+int32_t action_avx512_init(struct odp_execute_action_impl *self);
+
+/* Probe function to check ISA requirements. */
+int32_t action_avx512_probe(void);
+
 #endif 

[ovs-dev] [PATCH v1 5/8] pmd.at: Add test-cases for ovs-actions commands.

2021-12-02 Thread Emma Finn
From: Kumar Amber 

Added separate test-case for ovs-actions get/set commands:
1023: PMD - ovs-actions configuration

The above added tests are to test the commands which are used
to either get or set the ovs-actions function pointers to
various different implementations like AVX512 or auto-validator
based on different CPU ISA supported.

Signed-off-by: Kumar Amber 
Signed-off-by: Emma Finn 
---
 tests/pmd.at | 20 
 1 file changed, 20 insertions(+)

diff --git a/tests/pmd.at b/tests/pmd.at
index c875a744f..4384652ff 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -1160,3 +1160,23 @@ ovs-appctl: ovs-vswitchd: server returned an error
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([PMD - ovs-actions configuration])
+OVS_VSWITCHD_START([], [], [], [--dummy-numa 0,0])
+AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dummy-pmd])
+
+AT_CHECK([ovs-vsctl show], [], [stdout])
+AT_CHECK([ovs-appctl dpif-netdev/action-impl-get | grep "scalar"], [], [dnl
+  scalar (available: True, active: True)
+])
+
+AT_CHECK([ovs-appctl dpif-netdev/action-impl-get | grep "autovalidator"], [], 
[dnl
+  autovalidator (available: True, active: False)
+])
+
+AT_CHECK([ovs-appctl dpif-netdev/action-impl-set scalar], [0], [dnl
+action implementation set to scalar.
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
\ No newline at end of file
-- 
2.25.1

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


[ovs-dev] [PATCH v1 6/8] dpif-netdev: Add configure to enable autovalidator at build time.

2021-12-02 Thread Emma Finn
From: Kumar Amber 

This commit adds a new command to allow the user to enable
autovalidatior by default at build time thus allowing for
runnig unit test by default.

 $ ./configure --enable-actions-default-autovalidator

Signed-off-by: Kumar Amber 
Signed-off-by: Emma Finn 
---
 acinclude.m4  | 17 +
 configure.ac  |  1 +
 lib/odp-execute.c |  4 
 3 files changed, 22 insertions(+)

diff --git a/acinclude.m4 b/acinclude.m4
index 8ab690f47..d878ea4e7 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -14,6 +14,23 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+dnl Set OVS Actions Autovalidator as default action at compile time?
+dnl This enables automatically running all unit tests with all actions
+dnl implementations.
+AC_DEFUN([OVS_CHECK_ACTIONS_AUTOVALIDATOR], [
+  AC_ARG_ENABLE([actions-default-autovalidator],
+[AC_HELP_STRING([--enable-actions-default-autovalidator], 
[Enable actions autovalidator as default ovs actions implementation.])],
+[autovalidator=yes],[autovalidator=no])
+  AC_MSG_CHECKING([whether actions Autovalidator is default implementation])
+  if test "$autovalidator" != yes; then
+AC_MSG_RESULT([no])
+  else
+OVS_CFLAGS="$OVS_CFLAGS -DACTIONS_AUTOVALIDATOR_DEFAULT"
+AC_MSG_RESULT([yes])
+  fi
+])
+
+
 dnl Set OVS MFEX Autovalidator as default miniflow extract at compile time?
 dnl This enables automatically running all unit tests with all MFEX
 dnl implementations.
diff --git a/configure.ac b/configure.ac
index eaa9bf7ee..bfd0a9aff 100644
--- a/configure.ac
+++ b/configure.ac
@@ -185,6 +185,7 @@ OVS_CTAGS_IDENTIFIERS
 OVS_CHECK_DPCLS_AUTOVALIDATOR
 OVS_CHECK_DPIF_AVX512_DEFAULT
 OVS_CHECK_MFEX_AUTOVALIDATOR
+OVS_CHECK_ACTIONS_AUTOVALIDATOR
 OVS_CHECK_AVX512
 
 AC_ARG_VAR(KARCH, [Kernel Architecture String])
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index ab051aecc..1bc9fae09 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -865,7 +865,11 @@ odp_execute_init(void)
 static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
 if (ovsthread_once_start()) {
 odp_execute_action_init();
+#ifdef ACTIONS_AUTOVALIDATOR_DEFAULT
+odp_actions_impl_set("autovalidator");
+#else
 odp_actions_impl_set("scalar");
+#endif
 ovsthread_once_done();
 }
 }
-- 
2.25.1

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


[ovs-dev] [PATCH v1 3/8] odp-execute: Add auto validation function for actions.

2021-12-02 Thread Emma Finn
This commit introduced the auto-validation function which
allows users to compare the batch of packets obtained from
different action implementations against the linear
action implementation.

The autovalidator function can be triggered at runtime using the
following command:

$ ovs-appctl dpif-netdev/action-impl-set autovalidator

Signed-off-by: Emma Finn 
---
 lib/dp-packet.c   |  23 +
 lib/dp-packet.h   |   5 ++
 lib/odp-execute-private.c | 100 +-
 lib/odp-execute-private.h |   3 ++
 4 files changed, 130 insertions(+), 1 deletion(-)

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 72f6d09ac..1e4ff35ef 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -506,3 +506,26 @@ dp_packet_resize_l2(struct dp_packet *b, int increment)
 dp_packet_adjust_layer_offset(>l2_5_ofs, increment);
 return dp_packet_data(b);
 }
+
+bool
+dp_packet_compare_and_log(struct dp_packet *good, struct dp_packet *test,
+  struct ds *err_str)
+{
+if ((good->l2_pad_size != test->l2_pad_size) ||
+(good->l2_5_ofs != test->l2_5_ofs) ||
+(good->l3_ofs != test->l3_ofs) ||
+(good->l4_ofs != test->l4_ofs)) {
+ds_put_format(err_str, "Autovalidation packet offsets failed"
+  "\n");
+ds_put_format(err_str, "Good offsets: l2_pad_size %u,"
+  " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
+  good->l2_pad_size, good->l2_5_ofs,
+  good->l3_ofs, good->l4_ofs);
+ds_put_format(err_str, "Test offsets: l2_pad_size %u,"
+  " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
+  test->l2_pad_size, test->l2_5_ofs,
+  test->l3_ofs, test->l4_ofs);
+return false;
+}
+return true;
+}
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 3dc582fbf..cc4c0d6da 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -234,6 +234,11 @@ void *dp_packet_steal_data(struct dp_packet *);
 static inline bool dp_packet_equal(const struct dp_packet *,
const struct dp_packet *);
 
+
+bool dp_packet_compare_and_log(struct dp_packet *good,
+   struct dp_packet *test,
+   struct ds *err_str);
+
 
 /* Frees memory that 'b' points to, as well as 'b' itself. */
 static inline void
diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c
index 1b02be223..d5631ba0a 100644
--- a/lib/odp-execute-private.c
+++ b/lib/odp-execute-private.c
@@ -38,6 +38,13 @@ static struct odp_execute_action_impl action_impls[] = {
 .probe = NULL,
 .init_func = odp_action_scalar_init,
 },
+
+[ACTION_IMPL_AUTOVALIDATOR] = {
+.available = 1,
+.name = "autovalidator",
+.probe = NULL,
+.init_func = action_autoval_init,
+},
 };
 
 static uint32_t active_action_impl_index;
@@ -84,4 +91,95 @@ odp_execute_action_init(void)
 action_impls[i].init_func(_impls[i]);
 }
 }
-}
\ No newline at end of file
+}
+
+/* Init sequence required to be scalar first to pick up the default scalar
+* implementations, allowing over-riding of the optimized functions later.
+*/
+BUILD_ASSERT_DECL(ACTION_IMPL_SCALAR == 0);
+BUILD_ASSERT_DECL(ACTION_IMPL_AUTOVALIDATOR == 1);
+
+/* Loop over packets, and validate each one for the given action. */
+static void
+action_autoval_generic(void *dp OVS_UNUSED, struct dp_packet_batch *batch,
+   const struct nlattr *a, bool should_steal)
+{
+uint32_t failed = 0;
+
+int type = nl_attr_type(a);
+enum ovs_action_attr attr_type = (enum ovs_action_attr) type;
+
+struct odp_execute_action_impl *scalar = _impls[ACTION_IMPL_SCALAR];
+
+struct dp_packet_batch good_batch;
+dp_packet_batch_clone(_batch, batch);
+
+scalar->funcs[attr_type](NULL, _batch, a, should_steal);
+
+for (uint32_t impl = ACTION_IMPL_BEGIN; impl < ACTION_IMPL_MAX; impl++) {
+/* Clone original batch and execute implementation under test. */
+struct dp_packet_batch test_batch;
+dp_packet_batch_clone(_batch, batch);
+action_impls[impl].funcs[attr_type](NULL, _batch, a,
+should_steal);
+
+/* Loop over implementations, checking each one. */
+for (uint32_t pidx = 0; pidx < batch->count; pidx++) {
+struct dp_packet *good_pkt = good_batch.packets[pidx];
+struct dp_packet *test_pkt = test_batch.packets[pidx];
+
+struct ds log_msg = DS_EMPTY_INITIALIZER;
+
+/* Compare packet length and payload contents. */
+bool eq = dp_packet_equal(good_pkt, test_pkt);
+
+if (!eq) {
+ds_put_format(_msg, "Packet: %d\nAction : ", pidx);
+format_odp_actions(_msg, a, a->nla_len, NULL);
+

[ovs-dev] [PATCH v1 4/8] odp-execute: Add command to switch action implementation.

2021-12-02 Thread Emma Finn
This commit adds a new command to allow the user to switch
the active action implementation at runtime. A probe function
is executed before switching the implementation, to ensure
the CPU is capable of running the ISA required.

Usage:
  $ ovs-appctl dpif-netdev/action-impl-set scalar

This commit also adds a new command to retrieve the list of available
action implementations. This can be used by to check what implementations
of actions are available and what implementation is active during runtime.

Usage:
   $ ovs-appctl dpif-netdev/action-impl-get

Signed-off-by: Emma Finn 
---
 lib/dpif-netdev.c | 39 +++
 lib/odp-execute-private.c | 30 ++
 lib/odp-execute.c | 11 +++
 lib/odp-execute.h |  5 +
 4 files changed, 85 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 277e0d6c3..9684bbbc4 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -59,6 +59,7 @@
 #include "netdev-vport.h"
 #include "netlink.h"
 #include "odp-execute.h"
+#include "odp-execute-private.h"
 #include "odp-util.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/list.h"
@@ -1310,6 +1311,38 @@ error:
 ds_destroy();
 }
 
+static void
+action_impl_set(struct unixctl_conn *conn, int argc OVS_UNUSED,
+const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
+{
+struct ds reply = DS_EMPTY_INITIALIZER;
+
+int32_t err = odp_actions_impl_set(argv[1]);
+if (err) {
+ds_put_format(, "action implementation %s not found.\n",
+  argv[1]);
+const char *reply_str = ds_cstr();
+unixctl_command_reply_error(conn, reply_str);
+VLOG_ERR("%s", reply_str);
+ds_destroy();
+return;
+}
+
+ds_put_format(, "action implementation set to %s.\n", argv[1]);
+unixctl_command_reply(conn, ds_cstr());
+ds_destroy();
+}
+
+static void
+action_impl_get(struct unixctl_conn *conn, int argc OVS_UNUSED,
+const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
+{
+struct ds reply = DS_EMPTY_INITIALIZER;
+odp_execute_action_get();
+unixctl_command_reply(conn, ds_cstr());
+ds_destroy();
+}
+
 static void
 dpif_netdev_pmd_rebalance(struct unixctl_conn *conn, int argc,
   const char *argv[], void *aux OVS_UNUSED)
@@ -1547,6 +1580,12 @@ dpif_netdev_init(void)
 unixctl_command_register("dpif-netdev/miniflow-parser-get", "",
  0, 0, dpif_miniflow_extract_impl_get,
  NULL);
+unixctl_command_register("dpif-netdev/action-impl-set", "name",
+ 1, 1, action_impl_set,
+ NULL);
+unixctl_command_register("dpif-netdev/action-impl-get", "",
+ 0, 0, action_impl_get,
+ NULL);
 return 0;
 }
 
diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c
index d5631ba0a..3d1176cdd 100644
--- a/lib/odp-execute-private.c
+++ b/lib/odp-execute-private.c
@@ -59,6 +59,36 @@ action_impl_copy_funcs(struct odp_execute_action_impl *to,
 }
 }
 
+void
+odp_execute_action_get(struct ds *string)
+{
+uint32_t i;
+
+ds_put_cstr(string, "Available Actions implementations:\n");
+for (i = 0; i < ACTION_IMPL_MAX; i++) {
+ds_put_format(string, "  %s (available: %s, active: %s)\n",
+  action_impls[i].name,
+  action_impls[i].available ? "True" : "False",
+  i == active_action_impl_index ? "True" : "False");
+}
+}
+
+int32_t
+odp_execute_action_set(const char *name,
+   struct odp_execute_action_impl *active)
+{
+uint32_t i;
+for (i = 0; i < ACTION_IMPL_MAX; i++) {
+/* string compare, and set ptrs *atomically*. */
+if (strcmp(action_impls[i].name, name) == 0) {
+action_impl_copy_funcs(active, _impls[i]);
+active_action_impl_index = i;
+return 0;
+}
+}
+return -1;
+}
+
 void
 odp_execute_action_init(void)
 {
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index a447b4391..ab051aecc 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -870,6 +870,17 @@ odp_execute_init(void)
 }
 }
 
+int32_t
+odp_actions_impl_set(const char *name)
+{
+
+int err = odp_execute_action_set(name, _active_impl);
+if (err) {
+VLOG_ERR("error %d from action set to %s\n", err, name);
+return -1;
+}
+return 0;
+}
 
 /* Executes all of the 'actions_len' bytes of datapath actions in 'actions' on
  * the packets in 'batch'.  If 'steal' is true, possibly modifies and
diff --git a/lib/odp-execute.h b/lib/odp-execute.h
index c4f5303e7..4f4cdc4ac 100644
--- a/lib/odp-execute.h
+++ b/lib/odp-execute.h
@@ -23,6 +23,7 @@
 #include 
 #include "openvswitch/types.h"
 
+struct ds;
 struct nlattr;
 struct dp_packet;
 struct pkt_metadata;
@@ -32,6 +33,10 @@ 

[ovs-dev] [PATCH v1 0/8] Actions Infrastructure + Optimizations

2021-12-02 Thread Emma Finn
This patchset introduces actions infrastructure changes
which allows the user to choose between different action
implementations based on CPU ISA by using different commands.
The Infrastructure also provides a way to check the correctness of
the ISA optimized action version against the scalar
version.
This patchset also introduces an optimized version of the pop_vlan
action.

Emma Finn (6):
  odp-execute: Add function pointers to odp-execute for different action
implementations.
  odp-execute: Add function pointer for pop_vlan action.
  odp-execute: Add auto validation function for actions.
  odp-execute: Add command to switch action implementation.
  odp-execute: Add ISA implementation of actions.
  odp-execute: Add ISA implementation of pop_vlan action.

Kumar Amber (2):
  pmd.at: Add test-cases for ovs-actions commands.
  dpif-netdev: Add configure to enable autovalidator at build time.

 acinclude.m4  |  17 +++
 configure.ac  |   1 +
 lib/automake.mk   |   6 +-
 lib/dp-packet.c   |  23 
 lib/dp-packet.h   |   5 +
 lib/dpdk.c|   1 +
 lib/dpif-netdev.c |  41 +++
 lib/odp-execute-avx512.c  | 144 
 lib/odp-execute-private.c | 224 ++
 lib/odp-execute-private.h | 110 +++
 lib/odp-execute.c |  84 --
 lib/odp-execute.h |   9 ++
 tests/pmd.at  |  20 
 13 files changed, 673 insertions(+), 12 deletions(-)
 create mode 100644 lib/odp-execute-avx512.c
 create mode 100644 lib/odp-execute-private.c
 create mode 100644 lib/odp-execute-private.h

-- 
2.25.1

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


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

2021-12-02 Thread Mohammad Heib
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 | 119 ++
 tests/ovn.at  |   8 +++
 4 files changed, 167 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..44b6c30d1 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(const ovs_be32);
 
 #endif /* ovn/actions.h */
diff --git a/lib/actions.c b/lib/actions.c
index 6b9a426ae..3d6b33ad6 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -4315,3 +4315,122 @@ ovnacts_free(struct ovnact *ovnacts, size_t ovnacts_len)
 }
 }
 }
+
+/* Return ovn action opcode string representation.*/
+char *
+ovnact_op_to_string(const ovs_be32 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:
+ds_put_cstr(_str, "ND_NA");
+break;
+
+case ACTION_OPCODE_ND_NA_ROUTER:
+ds_put_cstr(_str, "ND_NA_ROUTER");
+break;
+
+case ACTION_OPCODE_PUT_ND:
+ds_put_cstr(_str, "PUT_ND");
+break;
+
+case ACTION_OPCODE_PUT_FDB:
+ds_put_cstr(_str, "PUT_FDB");
+break;
+
+case ACTION_OPCODE_PUT_DHCPV6_OPTS:
+

[ovs-dev] [PATCH v1 2/8] odp-execute: Add function pointer for pop_vlan action.

2021-12-02 Thread Emma Finn
This commit removes the pop_vlan action from the large switch
and creates a separate function for batched processing. A function
pointer is also added to call the new batched function for the pop_vlan
action.

Signed-off-by: Emma Finn 
---
 lib/odp-execute.c | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 94adebd4c..a447b4391 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -831,6 +831,28 @@ requires_datapath_assistance(const struct nlattr *a)
 return false;
 }
 
+static void
+action_pop_vlan(void *dp OVS_UNUSED, struct dp_packet_batch *batch,
+const struct nlattr *a OVS_UNUSED,
+bool should_steal OVS_UNUSED)
+{
+struct dp_packet *packet;
+DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
+eth_pop_vlan(packet);
+}
+}
+
+/* Implementation of the scalar actions impl init function. Build up the
+ * array of func ptrs here.
+ */
+int32_t
+odp_action_scalar_init(struct odp_execute_action_impl *self)
+{
+self->funcs[OVS_ACTION_ATTR_POP_VLAN] = action_pop_vlan;
+
+return 0;
+}
+
 /* The active function pointers on the datapath. ISA optimized implementations
  * are enabled by plugging them into this static arary, which is consulted when
  * applying actions on the datapath.
@@ -963,12 +985,6 @@ odp_execute_actions(void *dp, struct dp_packet_batch 
*batch, bool steal,
 break;
 }
 
-case OVS_ACTION_ATTR_POP_VLAN:
-DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
-eth_pop_vlan(packet);
-}
-break;
-
 case OVS_ACTION_ATTR_PUSH_MPLS: {
 const struct ovs_action_push_mpls *mpls = nl_attr_get(a);
 
@@ -1101,6 +1117,7 @@ odp_execute_actions(void *dp, struct dp_packet_batch 
*batch, bool steal,
 }
 case OVS_ACTION_ATTR_OUTPUT:
 case OVS_ACTION_ATTR_LB_OUTPUT:
+case OVS_ACTION_ATTR_POP_VLAN:
 case OVS_ACTION_ATTR_TUNNEL_PUSH:
 case OVS_ACTION_ATTR_TUNNEL_POP:
 case OVS_ACTION_ATTR_USERSPACE:
-- 
2.25.1

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


[ovs-dev] [PATCH v1 1/8] odp-execute: Add function pointers to odp-execute for different action implementations.

2021-12-02 Thread Emma Finn
This commit introduces the initial infrastructure required to allow
different implementations for OvS actions. The patch introduces action
function pointers which allows user to switch between different action
implementations available. This will allow for more performance and flexibility
so the user can choose the action implementation to best suite their use case.

Signed-off-by: Emma Finn 
---
 lib/automake.mk   |  2 +
 lib/dpif-netdev.c |  2 +
 lib/odp-execute-private.c | 87 ++
 lib/odp-execute-private.h | 98 +++
 lib/odp-execute.c | 40 ++--
 lib/odp-execute.h |  4 ++
 6 files changed, 228 insertions(+), 5 deletions(-)
 create mode 100644 lib/odp-execute-private.c
 create mode 100644 lib/odp-execute-private.h

diff --git a/lib/automake.mk b/lib/automake.mk
index 46f869a33..16087031f 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -201,6 +201,8 @@ lib_libopenvswitch_la_SOURCES = \
lib/nx-match.h \
lib/object-collection.c \
lib/object-collection.h \
+   lib/odp-execute-private.c \
+   lib/odp-execute-private.h \
lib/odp-execute.c \
lib/odp-execute.h \
lib/odp-util.c \
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 69d7ec26e..277e0d6c3 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1598,6 +1598,8 @@ create_dpif_netdev(struct dp_netdev *dp)
 dpif->dp = dp;
 dpif->last_port_seq = seq_read(dp->port_seq);
 
+odp_execute_init();
+
 return >dpif;
 }
 
diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c
new file mode 100644
index 0..1b02be223
--- /dev/null
+++ b/lib/odp-execute-private.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2021 Intel.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "dpdk.h"
+
+#include "openvswitch/vlog.h"
+#include "odp-execute-private.h"
+#include "odp-netlink.h"
+#include "dp-packet.h"
+#include "odp-util.h"
+
+
+int32_t action_autoval_init(struct odp_execute_action_impl *self);
+VLOG_DEFINE_THIS_MODULE(odp_execute_private);
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+
+static struct odp_execute_action_impl action_impls[] = {
+[ACTION_IMPL_SCALAR] = {
+.available = 1,
+.name = "scalar",
+.probe = NULL,
+.init_func = odp_action_scalar_init,
+},
+};
+
+static uint32_t active_action_impl_index;
+
+static void
+action_impl_copy_funcs(struct odp_execute_action_impl *to,
+   const struct odp_execute_action_impl *from)
+{
+for (uint32_t i = 0; i < __OVS_KEY_ATTR_MAX; i++) {
+atomic_uintptr_t *func = (void *) >funcs[i];
+atomic_store_relaxed(func, (uintptr_t) from->funcs[i]);
+}
+}
+
+void
+odp_execute_action_init(void)
+{
+/* Call probe on each impl, and cache the result. */
+for (int i = 0; i < ACTION_IMPL_MAX; i++) {
+bool avail = true;
+if (action_impls[i].probe) {
+/* Return zero is success, non-zero means error. */
+avail = (action_impls[i].probe() == 0);
+}
+VLOG_INFO("Action implementation %s (available: %s)\n",
+  action_impls[i].name, avail ? "available" : "not available");
+action_impls[i].available = avail;
+}
+
+uint32_t i;
+for (i = 0; i < ACTION_IMPL_MAX; i++) {
+/* Each impl's function array is initialized to reflect the scalar
+ * implementation. This simplifies adding optimized implementations,
+ * as the autovalidator can always compare all actions.
+ *
+ * Below copies the scalar functions to all other implementations.
+ */
+if (i != ACTION_IMPL_SCALAR) {
+action_impl_copy_funcs(_impls[i],
+   _impls[ACTION_IMPL_SCALAR]);
+}
+
+if (action_impls[i].init_func) {
+action_impls[i].init_func(_impls[i]);
+}
+}
+}
\ No newline at end of file
diff --git a/lib/odp-execute-private.h b/lib/odp-execute-private.h
new file mode 100644
index 0..c2e86bbee
--- /dev/null
+++ b/lib/odp-execute-private.h
@@ -0,0 +1,98 @@
+/*
+ * Copyright (c) 2021 Intel.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *

[ovs-dev] [PATCH v4] dpcls: Change info-get function to fetch dpcls usage stats.

2021-12-02 Thread Kumar Amber
Modified the dplcs info-get command output to include
the count for different dpcls implementations.

$ovs-appctl dpif-netdev/subtable-lookup-prio-get

Available dpcls implementations:
  autovalidator (Use count: 1, Priority: 5)
  generic (Use count: 0, Priority: 1)
  avx512_gather (Use count: 0, Priority: 3)

Test case to verify changes:
1021: PMD - dpcls configuration ok

Signed-off-by: Kumar Amber 
Signed-off-by: Harry van Haaren 
Co-authored-by: Harry van Haaren 

---
v4:
- Fix comments on the patch.
- Change API from an overloaded method of counting, to returning the
  old and new subtable structs. This allows the caller to identify the
  modified subtable implementations, and update the statistics accordingly.
v3:
- Fix comments on the patch.
- Function API remains same, see discussion on OVS ML here:
  "https://mail.openvswitch.org/pipermail/ovs-dev/2021-October/388737.html;
v2:
- Dependency merged rebased to master.

---
---
 Documentation/topics/dpdk/bridge.rst | 16 +++
 lib/dpif-netdev-lookup.c | 70 +++-
 lib/dpif-netdev-lookup.h | 20 +++-
 lib/dpif-netdev.c| 61 +---
 tests/pmd.at | 16 +++
 5 files changed, 137 insertions(+), 46 deletions(-)

diff --git a/Documentation/topics/dpdk/bridge.rst 
b/Documentation/topics/dpdk/bridge.rst
index f645b9ade..63a54da1c 100644
--- a/Documentation/topics/dpdk/bridge.rst
+++ b/Documentation/topics/dpdk/bridge.rst
@@ -156,10 +156,10 @@ OVS provides multiple implementations of dpcls. The 
following command enables
 the user to check what implementations are available in a running instance ::
 
 $ ovs-appctl dpif-netdev/subtable-lookup-prio-get
-Available lookup functions (priority : name)
-0 : autovalidator
-1 : generic
-0 : avx512_gather
+Available dpcls implementations:
+autovalidator (Use count: 1, Priority: 5)
+generic (Use count: 0, Priority: 1)
+avx512_gather (Use count: 0, Priority: 3)
 
 To set the priority of a lookup function, run the ``prio-set`` command ::
 
@@ -172,10 +172,10 @@ function due to the command being run. To verify the 
prioritization, re-run the
 get command, note the updated priority of the ``avx512_gather`` function ::
 
 $ ovs-appctl dpif-netdev/subtable-lookup-prio-get
-Available lookup functions (priority : name)
-0 : autovalidator
-1 : generic
-5 : avx512_gather
+Available dpcls implementations:
+autovalidator (Use count: 0, Priority: 0)
+generic (Use count: 0, Priority: 0)
+avx512_gather (Use count: 1, Priority: 5)
 
 If two lookup functions have the same priority, the first one in the list is
 chosen, and the 2nd occurance of that priority is not used. Put in logical
diff --git a/lib/dpif-netdev-lookup.c b/lib/dpif-netdev-lookup.c
index bd0a99abe..0aa79e27c 100644
--- a/lib/dpif-netdev-lookup.c
+++ b/lib/dpif-netdev-lookup.c
@@ -36,18 +36,21 @@ static struct dpcls_subtable_lookup_info_t 
subtable_lookups[] = {
 { .prio = 0,
 #endif
   .probe = dpcls_subtable_autovalidator_probe,
-  .name = "autovalidator", },
+  .name = "autovalidator",
+  .usage_cnt = ATOMIC_COUNT_INIT(0), },
 
 /* The default scalar C code implementation. */
 { .prio = 1,
   .probe = dpcls_subtable_generic_probe,
-  .name = "generic", },
+  .name = "generic",
+  .usage_cnt = ATOMIC_COUNT_INIT(0), },
 
 #if (__x86_64__ && HAVE_AVX512F && HAVE_LD_AVX512_GOOD && __SSE4_2__)
 /* Only available on x86_64 bit builds with SSE 4.2 used for OVS core. */
 { .prio = 0,
   .probe = dpcls_subtable_avx512_gather_probe,
-  .name = "avx512_gather", },
+  .name = "avx512_gather",
+  .usage_cnt = ATOMIC_COUNT_INIT(0), },
 #else
 /* Disabling AVX512 at compile time, as compile time requirements not met.
  * This could be due to a number of reasons:
@@ -93,27 +96,46 @@ dpcls_subtable_set_prio(const char *name, uint8_t priority)
 }
 
 dpcls_subtable_lookup_func
-dpcls_subtable_get_best_impl(uint32_t u0_bit_count, uint32_t u1_bit_count)
+dpcls_subtable_get_best_impl(uint32_t u0_bit_count, uint32_t u1_bit_count,
+ dpcls_subtable_lookup_func old_func,
+ struct dpcls_subtable_lookup_info_t **old_info,
+ struct dpcls_subtable_lookup_info_t **new_info)
 {
 /* Iter over each subtable impl, and get highest priority one. */
 int32_t prio = -1;
 const char *name = NULL;
+int best_idx = 0;
 dpcls_subtable_lookup_func best_func = NULL;
 
 for (int i = 0; i < ARRAY_SIZE(subtable_lookups); i++) {
 int32_t probed_prio = subtable_lookups[i].prio;
+dpcls_subtable_lookup_func probed_func;
+
+probed_func = subtable_lookups[i].probe(u0_bit_count,
+

[ovs-dev] [PATCH v1] dpcls: Change info-get function to fetch dpcls usage stats.

2021-12-02 Thread Kumar Amber
Modified the dplcs info-get command output to include
the count for different dpcls implementations.

$ovs-appctl dpif-netdev/subtable-lookup-prio-get

Available dpcls implementations:
  autovalidator (Use count: 1, Priority: 5)
  generic (Use count: 0, Priority: 1)
  avx512_gather (Use count: 0, Priority: 3)

Test case to verify changes:
1021: PMD - dpcls configuration ok

Signed-off-by: Kumar Amber 
Signed-off-by: Harry van Haaren 
Co-authored-by: Harry van Haaren 

---
v4:
- Fix comments on the patch.
- Change API from an overloaded method of counting, to returning the
  old and new subtable structs. This allows the caller to identify the
  modified subtable implementations, and update the statistics accordingly.
v3:
- Fix comments on the patch.
- Function API remains same, see discussion on OVS ML here:
  "https://mail.openvswitch.org/pipermail/ovs-dev/2021-October/388737.html;
v2:
- Dependency merged rebased to master.

---
---
 Documentation/topics/dpdk/bridge.rst | 16 +++
 lib/dpif-netdev-lookup.c | 70 +++-
 lib/dpif-netdev-lookup.h | 20 +++-
 lib/dpif-netdev.c| 61 +---
 tests/pmd.at | 16 +++
 5 files changed, 137 insertions(+), 46 deletions(-)

diff --git a/Documentation/topics/dpdk/bridge.rst 
b/Documentation/topics/dpdk/bridge.rst
index f645b9ade..63a54da1c 100644
--- a/Documentation/topics/dpdk/bridge.rst
+++ b/Documentation/topics/dpdk/bridge.rst
@@ -156,10 +156,10 @@ OVS provides multiple implementations of dpcls. The 
following command enables
 the user to check what implementations are available in a running instance ::
 
 $ ovs-appctl dpif-netdev/subtable-lookup-prio-get
-Available lookup functions (priority : name)
-0 : autovalidator
-1 : generic
-0 : avx512_gather
+Available dpcls implementations:
+autovalidator (Use count: 1, Priority: 5)
+generic (Use count: 0, Priority: 1)
+avx512_gather (Use count: 0, Priority: 3)
 
 To set the priority of a lookup function, run the ``prio-set`` command ::
 
@@ -172,10 +172,10 @@ function due to the command being run. To verify the 
prioritization, re-run the
 get command, note the updated priority of the ``avx512_gather`` function ::
 
 $ ovs-appctl dpif-netdev/subtable-lookup-prio-get
-Available lookup functions (priority : name)
-0 : autovalidator
-1 : generic
-5 : avx512_gather
+Available dpcls implementations:
+autovalidator (Use count: 0, Priority: 0)
+generic (Use count: 0, Priority: 0)
+avx512_gather (Use count: 1, Priority: 5)
 
 If two lookup functions have the same priority, the first one in the list is
 chosen, and the 2nd occurance of that priority is not used. Put in logical
diff --git a/lib/dpif-netdev-lookup.c b/lib/dpif-netdev-lookup.c
index bd0a99abe..0aa79e27c 100644
--- a/lib/dpif-netdev-lookup.c
+++ b/lib/dpif-netdev-lookup.c
@@ -36,18 +36,21 @@ static struct dpcls_subtable_lookup_info_t 
subtable_lookups[] = {
 { .prio = 0,
 #endif
   .probe = dpcls_subtable_autovalidator_probe,
-  .name = "autovalidator", },
+  .name = "autovalidator",
+  .usage_cnt = ATOMIC_COUNT_INIT(0), },
 
 /* The default scalar C code implementation. */
 { .prio = 1,
   .probe = dpcls_subtable_generic_probe,
-  .name = "generic", },
+  .name = "generic",
+  .usage_cnt = ATOMIC_COUNT_INIT(0), },
 
 #if (__x86_64__ && HAVE_AVX512F && HAVE_LD_AVX512_GOOD && __SSE4_2__)
 /* Only available on x86_64 bit builds with SSE 4.2 used for OVS core. */
 { .prio = 0,
   .probe = dpcls_subtable_avx512_gather_probe,
-  .name = "avx512_gather", },
+  .name = "avx512_gather",
+  .usage_cnt = ATOMIC_COUNT_INIT(0), },
 #else
 /* Disabling AVX512 at compile time, as compile time requirements not met.
  * This could be due to a number of reasons:
@@ -93,27 +96,46 @@ dpcls_subtable_set_prio(const char *name, uint8_t priority)
 }
 
 dpcls_subtable_lookup_func
-dpcls_subtable_get_best_impl(uint32_t u0_bit_count, uint32_t u1_bit_count)
+dpcls_subtable_get_best_impl(uint32_t u0_bit_count, uint32_t u1_bit_count,
+ dpcls_subtable_lookup_func old_func,
+ struct dpcls_subtable_lookup_info_t **old_info,
+ struct dpcls_subtable_lookup_info_t **new_info)
 {
 /* Iter over each subtable impl, and get highest priority one. */
 int32_t prio = -1;
 const char *name = NULL;
+int best_idx = 0;
 dpcls_subtable_lookup_func best_func = NULL;
 
 for (int i = 0; i < ARRAY_SIZE(subtable_lookups); i++) {
 int32_t probed_prio = subtable_lookups[i].prio;
+dpcls_subtable_lookup_func probed_func;
+
+probed_func = subtable_lookups[i].probe(u0_bit_count,
+

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

2021-12-02 Thread Maxime Coquelin

Hi Ian,

On 11/30/21 16:53, 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.


Thanks for taking care of this one.



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 +-
  NEWS |   1 +
  lib/dp-packet.h  |  26 +++
  lib/netdev-dpdk.c| 115 ---
  11 files changed, 98 insertions(+), 86 deletions(-)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

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