[ovs-dev] New mail (25)

[arivumani]

View message... 



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

Re: [ovs-dev] [PATCH v2 2/3] travis: Move x86-only addon packages to linux-prepare.sh

[Lance Yang (Arm Technology China)]

> -Original Message-
> From: Ilya Maximets 
> Sent: Saturday, December 14, 2019 1:55 AM
> To: dwilder ; Lance Yang (Arm Technology China)
> 
> Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org; b...@ovn.org; Yanqin Wei (Arm
> Technology China) ; Gavin Hu (Arm Technology China)
> ; Ruifeng Wang (Arm Technology China) 
> ;
> Jieqiang Wang (Arm Technology China) ; Malvika Gupta
> ; nd 
> Subject: Re: [ovs-dev] [PATCH v2 2/3] travis: Move x86-only addon packages to 
> linux-
> prepare.sh
>
> On 06.12.2019 23:32, dwilder wrote:
> > On 2019-12-05 19:26, Lance Yang wrote:
> >> To enable multiple CPU architectures support, it is necessary to move
> >> the x86-only addon packages from .travis.yml file. Otherwise, the
> >> x86-only addon packages will break the builds on some other CPU 
> >> architectures.
> >>
> >> Reviewed-by: Yanqin Wei 
> >> Reviewed-by: Malvika Gupta 
> >> Reviewed-by: Gavin Hu 
> >> Reviewed-by: Ruifeng Wang 
> >> Signed-off-by: Lance Yang 
> >> ---
> >>  .travis.yml  | 2 --
> >>  .travis/linux-prepare.sh | 3 ++-
> >>  2 files changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/.travis.yml b/.travis.yml index 482efd2..2dc4d43 100644
> >> --- a/.travis.yml
> >> +++ b/.travis.yml
> >> @@ -14,7 +14,6 @@ addons:
> >>apt:
> >>  packages:
> >>- bc
> >> -  - gcc-multilib
> >>- libssl-dev
> >>- llvm-dev
> >>- libjemalloc1
> >> @@ -26,7 +25,6 @@ addons:
> >>- libelf-dev
> >>- selinux-policy-dev
> >>- libunbound-dev
> >> -  - libunbound-dev:i386
> >>- libunwind-dev
> >>
> >>  before_install: ./.travis/${TRAVIS_OS_NAME}-prepare.sh
> >> diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh
> >> index 9e3ac0d..6421066 100755
> >> --- a/.travis/linux-prepare.sh
> >> +++ b/.travis/linux-prepare.sh
> >> @@ -18,7 +18,8 @@ pip install --user --upgrade docutils
> >>  if [ "$M32" ]; then
> >>  # 32-bit and 64-bit libunwind can not be installed at the same time.
> >>  # This will remove the 64-bit libunwind and install 32-bit version.
> >> -sudo apt-get install -y libunwind-dev:i386
> >> +sudo apt-get install -y \
> >> +libunwind-dev:i386 libunbound-dev:i386 gcc-multilib
> >>  fi
> >>
> >>  # IPv6 is supported by kernel but disabled in TravisCI images:
> >
> > LGTM:
> > With this patch applied ppc64le simply needs to be include into the matrix.
> > I will submit an updated ppc64le patch to be layered on top of this one.
> >
> > Acked-by: David Wilder 
>
> Thanks.  First two patches of this series are good even without multiarch 
> support so I went
> ahead and applied them (with minor visual/spelling changes) to master.  We'll 
> need to think
> more about actual enabling of ppc/arm since they are not that stable as we 
> would want
> them to be.
>
> Best regards, Ilya Maximets.
[Lance]

Hi Ilya,

Thank you for all the comments. I am glad to that the patches can be merged.

We reported the segment fault issue to Travis CI community. You can see the 
threads for segment faults in arm64/ppc64 environment:
https://travis-ci.community/t/segfaults-in-arm64-environment/5617/8
https://travis-ci.community/t/arm64-ppc64le-segfaults/6158

There might be a period of time for the Travis CI community to investigate. We 
will keep you updated about the latest process.

Best regards, Lance
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] XDP/AF_XDP checksum use case in OVS

[William Tu]

Hi,

We are measuring the performance of OVS using AF_XDP, and
have the following use case for XDP checksum:

High level Topology, two physical machines (PM1,2):
  VM1 -> OVS encap with Geneve (PM1) -> physical NIC
  -> network ->
  physical NIC -> OVS decap (PM2) -> VM2

When running an iperf TCP client/tx in a VM1, a tcp packet goes
through:
VM1
  1) VM's virtio net device
  2) by default, tx offload is on
PM1 Hypervisor
  3) OVS receives the tcp packet (no checksum)
  4) OVS encap outer Geneve header (no inner, no outer checksum)
  5) AF_XDP sends this frame to network (no inner, no outer checksum)
PM2 Hypervisor
  6) OVS receives using af_xdp port
  7) OVS removes Geneve UDP header, send to VM port
VM2
  8) VM2 kernel drops the TCP packet

A software work around is at 2) disable tx offload by using
ethtool -K tx off. This cause VM1 vcpu calculates inner tcp
csum but drops performance.

If XDP hints can somehow support TX checksum offload at 5),
this overhead can be avoided.

Thanks!
William

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

Re: [ovs-dev] thundering herd wakeup of handler threads

[David Ahern]

On 12/16/19 2:42 PM, Aaron Conole wrote:
> Can you try the following and see if your scalability issue is
> addressed?  I think it could be better integrated, but this is a
> different quick 'n dirty.

I'll to get to it before the end of the week.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 ovn 2/2] northd: add logical flows for dhcpv6 pfd parsing

[Numan Siddique]

On Thu, Dec 5, 2019 at 5:51 AM Lorenzo Bianconi
 wrote:
>
> Introduce logical flows in ovn router pipeline in order to parse dhcpv6
> advertise/reply from IPv6 prefix delegation router.
> Do not overwrite ipv6_ra_pd_list info in options column of SB port_binding
> table written by ovn-controller
>
> Signed-off-by: Lorenzo Bianconi 
> ---
>  northd/ovn-northd.c |  69 +++-
>  ovn-nb.xml  |  17 +++
>  tests/atlocal.in|   5 +-
>  tests/system-ovn.at | 109 
>  4 files changed, 198 insertions(+), 2 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index f0847d81e..8c50218ee 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -2644,6 +2644,8 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>struct sset *active_ha_chassis_grps)
>  {
>  sbrec_port_binding_set_datapath(op->sb, op->od->sb);
> +const char *ipv6_pd_list = NULL;
> +
>  if (op->nbrp) {
>  /* If the router is for l3 gateway, it resides on a chassis
>   * and its port type is "l3gateway". */
> @@ -2766,6 +2768,12 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>  smap_add(, "l3gateway-chassis", chassis_name);
>  }
>  }
> +
> +ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
> +if (ipv6_pd_list) {
> +smap_add(, "ipv6_ra_pd_list", ipv6_pd_list);
> +}
> +
>  sbrec_port_binding_set_options(op->sb, );
>  smap_destroy();
>
> @@ -2815,6 +2823,12 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>  smap_add_format(,
>  "qdisc_queue_id", "%d", queue_id);
>  }
> +
> +ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
> +if (ipv6_pd_list) {
> +smap_add(, "ipv6_ra_pd_list", ipv6_pd_list);
> +}
> +
>  sbrec_port_binding_set_options(op->sb, );
>  smap_destroy();
>  if (ovn_is_known_nb_lsp_type(op->nbsp->type)) {
> @@ -2864,6 +2878,12 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>  if (chassis) {
>  smap_add(, "l3gateway-chassis", chassis);
>  }
> +
> +ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
> +if (ipv6_pd_list) {
> +smap_add(, "ipv6_ra_pd_list", ipv6_pd_list);
> +}
> +
>  sbrec_port_binding_set_options(op->sb, );
>  smap_destroy();
>  } else {
> @@ -7833,7 +7853,36 @@ build_lrouter_flows(struct hmap *datapaths, struct 
> hmap *ports,
>  free(snat_ips);
>  }
>
> -/* Logical router ingress table 3: IP Input for IPv6. */
> +/* DHCPv6 reply handling */
> +HMAP_FOR_EACH (op, key_node, ports) {
> +if (!op->nbrp) {
> +continue;
> +}
> +
> +if (op->derived) {
> +continue;
> +}
> +
> +struct lport_addresses lrp_networks;
> +if (!extract_lrp_networks(op->nbrp, _networks)) {
> +continue;
> +}
> +
> +for (size_t i = 0; i < lrp_networks.n_ipv6_addrs; i++) {
> +ds_clear();
> +ds_clear();
> +ds_put_format(, "ip6.dst == %s && udp.src == 547 &&"
> +  " udp.dst == 546",
> +  lrp_networks.ipv6_addrs[i].addr_s);
> +ds_put_format(, "reg0 = 0; handle_dhcpv6_reply { "
> +  "eth.dst <-> eth.src; ip6.dst <-> ip6.src; "
> +  "outport <-> inport; output; };");
> +ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 100,
> +  ds_cstr(), ds_cstr());
> +}
> +}
> +
> +/* Logical router ingress table 1: IP Input for IPv6. */
>  HMAP_FOR_EACH (op, key_node, ports) {
>  if (!op->nbrp) {
>  continue;
> @@ -8634,6 +8683,24 @@ build_lrouter_flows(struct hmap *datapaths, struct 
> hmap *ports,
>  continue;
>  }
>
> +struct smap options;
> +/* enable IPv6 prefix delegation */
> +bool prefix_delegation = smap_get_bool(>nbrp->options,
> +   "prefix_delegation", false);
> +if (prefix_delegation) {
> +smap_clone(, >sb->options);
> +smap_add(, "ipv6_prefix_delegation", "true");
> +sbrec_port_binding_set_options(op->sb, );
> +smap_destroy();
> +}
> +
> +if (smap_get_bool(>nbrp->options, "prefix", false)) {
> +smap_clone(, >sb->options);
> +smap_add(, "ipv6_prefix", "true");
> +sbrec_port_binding_set_options(op->sb, );
> +smap_destroy();
> +}
> +
>  const char *address_mode = smap_get(
>  >nbrp->ipv6_ra_configs, "address_mode");

Re: [ovs-dev] thundering herd wakeup of handler threads

[Aaron Conole]

David Ahern  writes:

> On 12/13/19 1:52 PM, Aaron Conole wrote:
>> Jason Baron via dev  writes:
>> 
>>> On 12/10/19 5:20 PM, David Ahern wrote:
 On 12/10/19 3:09 PM, Jason Baron wrote:
> Hi David,
>
> The idea is that we try and queue new work to 'idle' threads in an
> attempt to distribute a workload. Thus, once we find an 'idle' thread we
> stop waking up other threads. While we are searching the wakeup list for
> idle threads, we do queue an epoll event to the non-idle threads, this
> doesn't mean they are woken up. It just means that when they go to
> epoll_wait() to harvest events from the kernel, if the event is still
> available it will be reported. If the condition for the event is no
> longer true (because another thread consumed it), they the event
> wouldn't be visible. So its a way of load balancing a workload while
> also reducing the number of wakeups. Its 'exclusive' in the sense that
> it will stop after it finds the first idle thread.
>
> We certainly can employ other wakeup strategies - there was interest
> (and patches) for a strict 'round robin' but that has not been merged
> upstream.
>
> I would like to better understand the current usecase. It sounds like
> each thread as an epoll file descriptor. And each epoll file descriptor
> is attached the name netlink socket. But when that netlink socket gets a
> packet it causes all the threads to wakeup? Are you sure there is just 1
> netlink socket that all epoll file desciptors are are attached to?
>

 Thanks for the response.

 This is the code in question:

 https://github.com/openvswitch/ovs/blob/branch-2.11/lib/dpif-netlink.c#L492

 Yes, prior to finding the above code reference I had traced it to a
 single socket with all handler threads (71 threads on this 96 cpu box)
 on the wait queue.

 The ovs kernel module is punting a packet to userspace. It generates a
 netlink message and invokes netlink_unicast. This the stack trace:

 ad09cc02 ttwu_do_wakeup+0x92 ([kernel.kallsyms])
 ad09d945 try_to_wake_up+0x1d5 ([kernel.kallsyms])
 ad257275 pollwake+0x75 ([kernel.kallsyms])
 ad0b58a4 __wake_up_common+0x74 ([kernel.kallsyms])
 ad0b59cc __wake_up_common_lock+0x7c ([kernel.kallsyms])
 ad289ecc ep_poll_wakeup_proc+0x1c ([kernel.kallsyms])
 ad28a4bc ep_call_nested.constprop.18+0xbc
 ([kernel.kallsyms])
 ad28b0f2 ep_poll_callback+0x172 ([kernel.kallsyms])
 ad0b58a4 __wake_up_common+0x74 ([kernel.kallsyms])
 ad0b59cc __wake_up_common_lock+0x7c ([kernel.kallsyms])
 ad794af9 sock_def_readable+0x39 ([kernel.kallsyms])
 ad7e846e __netlink_sendskb+0x3e ([kernel.kallsyms])
 ad7eb11a netlink_unicast+0x20a ([kernel.kallsyms])
 c07abd44 queue_userspace_packet+0x2d4 ([kernel.kallsyms])
 c07ac330 ovs_dp_upcall+0x50 ([kernel.kallsyms])


 A probe on sock_def_readable shows it is a single socket that the wait
 queue is processed. Eventually ttwu_do_wakeup is invoked 71 times (once
 for each thread). In some cases I see the awakened threads immediately
 running on the target_cpu, while the queue walk continues to wake up all
 threads. Only the first one is going to handle the packet so the rest of
 the wakeups are just noise.

 On this system in just a 1-second interval I see this sequence play out
 400+ times.

>>>
>>>
>>> That stack trace is interesting. I *think* it means you are doing
>>> select() or poll() on the epoll file descriptors? I say that because I
>>> see 'pollwake()' in the stack. Notice that pollwake() is only in
>>> fs/select.c.
>> 
>> During ofproto upcall processing, if there aren't any upcalls to
>> receive, a dpif_recv_wait() is called, which will call into
>> dpif_netlink_recv_wait() which will set up the next poll_block() call.
>> 
>> This eventually uses the poll() interface (through time_poll() call) .
>> 
>> Maybe we can confirm the thundering herd by just making a quick hack to
>> shunt out the poll block (included).  Just a quick 'n dirty hack to test
>> with.
>> 
>
> Thanks for the patch.
>
> There is nothing special to be done my end to see the problem and figure
> out solutions. Just run ovs 2.11, have traffic flows that cause upcalls,
> and then watch the scheduling tracepoints.
>
> e.g.,
> sudo perf record -c1 -e sched:sched_switch,sched:sched_wakeup -a -m 8192
> -- sleep 1
>
> sudo perf sched timehist -w
>
> That output shows on a given CPU the wakeup of every one of the handler
> threads. record with -g to get stack traces and see that it is due to
> the upcall. If you still are not convinced add a probe
>
> perf probe 

Re: [ovs-dev] [PATCH V4 12/17] netdev-offload-dpdk: Framework for actions offload

[Ilya Maximets]

On 16.12.2019 16:10, Eli Britstein wrote:
> Currently HW offload is accelerating only the rule matching sequence.
> Introduce a framework for offloading rule actions as a pre-step for
> processing the rule actions in HW. In case of a failure, fallback to the
> legacy partial offload scheme.
> 
> Note: a flow will be fully offloaded only if it can process all its
> actions in HW.
> 
> Signed-off-by: Eli Britstein 
> Reviewed-by: Oz Shlomo 
> ---
>  lib/netdev-offload-dpdk.c | 104 
> --
>  1 file changed, 91 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index 79a7d33fb..a60d68d2c 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -691,6 +691,89 @@ add_flow_mark_rss_actions(struct flow_actions *actions,
>  add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
>  }
>  
> +static struct rte_flow *
> +netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
> + struct netdev *netdev,
> + uint32_t flow_mark)
> +{
> +struct flow_actions actions = { .actions = NULL, .cnt = 0 };
> +const struct rte_flow_attr flow_attr = {
> +.group = 0,
> +.priority = 0,
> +.ingress = 1,
> +.egress = 0
> +};
> +struct rte_flow_error error;
> +struct rte_flow *flow;
> +
> +add_flow_mark_rss_actions(, flow_mark, netdev);
> +
> +flow = netdev_offload_dpdk_flow_create(netdev, _attr, 
> patterns->items,
> +   actions.actions, );
> +
> +if (!flow) {
> +VLOG_ERR("%s: Failed to create flow: %s (%u)\n",
> + netdev_get_name(netdev), error.message, error.type);

netdev_offload_dpdk_flow_create() already printed the whole flow and the
error.  Do we need a duplicated message?

> +}
> +
> +free_flow_actions();
> +return flow;
> +}
> +
> +static int
> +parse_flow_actions(struct netdev *netdev OVS_UNUSED,
> +   struct flow_actions *actions,
> +   struct nlattr *nl_actions,
> +   size_t nl_actions_len,
> +   struct offload_info *info OVS_UNUSED)
> +{
> +struct nlattr *nla;
> +size_t left;
> +
> +NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
> +VLOG_DBG_RL(_rl,
> +"Unsupported action type %d", nl_attr_type(nla));
> +return -1;
> +}
> +
> +if (nl_actions_len == 0) {
> +VLOG_DBG_RL(_rl,
> +"Unsupported action type drop");
> +return -1;
> +}
> +
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
> +return 0;
> +}
> +
> +static struct rte_flow *
> +netdev_offload_dpdk_actions(struct netdev *netdev,
> +struct flow_patterns *patterns,
> +struct nlattr *nl_actions,
> +size_t actions_len,
> +struct offload_info *info)
> +{
> +const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1 };
> +struct flow_actions actions = { .actions = NULL, .cnt = 0 };
> +struct rte_flow *flow = NULL;
> +struct rte_flow_error error;
> +int ret;
> +
> +ret = parse_flow_actions(netdev, , nl_actions, actions_len, 
> info);
> +if (ret) {
> +goto out;
> +}
> +flow = netdev_offload_dpdk_flow_create(netdev, _attr, 
> patterns->items,
> +   actions.actions, );
> +if (!flow) {
> +VLOG_ERR("%s: Failed to create flow: %s (%u)\n",
> + netdev_get_name(netdev), error.message, error.type);

Ditto.

> +}
> +out:
> +free_flow_actions();
> +return flow;
> +}
> +
>  static int
>  netdev_offload_dpdk_add_flow(struct netdev *netdev,
>   const struct match *match,
> @@ -699,16 +782,8 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
>   const ovs_u128 *ufid,
>   struct offload_info *info)
>  {
> -const struct rte_flow_attr flow_attr = {
> -.group = 0,
> -.priority = 0,
> -.ingress = 1,
> -.egress = 0
> -};
>  struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
> -struct flow_actions actions = { .actions = NULL, .cnt = 0 };
>  struct rte_flow *flow;
> -struct rte_flow_error error;
>  int ret = 0;
>  
>  ret = parse_flow_match(, match);
> @@ -716,10 +791,14 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
>  goto out;
>  }
>  
> -add_flow_mark_rss_actions(, info->flow_mark, netdev);
> -
> -flow = netdev_offload_dpdk_flow_create(netdev, _attr, 
> patterns.items,
> -   actions.actions, );
> +flow = netdev_offload_dpdk_actions(netdev, , nl_actions,
> +   actions_len, 

Re: [ovs-dev] [PATCH V4 04/17] netdev-offload-dpdk: Improve HW offload flow debuggability

[Ilya Maximets]

On 16.12.2019 16:10, Eli Britstein wrote:
> Add debug prints when creating and destroying rte flows, with all the
> flow details (attributes, patterns, actions).
> 
> Signed-off-by: Eli Britstein 
> Reviewed-by: Oz Shlomo 
> ---
>  lib/netdev-offload-dpdk.c | 200 
> ++
>  1 file changed, 130 insertions(+), 70 deletions(-)
> 
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index a5bc4ad33..7cb24ddcd 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -28,6 +28,7 @@
>  #include "uuid.h"
>  
>  VLOG_DEFINE_THIS_MODULE(netdev_offload_dpdk);
> +static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(100, 5);

Why it's 'error_rl'?  You're using it for everything, not only for
errors.  It might be better to rename it to just 'rl'.  This will
allow to decrease line length in a few places and put commands on a
single line.

>  
>  /* Thread-safety
>   * =
> @@ -132,72 +133,70 @@ struct flow_actions {
>  };
>  
>  static void
> -dump_flow_pattern(struct rte_flow_item *item)
> +dump_flow_attr(struct ds *s, const struct rte_flow_attr *attr)
>  {
> -struct ds s;
> -
> -if (!VLOG_IS_DBG_ENABLED() || item->type == RTE_FLOW_ITEM_TYPE_END) {
> -return;
> -}
> -
> -ds_init();
> +ds_put_format(s,
> +  "  Attributes: "
> +  "ingress=%d, egress=%d, prio=%d, group=%d, transfer=%d\n",
> +  attr->ingress, attr->egress, attr->priority, attr->group,
> +  attr->transfer);
> +}
>  
> +static void
> +dump_flow_pattern(struct ds *s, const struct rte_flow_item *item)
> +{
>  if (item->type == RTE_FLOW_ITEM_TYPE_ETH) {
>  const struct rte_flow_item_eth *eth_spec = item->spec;
>  const struct rte_flow_item_eth *eth_mask = item->mask;
>  
> -ds_put_cstr(, "rte flow eth pattern:\n");
> +ds_put_cstr(s, "rte flow eth pattern:\n");
>  if (eth_spec) {
> -ds_put_format(,
> +ds_put_format(s,
>"  Spec: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
>"type=0x%04" PRIx16"\n",
>ETH_ADDR_BYTES_ARGS(eth_spec->src.addr_bytes),
>ETH_ADDR_BYTES_ARGS(eth_spec->dst.addr_bytes),
>ntohs(eth_spec->type));
>  } else {
> -ds_put_cstr(, "  Spec = null\n");
> +ds_put_cstr(s, "  Spec = null\n");
>  }
>  if (eth_mask) {
> -ds_put_format(,
> +ds_put_format(s,
>"  Mask: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
>"type=0x%04"PRIx16"\n",
>ETH_ADDR_BYTES_ARGS(eth_mask->src.addr_bytes),
>ETH_ADDR_BYTES_ARGS(eth_mask->dst.addr_bytes),
>ntohs(eth_mask->type));
>  } else {
> -ds_put_cstr(, "  Mask = null\n");
> +ds_put_cstr(s, "  Mask = null\n");
>  }
> -}
> -
> -if (item->type == RTE_FLOW_ITEM_TYPE_VLAN) {
> +} else if (item->type == RTE_FLOW_ITEM_TYPE_VLAN) {
>  const struct rte_flow_item_vlan *vlan_spec = item->spec;
>  const struct rte_flow_item_vlan *vlan_mask = item->mask;
>  
> -ds_put_cstr(, "rte flow vlan pattern:\n");
> +ds_put_cstr(s, "rte flow vlan pattern:\n");
>  if (vlan_spec) {
> -ds_put_format(,
> +ds_put_format(s,
>"  Spec: inner_type=0x%"PRIx16", 
> tci=0x%"PRIx16"\n",
>ntohs(vlan_spec->inner_type), 
> ntohs(vlan_spec->tci));
>  } else {
> -ds_put_cstr(, "  Spec = null\n");
> +ds_put_cstr(s, "  Spec = null\n");
>  }
>  
>  if (vlan_mask) {
> -ds_put_format(,
> +ds_put_format(s,
>"  Mask: inner_type=0x%"PRIx16", 
> tci=0x%"PRIx16"\n",
>ntohs(vlan_mask->inner_type), 
> ntohs(vlan_mask->tci));
>  } else {
> -ds_put_cstr(, "  Mask = null\n");
> +ds_put_cstr(s, "  Mask = null\n");
>  }
> -}
> -
> -if (item->type == RTE_FLOW_ITEM_TYPE_IPV4) {
> +} else if (item->type == RTE_FLOW_ITEM_TYPE_IPV4) {
>  const struct rte_flow_item_ipv4 *ipv4_spec = item->spec;
>  const struct rte_flow_item_ipv4 *ipv4_mask = item->mask;
>  
> -ds_put_cstr(, "rte flow ipv4 pattern:\n");
> +ds_put_cstr(s, "rte flow ipv4 pattern:\n");
>  if (ipv4_spec) {
> -ds_put_format(,
> +ds_put_format(s,
>"  Spec: tos=0x%"PRIx8", ttl=%"PRIx8
>", proto=0x%"PRIx8
>", src="IP_FMT", dst="IP_FMT"\n",
> @@ -207,10 +206,10 @@ dump_flow_pattern(struct rte_flow_item *item)
>

[ovs-dev] Is this d...@openvswitch.org still active?

[Nelson Corey]

I have tried to email this your d...@openvswitch.org account severally but I 
got no response, Please get back to me at your earliest convenience if you 
receive this email. 

Sincerely,
Nelson Corey

Ich habe versucht, Ihrem Konto d...@openvswitch.org eine E-Mail zu senden, habe 
jedoch keine Antwort erhalten. Wenn Sie diese E-Mail erhalten, setzen Sie sich 
bitte umgehend mit mir in Verbindung.

Mit freundlichen Grüßen,
Nelson Corey

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

Re: [ovs-dev] [PATCH V4 17/17] netdev-offload-dpdk: Support offload of set TCP/UDP ports actions

[Ilya Maximets]

On 16.12.2019 16:10, Eli Britstein wrote:
> Signed-off-by: Eli Britstein 
> Reviewed-by: Oz Shlomo 
> ---
>  Documentation/howto/dpdk.rst |  1 +
>  NEWS |  4 +--
>  lib/netdev-offload-dpdk.c| 85 
> 
>  3 files changed, 88 insertions(+), 2 deletions(-)
> 

In order to reduce code size and improve readability suggesting following
incremental (based on top of a full patch-set, compile tested only).
Explanations:
1. We're allowed to modify actions here as they are not constant.
   So, no need to maintain copy of the mask.  tc offload does the same.
2. All build asserts moved out of function to a single place.
3. Macros for repeated code pattern.
4. mask could be obtained as just key + 1.
---
Subject: [PATCH] netdev-offload-dpdk: Refactor set actions.

Signed-off-by: Ilya Maximets 
---
 lib/netdev-offload-dpdk.c | 186 +++---
 1 file changed, 51 insertions(+), 135 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index d1c066992..02fe551d0 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -848,11 +848,9 @@ add_output_action(struct netdev *netdev,
 }
 
 static int
-add_set_flow_action(struct flow_actions *actions,
-const void *value,
-const void *mask,
-const size_t size,
-const int attr)
+add_set_flow_action__(struct flow_actions *actions,
+  const void *value, void *mask,
+  const size_t size, const int attr)
 {
 void *spec;
 
@@ -874,179 +872,97 @@ add_set_flow_action(struct flow_actions *actions,
 memcpy(spec, value, size);
 add_flow_action(actions, attr, spec);
 
+/* Clear used mask for later checking. */
+if (mask) {
+memset(mask, 0, size);
+}
 return 0;
 }
 
-/* Mask is at the midpoint of the data. */
-#define get_mask(a, type) ((const type *)(const void *)(a + 1) + 1)
+BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_mac) ==
+MEMBER_SIZEOF(struct ovs_key_ethernet, eth_src));
+BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_mac) ==
+MEMBER_SIZEOF(struct ovs_key_ethernet, eth_dst));
+BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_ipv4) ==
+MEMBER_SIZEOF(struct ovs_key_ipv4, ipv4_src));
+BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_ipv4) ==
+MEMBER_SIZEOF(struct ovs_key_ipv4, ipv4_dst));
+BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_ttl) ==
+MEMBER_SIZEOF(struct ovs_key_ipv4, ipv4_ttl));
+BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_tp) ==
+MEMBER_SIZEOF(struct ovs_key_tcp, tcp_src));
+BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_tp) ==
+MEMBER_SIZEOF(struct ovs_key_tcp, tcp_dst));
+BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_tp) ==
+MEMBER_SIZEOF(struct ovs_key_udp, udp_src));
+BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_tp) ==
+MEMBER_SIZEOF(struct ovs_key_udp, udp_dst));
 
 static int
 parse_set_actions(struct flow_actions *actions,
-  const struct nlattr *set_actions,
+  struct nlattr *set_actions,
   const size_t set_actions_len,
   bool masked)
 {
 const struct nlattr *sa;
 unsigned int sleft;
 
+#define add_set_flow_action(field, type)  \
+if (add_set_flow_action__(actions, >field,   \
+  mask ? CONST_CAST(void *, >field) : NULL, \
+  sizeof key->field, type)) { \
+return -1;\
+}
+
 NL_ATTR_FOR_EACH_UNSAFE (sa, sleft, set_actions, set_actions_len) {
 if (nl_attr_type(sa) == OVS_KEY_ATTR_ETHERNET) {
 const struct ovs_key_ethernet *key = nl_attr_get(sa);
-const struct ovs_key_ethernet *mask = masked ?
-get_mask(sa, struct ovs_key_ethernet) : NULL;
-struct ovs_key_ethernet consumed_mask;
-
-if (masked) {
-memcpy(_mask, mask, sizeof consumed_mask);
-} else {
-memset(_mask, 0, sizeof consumed_mask);
-}
+const struct ovs_key_ethernet *mask = masked ? key + 1 : NULL;
 
-BUILD_ASSERT(sizeof(struct rte_flow_action_set_mac) ==
- sizeof key->eth_src);
-if (add_set_flow_action(actions, >eth_src,
-mask ? >eth_src : NULL,
-sizeof key->eth_src,
-RTE_FLOW_ACTION_TYPE_SET_MAC_SRC)) {
-return -1;
-}
-memset(_mask.eth_src, 0, sizeof consumed_mask.eth_src);
-
-

Re: [ovs-dev] [PATCH ovn] system-ovn.at: Fix failing system tests.

[Dumitru Ceara]

On Mon, Dec 16, 2019 at 6:44 PM Numan Siddique  wrote:
>
> On Mon, Dec 16, 2019 at 10:16 AM Mark Michelson  wrote:
> >
> > Acked-by: Mark Michelson 
> >
>
> Thanks Dumitru and Mark. I applied this patch to master.
>
> Numan

Thanks!

>
> > On 12/6/19 10:14 AM, Dumitru Ceara wrote:
> > > Upstream OVS commit:
> > >  commit 03ccfe482ffdc6c1132aeb62467e0fbe6768f25d
> > >  Author: William Tu 
> > >  Date:   Tue Jun 25 14:52:38 2019 -0700
> > >
> > >  vswitchd: Separate disable system and route.
> > >
> > >  Previously, '--disable-system' disables both system dp and the system
> > >  routing table.  The patch makes '--disable-system' only disable 
> > > system
> > >  dp and adds '--disable-system-route' for disabling the route table.
> > >  This fixes failures when 'make check-system-userspace' for tunnel 
> > > cases.
> > >
> > >  As a consequence, hitting errors due to OVS userspace parses the 
> > > IGMP packet
> > >  but its datapaths do not, so odp_flow_key_to_flow() return 
> > > ODP_FIT_TOO_LITTLE.
> > >  commit c645550bb249 ("odp-util: Always report ODP_FIT_TOO_LITTLE for 
> > > IGMP.")
> > >  Fix it by filtering out the IGMP-related error message.
> > >
> > >  Signed-off-by: William Tu 
> > >  Signed-off-by: Yi-Hung Wei 
> > >  Co-authored-by: Yi-Hung Wei 
> > >  Signed-off-by: Ben Pfaff 
> > >
> > > This OVS change was performed in the OVS repo after the OVS-OVN split
> > > and after the OVN tests/system-userspace-*.at files were created by
> > > commit c17862ff04c3b315e5793c9c7fb04a79a892e6f3 and needs to be ported
> > > to OVN too.
> > >
> > > CC: Numan Siddique 
> > > Signed-off-by: Dumitru Ceara 
> > > ---
> > >   tests/ofproto-macros.at  | 2 +-
> > >   tests/system-userspace-macros.at | 4 +++-
> > >   2 files changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
> > > index 1fd546a..6c4ff60 100644
> > > --- a/tests/ofproto-macros.at
> > > +++ b/tests/ofproto-macros.at
> > > @@ -196,7 +196,7 @@ m4_define([_OVS_VSWITCHD_START],
> > >   # 'vswitchd-aux-args' provides a way to pass extra command line 
> > > arguments
> > >   # to ovs-vswitchd
> > >   m4_define([OVS_VSWITCHD_START],
> > > -  [_OVS_VSWITCHD_START([--enable-dummy$3 --disable-system $4])
> > > +  [_OVS_VSWITCHD_START([--enable-dummy$3 --disable-system 
> > > --disable-system-route $4])
> > >  AT_CHECK([add_of_br 0 $1 m4_if([$2], [], [], [| uuidfilt])], [0], 
> > > [$2])
> > >   ])
> > >
> > > diff --git a/tests/system-userspace-macros.at 
> > > b/tests/system-userspace-macros.at
> > > index d8cc686..86ed226 100644
> > > --- a/tests/system-userspace-macros.at
> > > +++ b/tests/system-userspace-macros.at
> > > @@ -36,7 +36,9 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START],
> > >   m4_define([OVS_TRAFFIC_VSWITCHD_STOP],
> > > [OVS_VSWITCHD_STOP([dnl
> > >   $1";/netdev_linux.*obtaining netdev stats via vport failed/d
> > > -/dpif_netlink.*Generic Netlink family 'ovs_datapath' does not exist. The 
> > > Open vSwitch kernel module is probably not loaded./d"])
> > > +/dpif_netlink.*Generic Netlink family 'ovs_datapath' does not exist. The 
> > > Open vSwitch kernel module is probably not loaded./d
> > > +/dpif_netdev(revalidator.*)|ERR|internal error parsing flow 
> > > key.*proto=2.*/d
> > > +/dpif(revalidator.*)|WARN|netdev@ovs-netdev: failed to.*proto=2.*/d"])
> > >  AT_CHECK([:; $2])
> > > ])
> > >
> > >
> >
> > ___
> > 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] Use batch process recv for tap and raw socket in netdev datapath

[William Tu]

On Fri, Dec 06, 2019 at 02:09:24AM -0500, yang_y...@163.com wrote:
> From: Yi Yang 
> 
> Current netdev_linux_rxq_recv_tap and netdev_linux_rxq_recv_sock
> just receive single packet, that is very inefficient, per my test
> case which adds two tap ports or veth ports into OVS bridge
> (datapath_type=netdev) and use iperf3 to do performance test
> between two ports (they are set into different network name space).
> 
> The result is as below:
> 
>   tap:  295 Mbits/sec
>   veth: 207 Mbits/sec
> 
> After I change netdev_linux_rxq_recv_tap and
> netdev_linux_rxq_recv_sock to use batch process, the performance
> is boosted by about 7 times, here is the result:
> 
>   tap:  1.96 Gbits/sec
>   veth: 1.47 Gbits/sec
> 
> Undoubtedly this is a huge improvement although it can't match
> OVS kernel datapath yet.
> 
> FYI: here is thr result for OVS kernel datapath:
> 
>   tap:  37.2 Gbits/sec
>   veth: 36.3 Gbits/sec
> 
> Note: performance result is highly related with your test machine
> , you shouldn't expect the same results on your test machine.
> 
> Signed-off-by: Yi Yang 

Hi Yi Yang,

Are you testing this using OVS-DPDK?
If you're using OVS-DPDK, then you should use DPDK's vdev to
open and attach tap/veth device to OVS. I think you'll see much
better performance.

The performance issue you pointed out only happens when using
userspace datapath without DPDK library, where afxdp is used.
I'm still looking for a better solutions for faster interface
for veth (af_packet) and tap.

Thanks
William

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

Re: [ovs-dev] [PATCH V4 08/17] dpif-netdev: Update offloaded flows statistics

[Ilya Maximets]

On 16.12.2019 16:10, Eli Britstein wrote:
> From: Ophir Munk 
> 
> In case a flow is HW offloaded, packets do not reach the SW, thus not
> counted for statistics. Use netdev flow get API in order to update the
> statistics of flows by the HW statistics.
> 
> Co-authored-by: Eli Britstein 
> Signed-off-by: Ophir Munk 
> Reviewed-by: Oz Shlomo 
> Signed-off-by: Eli Britstein 
> ---
>  lib/dpif-netdev.c | 81 
> ++-
>  1 file changed, 68 insertions(+), 13 deletions(-)

Some issues of this patch:

1. stats are duplicated now in dpif-netdev.  We already have them
   in netdev-offload-dpdk, so we don't need to store them here too.

2. dpif_flow_attrs should be filled by the offload provider, not by
   the datapath.  netdev-offload-tc fills this information and it
   will be lost in current implementation of dpif-netdev.

3. New 'dp_layer' types has to be documented in dpctl man.
   Also, 'in_hw' doesn't look like a datapath layer name.
   Suggesting to use 'dpdk' as netdev-offload-tc uses 'tc'.
   We also could return specific dp_layer for partially offloaded
   flows, i.e. 'ovs-partial'.

Suggesting following incremental patch where I tried to address above
issues (didn't update man, only compile tested, made on top of the full set):
-
Subject: [PATCH] dpif-netdev: Cleanup for stats/attrs of offloaded flows.

Signed-off-by: Ilya Maximets 
---
 lib/dpif-netdev.c | 146 +++---
 lib/netdev-offload-dpdk.c |  35 +
 2 files changed, 94 insertions(+), 87 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 91fefb877..892b2d0f5 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -530,7 +530,6 @@ struct dp_netdev_flow {
 
 /* Statistics. */
 struct dp_netdev_flow_stats stats;
-struct dp_netdev_flow_stats hw_stats;
 
 /* Actions. */
 OVSRCU_TYPE(struct dp_netdev_actions *) actions;
@@ -3011,11 +3010,49 @@ dp_netdev_pmd_find_flow(const struct 
dp_netdev_pmd_thread *pmd,
 return NULL;
 }
 
+static bool
+dpif_netdev_get_flow_offload_status(const struct dp_netdev *dp,
+const struct dp_netdev_flow *netdev_flow,
+struct dpif_flow_stats *stats,
+struct dpif_flow_attrs *attrs)
+{
+struct nlattr *actions;
+struct ofpbuf wbuffer;
+struct netdev *netdev;
+struct match match;
+
+int ret = 0;
+
+if (!netdev_is_flow_api_enabled()) {
+return false;
+}
+
+netdev = netdev_ports_get(netdev_flow->flow.in_port.odp_port, dp->class);
+if (!netdev) {
+return false;
+}
+/* Taking a global 'port_mutex' to fulfill thread safety
+ * restrictions for the netdev-offload-dpdk module. */
+ovs_mutex_lock(>port_mutex);
+ret = netdev_flow_get(netdev, , , _flow->mega_ufid,
+  stats, attrs, );
+ovs_mutex_unlock(>port_mutex);
+netdev_close(netdev);
+if (ret) {
+return false;
+}
+
+return true;
+}
+
 static void
-get_dpif_flow_stats(const struct dp_netdev_flow *netdev_flow_,
-struct dpif_flow_stats *stats, bool hw)
+get_dpif_flow_status(const struct dp_netdev *dp,
+ const struct dp_netdev_flow *netdev_flow_,
+ struct dpif_flow_stats *stats,
+ struct dpif_flow_attrs *attrs)
 {
-struct dp_netdev_flow_stats *flow_stats;
+struct dpif_flow_stats offload_stats;
+struct dpif_flow_attrs offload_attrs;
 struct dp_netdev_flow *netdev_flow;
 unsigned long long n;
 long long used;
@@ -3023,16 +3060,29 @@ get_dpif_flow_stats(const struct dp_netdev_flow 
*netdev_flow_,
 
 netdev_flow = CONST_CAST(struct dp_netdev_flow *, netdev_flow_);
 
-flow_stats = (hw) ? _flow->hw_stats : _flow->stats;
-
-atomic_read_relaxed(_stats->packet_count, );
+atomic_read_relaxed(_flow->stats.packet_count, );
 stats->n_packets = n;
-atomic_read_relaxed(_stats->byte_count, );
+atomic_read_relaxed(_flow->stats.byte_count, );
 stats->n_bytes = n;
-atomic_read_relaxed(_stats->used, );
+atomic_read_relaxed(_flow->stats.used, );
 stats->used = used;
-atomic_read_relaxed(_stats->tcp_flags, );
+atomic_read_relaxed(_flow->stats.tcp_flags, );
 stats->tcp_flags = flags;
+
+if (dpif_netdev_get_flow_offload_status(dp, netdev_flow,
+_stats, _attrs)) {
+stats->n_packets += offload_stats.n_packets;
+stats->n_bytes += offload_stats.n_bytes;
+stats->used = MAX(stats->used, offload_stats.used);
+stats->tcp_flags |= offload_stats.tcp_flags;
+if (attrs) {
+attrs->offloaded = offload_attrs.offloaded;
+attrs->dp_layer = offload_attrs.dp_layer;
+}
+} else if (attrs) {
+attrs->offloaded = false;
+attrs->dp_layer = "ovs";
+}
 }
 
 /* Converts to 

Re: [ovs-dev] [PATCH] ovs-thread: Avoid huge alignment on a base spinlock structure.

[William Tu]

On Mon, Dec 16, 2019 at 03:22:00PM +0100, Ilya Maximets wrote:
> Marking the structure as 64 bytes aligned forces compiler to produce
> big holes in the containing structures in order to fulfill this
> requirement.  Also, any structure that contains this one as a member
> automatically inherits this huge alignment making resulted memory
> layout not efficient.  For example, 'struct umem_pool' currently
> uses 3 full cache lines (192 bytes) with only 32 bytes of actual data:
> 
>   struct umem_pool {
> intindex;/*  0   4 */
> unsigned int   size; /*  4   4 */
> 
> /* XXX 56 bytes hole, try to pack */
> 
> /* --- cacheline 1 boundary (64 bytes) --- */
> struct ovs_spin lock __attribute__((__aligned__(64))); /* 64  64 */
> 
> /* XXX last struct has 48 bytes of padding */
> 
> /* --- cacheline 2 boundary (128 bytes) --- */
> void * *   array;/* 128  8 */
> 
> /* size: 192, cachelines: 3, members: 4 */
> /* sum members: 80, holes: 1, sum holes: 56 */
> /* padding: 56 */
> /* paddings: 1, sum paddings: 48 */
> /* forced alignments: 1, forced holes: 1, sum forced holes: 56 */
>   } __attribute__((__aligned__(64)));
> 
> Actual alignment of a spin lock is required only for Tx queue locks
> inside netdev-afxdp to avoid false sharing, in all other cases
> alignment only produces inefficient memory usage.

yes, now I realize umem->lock does not have the false
sharing problem.

> 
> Also, CACHE_LINE_SIZE macro should be used instead of 64 as different
> platforms may have different cache line sizes.
> 
> Using PADDED_MEMBERS to avoid alignment inheritance.
> 
> CC: William Tu 
> Fixes: ae36d63d7e3c ("ovs-thread: Make struct spin lock cache aligned.")
> Signed-off-by: Ilya Maximets 
> ---

Somehow my pahole stops working, but the patch looks
good to me, and thanks for also fixing the include ordering.

Acked-by: William Tu 

>  include/openvswitch/thread.h |  2 +-
>  lib/netdev-afxdp.c   | 20 ++--
>  lib/netdev-afxdp.h   | 13 +++--
>  lib/netdev-linux-private.h   |  2 +-
>  4 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/include/openvswitch/thread.h b/include/openvswitch/thread.h
> index 5053cb309..acc822904 100644
> --- a/include/openvswitch/thread.h
> +++ b/include/openvswitch/thread.h
> @@ -34,7 +34,7 @@ struct OVS_LOCKABLE ovs_mutex {
>  };
>  
>  #ifdef HAVE_PTHREAD_SPIN_LOCK
> -OVS_ALIGNED_STRUCT(64, OVS_LOCKABLE ovs_spin) {
> +struct OVS_LOCKABLE ovs_spin {
>  pthread_spinlock_t lock;
>  const char *where;  /* NULL if and only if uninitialized. */
>  };
> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> index ca2dfd005..c59a671e7 100644
> --- a/lib/netdev-afxdp.c
> +++ b/lib/netdev-afxdp.c
> @@ -40,6 +40,7 @@
>  #include "openvswitch/compiler.h"
>  #include "openvswitch/dynamic-string.h"
>  #include "openvswitch/list.h"
> +#include "openvswitch/thread.h"
>  #include "openvswitch/vlog.h"
>  #include "packets.h"
>  #include "socket-util.h"
> @@ -158,6 +159,13 @@ struct xsk_socket_info {
>  atomic_uint64_t tx_dropped;
>  };
>  
> +struct netdev_afxdp_tx_lock {
> +/* Padding to make netdev_afxdp_tx_lock exactly one cache line long. */
> +PADDED_MEMBERS(CACHE_LINE_SIZE,
> +struct ovs_spin lock;
> +);
> +};
> +
>  #ifdef HAVE_XDP_NEED_WAKEUP
>  static inline void
>  xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
> @@ -512,10 +520,10 @@ xsk_configure_all(struct netdev *netdev)
>  }
>  
>  n_txq = netdev_n_txq(netdev);
> -dev->tx_locks = xcalloc(n_txq, sizeof *dev->tx_locks);
> +dev->tx_locks = xzalloc_cacheline(n_txq * sizeof *dev->tx_locks);
>  
>  for (i = 0; i < n_txq; i++) {
> -ovs_spin_init(>tx_locks[i]);
> +ovs_spin_init(>tx_locks[i].lock);
>  }
>  
>  return 0;
> @@ -577,9 +585,9 @@ xsk_destroy_all(struct netdev *netdev)
>  
>  if (dev->tx_locks) {
>  for (i = 0; i < netdev_n_txq(netdev); i++) {
> -ovs_spin_destroy(>tx_locks[i]);
> +ovs_spin_destroy(>tx_locks[i].lock);
>  }
> -free(dev->tx_locks);
> +free_cacheline(dev->tx_locks);
>  dev->tx_locks = NULL;
>  }
>  }
> @@ -1076,9 +1084,9 @@ netdev_afxdp_batch_send(struct netdev *netdev, int qid,
>  dev = netdev_linux_cast(netdev);
>  qid = qid % netdev_n_txq(netdev);
>  
> -ovs_spin_lock(>tx_locks[qid]);
> +ovs_spin_lock(>tx_locks[qid].lock);
>  ret = __netdev_afxdp_batch_send(netdev, qid, batch);
> -ovs_spin_unlock(>tx_locks[qid]);
> +ovs_spin_unlock(>tx_locks[qid].lock);
>  } else {
>  ret = __netdev_afxdp_batch_send(netdev, qid, batch);
>  }
> diff --git a/lib/netdev-afxdp.h b/lib/netdev-afxdp.h
> index 8188bc669..ae6971efd 100644
> --- a/lib/netdev-afxdp.h
> +++ b/lib/netdev-afxdp.h
> @@ -34,15 +34,16 @@ enum 

Re: [ovs-dev] [PATCH] netdev-afxdp: Add pcap dump support.

[William Tu]

On Mon, Dec 16, 2019 at 9:47 AM Ilya Maximets  wrote:
>
> On 16.12.2019 18:23, William Tu wrote:
> > On Mon, Dec 16, 2019 at 7:27 AM Ilya Maximets  wrote:
> >>
> >> On 16.12.2019 15:27, William Tu wrote:
> >>> On Mon, Dec 16, 2019 at 5:57 AM Ilya Maximets  wrote:
> 
>  On 16.12.2019 14:43, William Tu wrote:
> > On Mon, Dec 16, 2019 at 1:41 AM Ilya Maximets  
> > wrote:
> >>
> >> On 13.12.2019 21:59, William Tu wrote:
> >>> Debugging netdev-afxdp is hard because tcpdump does not work
> >>> at all, even for generic mode.  ovs-tcpdump which uses port
> >>> mirroring also does not work for netdev-afxdp.
> >>
> >> Hmm.  Why ovs-tcpdump doesn't work for you?  It should not depend
> >> on port type.  If it doesn't work we need to investigate this
> >> case and fix it because it's a very important tool.
> >
> > Because ovs-tcpdump still uses 'tcpdump' tool to capture packets,
> > (dump-cmd=tcpdump) and forward to mirror port.  Since tcpdump
> > sees no packet at all when type=afxdp, there is no packets to mirror.
> 
>  ovs-tcpdump creates simple tap interface and mirrors all the traffic
>  to it.  tcpdump command is started on that tap interface (no xdp here).
>  There is no difference from which port types you're mirroring the 
>  traffic.
>  It works for DPDK based ports and should work for afxdp.
> >>>
> >>> I got it working using ovs-tcpdump now, thanks!
> >>> ex: ovs-tcpdump -i afxdp-p0 -n -l -w /tmp/p0.pcap
> >>
> >> OK.  So, this patch is not needed.
> >
> > I think this patch is still useful, because using 'options:pcap'
> > is much easier than ovs-tcpdump (which requires python libs,
> > creates tap, etc).
> > Another use case is for testsuite, we can compare the packet
> > content by dumpling the pcap file.
> >
> > But this patch might have performance impact even
> > when not enabling it?
>
> Yes, it will have performance impact since you're calling it from
> the hot path.
>
> Also, current version will not work correctly.  clang build fails.
> (your travis link in commit-message is not correct).
> To implement the functionality properly you need to protect pcap
> streams by rcu.

Thanks, I paste the wrong link
The error was here:
https://travis-ci.org/williamtu/ovs-travis/builds/624759289

>
> I don't think that it's hard to use ovs-tcpdump.  Not harder than
> usual tcpdump.  All the additional ports and mirroring are created
> and configured automatically.
>
> Best regards, Ilya Maximets.

OK, make sense. I will use ovs-tcpdump.
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5 06/10] tc: Move tunnel_key unset action before output ports

[Marcelo Ricardo Leitner]

On Mon, Dec 16, 2019 at 05:53:17PM +0200, Paul Blakey wrote:
> diff --git a/lib/tc.c b/lib/tc.c
> index b2d8ca7..7a4acce 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -665,6 +665,12 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct 
> tc_flower *flower)

Not sure why but my 'git am' can't process this patch:
$ git am --show-current | patch -p1
patching file lib/tc.c
patch:  malformed patch at line 109: c_flower *flower)

git am --show-current  gives:

--- a/lib/tc.c
+++ b/lib/tc.c
@@ -665,6 +665,12 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct t=
  line broken here ^
c_flower *flower)
 flower->mask.tunnel.ttl =3D
  ^^ other encoding
 nl_attr_get_u8(attrs[TCA_FLOWER_KEY_ENC_IP_TTL_MASK]);
 }
+
+if (!is_all_zeros(>mask.tunnel, sizeof flower->mask.tunnel) ||
+!is_all_zeros(>key.tunnel, sizeof flower->key.tunnel)) {
+flower->tunnel =3D true;
 ^^ another here

but if I display it with mutt, I can see the patch just fine. Seems
git am is not decoding something here (git-2.23.0-1.fc31).


>  flower->mask.tunnel.ttl =
>  nl_attr_get_u8(attrs[TCA_FLOWER_KEY_ENC_IP_TTL_MASK]);
>  }
> +
> +if (!is_all_zeros(>mask.tunnel, sizeof flower->mask.tunnel) ||
> +!is_all_zeros(>key.tunnel, sizeof flower->key.tunnel)) {
> +flower->tunnel = true;
> +}
> +
>  if (attrs[TCA_FLOWER_KEY_ENC_OPTS] &&
>  attrs[TCA_FLOWER_KEY_ENC_OPTS_MASK]) {
>   err = nl_parse_flower_tunnel_opts(attrs[TCA_FLOWER_KEY_ENC_OPTS],

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

Re: [ovs-dev] [PATCH] netdev-afxdp: Add pcap dump support.

[Ilya Maximets]

On 16.12.2019 18:23, William Tu wrote:
> On Mon, Dec 16, 2019 at 7:27 AM Ilya Maximets  wrote:
>>
>> On 16.12.2019 15:27, William Tu wrote:
>>> On Mon, Dec 16, 2019 at 5:57 AM Ilya Maximets  wrote:

 On 16.12.2019 14:43, William Tu wrote:
> On Mon, Dec 16, 2019 at 1:41 AM Ilya Maximets  wrote:
>>
>> On 13.12.2019 21:59, William Tu wrote:
>>> Debugging netdev-afxdp is hard because tcpdump does not work
>>> at all, even for generic mode.  ovs-tcpdump which uses port
>>> mirroring also does not work for netdev-afxdp.
>>
>> Hmm.  Why ovs-tcpdump doesn't work for you?  It should not depend
>> on port type.  If it doesn't work we need to investigate this
>> case and fix it because it's a very important tool.
>
> Because ovs-tcpdump still uses 'tcpdump' tool to capture packets,
> (dump-cmd=tcpdump) and forward to mirror port.  Since tcpdump
> sees no packet at all when type=afxdp, there is no packets to mirror.

 ovs-tcpdump creates simple tap interface and mirrors all the traffic
 to it.  tcpdump command is started on that tap interface (no xdp here).
 There is no difference from which port types you're mirroring the traffic.
 It works for DPDK based ports and should work for afxdp.
>>>
>>> I got it working using ovs-tcpdump now, thanks!
>>> ex: ovs-tcpdump -i afxdp-p0 -n -l -w /tmp/p0.pcap
>>
>> OK.  So, this patch is not needed.
> 
> I think this patch is still useful, because using 'options:pcap'
> is much easier than ovs-tcpdump (which requires python libs,
> creates tap, etc).
> Another use case is for testsuite, we can compare the packet
> content by dumpling the pcap file.
> 
> But this patch might have performance impact even
> when not enabling it?

Yes, it will have performance impact since you're calling it from
the hot path.

Also, current version will not work correctly.  clang build fails.
(your travis link in commit-message is not correct).
To implement the functionality properly you need to protect pcap
streams by rcu.

I don't think that it's hard to use ovs-tcpdump.  Not harder than
usual tcpdump.  All the additional ports and mirroring are created
and configured automatically.

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

Re: [ovs-dev] [PATCH ovn] system-ovn.at: Fix failing system tests.

[Numan Siddique]

On Mon, Dec 16, 2019 at 10:16 AM Mark Michelson  wrote:
>
> Acked-by: Mark Michelson 
>

Thanks Dumitru and Mark. I applied this patch to master.

Numan

> On 12/6/19 10:14 AM, Dumitru Ceara wrote:
> > Upstream OVS commit:
> >  commit 03ccfe482ffdc6c1132aeb62467e0fbe6768f25d
> >  Author: William Tu 
> >  Date:   Tue Jun 25 14:52:38 2019 -0700
> >
> >  vswitchd: Separate disable system and route.
> >
> >  Previously, '--disable-system' disables both system dp and the system
> >  routing table.  The patch makes '--disable-system' only disable system
> >  dp and adds '--disable-system-route' for disabling the route table.
> >  This fixes failures when 'make check-system-userspace' for tunnel 
> > cases.
> >
> >  As a consequence, hitting errors due to OVS userspace parses the IGMP 
> > packet
> >  but its datapaths do not, so odp_flow_key_to_flow() return 
> > ODP_FIT_TOO_LITTLE.
> >  commit c645550bb249 ("odp-util: Always report ODP_FIT_TOO_LITTLE for 
> > IGMP.")
> >  Fix it by filtering out the IGMP-related error message.
> >
> >  Signed-off-by: William Tu 
> >  Signed-off-by: Yi-Hung Wei 
> >  Co-authored-by: Yi-Hung Wei 
> >  Signed-off-by: Ben Pfaff 
> >
> > This OVS change was performed in the OVS repo after the OVS-OVN split
> > and after the OVN tests/system-userspace-*.at files were created by
> > commit c17862ff04c3b315e5793c9c7fb04a79a892e6f3 and needs to be ported
> > to OVN too.
> >
> > CC: Numan Siddique 
> > Signed-off-by: Dumitru Ceara 
> > ---
> >   tests/ofproto-macros.at  | 2 +-
> >   tests/system-userspace-macros.at | 4 +++-
> >   2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
> > index 1fd546a..6c4ff60 100644
> > --- a/tests/ofproto-macros.at
> > +++ b/tests/ofproto-macros.at
> > @@ -196,7 +196,7 @@ m4_define([_OVS_VSWITCHD_START],
> >   # 'vswitchd-aux-args' provides a way to pass extra command line arguments
> >   # to ovs-vswitchd
> >   m4_define([OVS_VSWITCHD_START],
> > -  [_OVS_VSWITCHD_START([--enable-dummy$3 --disable-system $4])
> > +  [_OVS_VSWITCHD_START([--enable-dummy$3 --disable-system 
> > --disable-system-route $4])
> >  AT_CHECK([add_of_br 0 $1 m4_if([$2], [], [], [| uuidfilt])], [0], [$2])
> >   ])
> >
> > diff --git a/tests/system-userspace-macros.at 
> > b/tests/system-userspace-macros.at
> > index d8cc686..86ed226 100644
> > --- a/tests/system-userspace-macros.at
> > +++ b/tests/system-userspace-macros.at
> > @@ -36,7 +36,9 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START],
> >   m4_define([OVS_TRAFFIC_VSWITCHD_STOP],
> > [OVS_VSWITCHD_STOP([dnl
> >   $1";/netdev_linux.*obtaining netdev stats via vport failed/d
> > -/dpif_netlink.*Generic Netlink family 'ovs_datapath' does not exist. The 
> > Open vSwitch kernel module is probably not loaded./d"])
> > +/dpif_netlink.*Generic Netlink family 'ovs_datapath' does not exist. The 
> > Open vSwitch kernel module is probably not loaded./d
> > +/dpif_netdev(revalidator.*)|ERR|internal error parsing flow 
> > key.*proto=2.*/d
> > +/dpif(revalidator.*)|WARN|netdev@ovs-netdev: failed to.*proto=2.*/d"])
> >  AT_CHECK([:; $2])
> > ])
> >
> >
>
> ___
> 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] Documentation: Fix ovs-tcpdump options.

[Gregory Rose]

On 12/16/2019 6:18 AM, William Tu wrote:

Signed-off-by: William Tu 
---
  Documentation/ref/ovs-tcpdump.8.rst | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/ref/ovs-tcpdump.8.rst 
b/Documentation/ref/ovs-tcpdump.8.rst
index 048b70f23daf..b9f8cdf6f786 100644
--- a/Documentation/ref/ovs-tcpdump.8.rst
+++ b/Documentation/ref/ovs-tcpdump.8.rst
@@ -5,7 +5,7 @@ ovs-tcpdump
  Synopsis
  
  
-``ovs-tcpdump -i  tcpdump ...``

+``ovs-tcpdump -i  ...``
  
  Description

  ===


LGTM - Thanks!

Tested-by: Greg Rose 
Reviewed-by: Greg Rose 

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

Re: [ovs-dev] [PATCH] netdev-afxdp: Add pcap dump support.

[William Tu]

On Mon, Dec 16, 2019 at 7:27 AM Ilya Maximets  wrote:
>
> On 16.12.2019 15:27, William Tu wrote:
> > On Mon, Dec 16, 2019 at 5:57 AM Ilya Maximets  wrote:
> >>
> >> On 16.12.2019 14:43, William Tu wrote:
> >>> On Mon, Dec 16, 2019 at 1:41 AM Ilya Maximets  wrote:
> 
>  On 13.12.2019 21:59, William Tu wrote:
> > Debugging netdev-afxdp is hard because tcpdump does not work
> > at all, even for generic mode.  ovs-tcpdump which uses port
> > mirroring also does not work for netdev-afxdp.
> 
>  Hmm.  Why ovs-tcpdump doesn't work for you?  It should not depend
>  on port type.  If it doesn't work we need to investigate this
>  case and fix it because it's a very important tool.
> >>>
> >>> Because ovs-tcpdump still uses 'tcpdump' tool to capture packets,
> >>> (dump-cmd=tcpdump) and forward to mirror port.  Since tcpdump
> >>> sees no packet at all when type=afxdp, there is no packets to mirror.
> >>
> >> ovs-tcpdump creates simple tap interface and mirrors all the traffic
> >> to it.  tcpdump command is started on that tap interface (no xdp here).
> >> There is no difference from which port types you're mirroring the traffic.
> >> It works for DPDK based ports and should work for afxdp.
> >
> > I got it working using ovs-tcpdump now, thanks!
> > ex: ovs-tcpdump -i afxdp-p0 -n -l -w /tmp/p0.pcap
>
> OK.  So, this patch is not needed.

I think this patch is still useful, because using 'options:pcap'
is much easier than ovs-tcpdump (which requires python libs,
creates tap, etc).
Another use case is for testsuite, we can compare the packet
content by dumpling the pcap file.

But this patch might have performance impact even
when not enabling it?

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

Re: [ovs-dev] [PATCH V4 12/17] netdev-offload-dpdk: Framework for actions offload

[0-day Robot]

Bleep bloop.  Greetings Eli Britstein, 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: Line is 80 characters long (recommended limit is 79)
#145 FILE: lib/netdev-offload-dpdk.c:801:
flow = netdev_offload_dpdk_mark_rss(, netdev, info->flow_mark);

Lines checked: 160, 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 v5 04/10] netdev-offload-tc: Implement netdev tc flush via tc filter del

[Paul Blakey]

To be consistent with our tc-ufid mapping after flush, and to support tc
chains flushing in the next commit, implement flush operation via
deleting all the filters we actually added and delete their mappings.

This will also not delete the configured qos policing via matchall filters,
while old code did.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
 lib/netdev-offload-tc.c | 74 +++--
 1 file changed, 41 insertions(+), 33 deletions(-)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 460c27f..15b39e6 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -43,7 +43,8 @@ VLOG_DEFINE_THIS_MODULE(netdev_offload_tc);
 
 static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(60, 5);
 
-static struct hmap ufid_tc = HMAP_INITIALIZER(_tc);
+static struct hmap ufid_to_tc = HMAP_INITIALIZER(_to_tc);
+static struct hmap tc_to_ufid = HMAP_INITIALIZER(_to_ufid);
 static bool multi_mask_per_prio = false;
 static bool block_support = false;
 
@@ -143,44 +144,49 @@ static struct netlink_field set_flower_map[][4] = {
 static struct ovs_mutex ufid_lock = OVS_MUTEX_INITIALIZER;
 
 /**
- * struct ufid_tc_data - data entry for ufid_tc hmap.
- * @ufid_node: Element in @ufid_tc hash table by ufid key.
- * @tc_node: Element in @ufid_tc hash table by tcf_id key.
+ * struct ufid_tc_data - data entry for ufid-tc hashmaps.
+ * @ufid_to_tc_node: Element in @ufid_to_tc hash table by ufid key.
+ * @tc_to_ufid_node: Element in @tc_to_ufid hash table by tcf_id key.
  * @ufid: ufid assigned to the flow
  * @id: tc filter id (tcf_id)
  * @netdev: netdev associated with the tc rule
  */
 struct ufid_tc_data {
-struct hmap_node ufid_node;
-struct hmap_node tc_node;
+struct hmap_node ufid_to_tc_node;
+struct hmap_node tc_to_ufid_node;
 ovs_u128 ufid;
 struct tcf_id id;
 struct netdev *netdev;
 };
 
-/* Remove matching ufid entry from ufid_tc hashmap. */
 static void
-del_ufid_tc_mapping(const ovs_u128 *ufid)
+del_ufid_tc_mapping_unlocked(const ovs_u128 *ufid)
 {
 size_t ufid_hash = hash_bytes(ufid, sizeof *ufid, 0);
 struct ufid_tc_data *data;
 
-ovs_mutex_lock(_lock);
-HMAP_FOR_EACH_WITH_HASH(data, ufid_node, ufid_hash, _tc) {
+HMAP_FOR_EACH_WITH_HASH (data, ufid_to_tc_node, ufid_hash, _to_tc) {
 if (ovs_u128_equals(*ufid, data->ufid)) {
 break;
 }
 }
 
 if (!data) {
-ovs_mutex_unlock(_lock);
 return;
 }
 
-hmap_remove(_tc, >ufid_node);
-hmap_remove(_tc, >tc_node);
+hmap_remove(_to_tc, >ufid_to_tc_node);
+hmap_remove(_to_ufid, >tc_to_ufid_node);
 netdev_close(data->netdev);
 free(data);
+}
+
+/* Remove matching ufid entry from ufid-tc hashmaps. */
+static void
+del_ufid_tc_mapping(const ovs_u128 *ufid)
+{
+ovs_mutex_lock(_lock);
+del_ufid_tc_mapping_unlocked(ufid);
 ovs_mutex_unlock(_lock);
 }
 
@@ -195,7 +201,7 @@ del_filter_and_ufid_mapping(struct tcf_id *id, const 
ovs_u128 *ufid)
 return err;
 }
 
-/* Add ufid entry to ufid_tc hashmap. */
+/* Add ufid entry to ufid_to_tc hashmap. */
 static void
 add_ufid_tc_mapping(struct netdev *netdev, const ovs_u128 *ufid,
 struct tcf_id *id)
@@ -209,12 +215,12 @@ add_ufid_tc_mapping(struct netdev *netdev, const ovs_u128 
*ufid,
 new_data->netdev = netdev_ref(netdev);
 
 ovs_mutex_lock(_lock);
-hmap_insert(_tc, _data->ufid_node, ufid_hash);
-hmap_insert(_tc, _data->tc_node, tc_hash);
+hmap_insert(_to_tc, _data->ufid_to_tc_node, ufid_hash);
+hmap_insert(_to_ufid, _data->tc_to_ufid_node, tc_hash);
 ovs_mutex_unlock(_lock);
 }
 
-/* Get tc id from ufid_tc hashmap.
+/* Get tc id from ufid_to_tc hashmap.
  *
  * Returns 0 if successful and fills id.
  * Otherwise returns the error.
@@ -226,7 +232,7 @@ get_ufid_tc_mapping(const ovs_u128 *ufid, struct tcf_id *id)
 struct ufid_tc_data *data;
 
 ovs_mutex_lock(_lock);
-HMAP_FOR_EACH_WITH_HASH(data, ufid_node, ufid_hash, _tc) {
+HMAP_FOR_EACH_WITH_HASH (data, ufid_to_tc_node, ufid_hash, _to_tc) {
 if (ovs_u128_equals(*ufid, data->ufid)) {
 *id = data->id;
 ovs_mutex_unlock(_lock);
@@ -238,7 +244,7 @@ get_ufid_tc_mapping(const ovs_u128 *ufid, struct tcf_id *id)
 return ENOENT;
 }
 
-/* Find ufid entry in ufid_tc hashmap using tcf_id id.
+/* Find ufid entry in ufid_to_tc hashmap using tcf_id id.
  * The result is saved in ufid.
  *
  * Returns true on success.
@@ -250,7 +256,7 @@ find_ufid(struct netdev *netdev, struct tcf_id *id, 
ovs_u128 *ufid)
 struct ufid_tc_data *data;
 
 ovs_mutex_lock(_lock);
-HMAP_FOR_EACH_WITH_HASH(data, tc_node, tc_hash,  _tc) {
+HMAP_FOR_EACH_WITH_HASH (data, tc_to_ufid_node, tc_hash,  _to_ufid) {
 if (netdev == data->netdev && is_tcf_id_eq(>id, id)) {
 *ufid = data->ufid;
 break;
@@ -293,7 +299,7 @@ get_prio_for_tc_flower(struct tc_flower *flower)
 

[ovs-dev] [PATCH v5 08/10] netdev-offload-tc: Add conntrack support

[Paul Blakey]

Zone and ct_state first.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:
V4->V5:
Removed unused int err in parse_put_flow_ct_action
---
 lib/dpif-netlink.c  |   2 +
 lib/netdev-offload-tc.c | 135 
 lib/tc.c| 122 +++
 lib/tc.h|  11 
 4 files changed, 260 insertions(+), 10 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 5d785e4..cbd459a 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1643,6 +1643,8 @@ dpif_netlink_netdev_match_to_dpif_flow(struct match 
*match,
 .support = {
 .max_vlan_headers = 2,
 .recirc = true,
+.ct_state = true,
+.ct_zone = true,
 },
 };
 size_t offset;
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index e3b0415..6d2bcbe 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -595,6 +595,35 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 match_set_tp_dst_masked(match, key->sctp_dst, mask->sctp_dst);
 match_set_tp_src_masked(match, key->sctp_src, mask->sctp_src);
 }
+
+if (mask->ct_state) {
+uint8_t ct_statev = 0, ct_statem = 0;
+
+if (mask->ct_state & TCA_FLOWER_KEY_CT_FLAGS_NEW) {
+if (key->ct_state & TCA_FLOWER_KEY_CT_FLAGS_NEW) {
+ct_statev |= OVS_CS_F_NEW;
+}
+ct_statem |= OVS_CS_F_NEW;
+}
+
+if (mask->ct_state & TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED) {
+if (key->ct_state & TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED) {
+ct_statev |= OVS_CS_F_ESTABLISHED;
+}
+ct_statem |= OVS_CS_F_ESTABLISHED;
+}
+
+if (mask->ct_state & TCA_FLOWER_KEY_CT_FLAGS_TRACKED) {
+if (key->ct_state & TCA_FLOWER_KEY_CT_FLAGS_TRACKED) {
+ct_statev |= OVS_CS_F_TRACKED;
+}
+ct_statem |= OVS_CS_F_TRACKED;
+}
+
+match_set_ct_state_masked(match, ct_statev, ct_statem);
+}
+
+match_set_ct_zone_masked(match, key->ct_zone, mask->ct_zone);
 }
 
 if (flower->tunnel) {
@@ -746,6 +775,27 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 nl_msg_put_u32(buf, OVS_ACTION_ATTR_OUTPUT, 
odp_to_u32(outport));
 }
 break;
+case TC_ACT_CT: {
+size_t ct_offset;
+
+if (action->ct.clear) {
+nl_msg_put_flag(buf, OVS_ACTION_ATTR_CT_CLEAR);
+break;
+}
+
+ct_offset = nl_msg_start_nested(buf, OVS_ACTION_ATTR_CT);
+
+if (action->ct.commit) {
+nl_msg_put_flag(buf, OVS_CT_ATTR_COMMIT);
+}
+
+if (action->ct.zone) {
+nl_msg_put_u16(buf, OVS_CT_ATTR_ZONE, action->ct.zone);
+}
+
+nl_msg_end_nested(buf, ct_offset);
+}
+break;
 case TC_ACT_GOTO: {
 nl_msg_put_u32(buf, OVS_ACTION_ATTR_RECIRC, action->chain);
 }
@@ -835,6 +885,33 @@ parse_mpls_set_action(struct tc_flower *flower, struct 
tc_action *action,
 }
 
 static int
+parse_put_flow_ct_action(struct tc_flower *flower,
+ struct tc_action *action,
+ const struct nlattr *ct,
+ size_t ct_len)
+{
+const struct nlattr *ct_attr;
+size_t ct_left;
+
+NL_ATTR_FOR_EACH_UNSAFE (ct_attr, ct_left, ct, ct_len) {
+switch (nl_attr_type(ct_attr)) {
+case OVS_CT_ATTR_COMMIT: {
+action->ct.commit = true;
+}
+break;
+case OVS_CT_ATTR_ZONE: {
+action->ct.zone = nl_attr_get_u16(ct_attr);
+}
+break;
+}
+}
+
+action->type = TC_ACT_CT;
+flower->action_count++;
+return 0;
+}
+
+static int
 parse_put_flow_set_masked_action(struct tc_flower *flower,
  struct tc_action *action,
  const struct nlattr *set,
@@ -1016,16 +1093,6 @@ test_key_and_mask(struct match *match)
 return EOPNOTSUPP;
 }
 
-if (mask->ct_state) {
-VLOG_DBG_RL(, "offloading attribute ct_state isn't supported");
-return EOPNOTSUPP;
-}
-
-if (mask->ct_zone) {
-VLOG_DBG_RL(, "offloading attribute ct_zone isn't supported");
-return EOPNOTSUPP;
-}
-
 if (mask->ct_mark) {
 VLOG_DBG_RL(, "offloading attribute ct_mark isn't supported");
 return EOPNOTSUPP;
@@ -1364,6 +1431,42 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 }
 

[ovs-dev] [PATCH v5 10/10] netdev-offload-tc: Add conntrack nat support

[Paul Blakey]

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:
V4->V5:
Moved the unused 'int err' from patch 4/10 to it's first usage here
V2->V3:
Ipv6 nat dump fix (missing unspec type in policy)
Renamed tc range struct to be consistent with NL ATTRIBUTES and ovs

V1->V2:
Missing ntohs/htons on nat port range.
---
 lib/netdev-offload-tc.c | 105 
 lib/tc.c|  93 ++
 lib/tc.h|  28 +
 3 files changed, 226 insertions(+)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index b2c2c91..66cd45b 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -815,6 +815,40 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 ct_label->mask = action->ct.label_mask;
 }
 
+if (action->ct.nat_type) {
+size_t nat_offset = nl_msg_start_nested(buf,
+OVS_CT_ATTR_NAT);
+
+if (action->ct.nat_type == TC_NAT_SRC) {
+nl_msg_put_flag(buf, OVS_NAT_ATTR_SRC);
+} else if (action->ct.nat_type == TC_NAT_DST) {
+nl_msg_put_flag(buf, OVS_NAT_ATTR_DST);
+}
+
+if (action->ct.range.ip_family == AF_INET) {
+nl_msg_put_be32(buf, OVS_NAT_ATTR_IP_MIN,
+action->ct.range.ipv4.min);
+nl_msg_put_be32(buf, OVS_NAT_ATTR_IP_MAX,
+action->ct.range.ipv4.max);
+} else if (action->ct.range.ip_family == AF_INET6) {
+nl_msg_put_in6_addr(buf, OVS_NAT_ATTR_IP_MIN,
+>ct.range.ipv6.min);
+nl_msg_put_in6_addr(buf, OVS_NAT_ATTR_IP_MAX,
+>ct.range.ipv6.max);
+}
+
+if (action->ct.range.port.min) {
+nl_msg_put_u16(buf, OVS_NAT_ATTR_PROTO_MIN,
+   ntohs(action->ct.range.port.min));
+if (action->ct.range.port.max) {
+nl_msg_put_u16(buf, OVS_NAT_ATTR_PROTO_MAX,
+   ntohs(action->ct.range.port.max));
+}
+}
+
+nl_msg_end_nested(buf, nat_offset);
+}
+
 nl_msg_end_nested(buf, ct_offset);
 }
 break;
@@ -907,6 +941,66 @@ parse_mpls_set_action(struct tc_flower *flower, struct 
tc_action *action,
 }
 
 static int
+parse_put_flow_nat_action(struct tc_action *action,
+  const struct nlattr *nat,
+  size_t nat_len)
+{
+const struct nlattr *nat_attr;
+size_t nat_left;
+
+action->ct.nat_type = TC_NAT_RESTORE;
+NL_ATTR_FOR_EACH_UNSAFE (nat_attr, nat_left, nat, nat_len) {
+switch (nl_attr_type(nat_attr)) {
+case OVS_NAT_ATTR_SRC: {
+action->ct.nat_type = TC_NAT_SRC;
+};
+break;
+case OVS_NAT_ATTR_DST: {
+action->ct.nat_type = TC_NAT_DST;
+};
+break;
+case OVS_NAT_ATTR_IP_MIN: {
+if (nl_attr_get_size(nat_attr) == sizeof(ovs_be32)) {
+ovs_be32 addr = nl_attr_get_be32(nat_attr);
+
+action->ct.range.ipv4.min = addr;
+action->ct.range.ip_family = AF_INET;
+} else {
+struct in6_addr addr = nl_attr_get_in6_addr(nat_attr);
+
+action->ct.range.ipv6.min = addr;
+action->ct.range.ip_family = AF_INET6;
+}
+};
+break;
+case OVS_NAT_ATTR_IP_MAX: {
+if (nl_attr_get_size(nat_attr) == sizeof(ovs_be32)) {
+ovs_be32 addr = nl_attr_get_be32(nat_attr);
+
+action->ct.range.ipv4.max = addr;
+action->ct.range.ip_family = AF_INET;
+} else {
+struct in6_addr addr = nl_attr_get_in6_addr(nat_attr);
+
+action->ct.range.ipv6.max = addr;
+action->ct.range.ip_family = AF_INET6;
+}
+};
+break;
+case OVS_NAT_ATTR_PROTO_MIN: {
+action->ct.range.port.min = htons(nl_attr_get_u16(nat_attr));
+};
+break;
+case OVS_NAT_ATTR_PROTO_MAX: {
+action->ct.range.port.max = htons(nl_attr_get_u16(nat_attr));
+};
+break;
+}
+}
+return 0;
+}
+
+static int
 parse_put_flow_ct_action(struct tc_flower *flower,
   

[ovs-dev] [PATCH v5 01/10] match: Add match_set_ct_zone_masked helper

[Paul Blakey]

Sets zone in match.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
 include/openvswitch/match.h |  2 ++
 lib/match.c | 10 --
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
index 05ecee7..eeabd5f 100644
--- a/include/openvswitch/match.h
+++ b/include/openvswitch/match.h
@@ -127,6 +127,8 @@ void match_set_pkt_mark_masked(struct match *, uint32_t 
pkt_mark, uint32_t mask)
 void match_set_ct_state(struct match *, uint32_t ct_state);
 void match_set_ct_state_masked(struct match *, uint32_t ct_state, uint32_t 
mask);
 void match_set_ct_zone(struct match *, uint16_t ct_zone);
+void match_set_ct_zone_masked(struct match *match, uint16_t ct_zone,
+  uint16_t mask);
 void match_set_ct_mark(struct match *, uint32_t ct_mark);
 void match_set_ct_mark_masked(struct match *, uint32_t ct_mark, uint32_t mask);
 void match_set_ct_label(struct match *, ovs_u128 ct_label);
diff --git a/lib/match.c b/lib/match.c
index ae56828..0d1ec31 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -417,8 +417,14 @@ match_set_ct_state_masked(struct match *match, uint32_t 
ct_state, uint32_t mask)
 void
 match_set_ct_zone(struct match *match, uint16_t ct_zone)
 {
-match->flow.ct_zone = ct_zone;
-match->wc.masks.ct_zone = UINT16_MAX;
+match_set_ct_zone_masked(match, ct_zone, UINT16_MAX);
+}
+
+void
+match_set_ct_zone_masked(struct match *match, uint16_t ct_zone, uint16_t mask)
+{
+match->flow.ct_zone = ct_zone & mask;
+match->wc.masks.ct_zone = mask;
 }
 
 void
-- 
1.8.3.1

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

[ovs-dev] [PATCH v5 06/10] tc: Move tunnel_key unset action before output ports

[Paul Blakey]

Since OvS datapath gets packets already decapsulated from tunnel devices,
it doesn't explicitly decapsulate them. So in a recirculation setup,
the tunnel matching continues in the recirculation as the tunnel
metadata still exists on the SKB.

Tunnel key unset action unsets this metadata. Some drivers might rely
on this explicit tunnel key unset to know when to decapsulate the packet
instead of the device type. So instead of removing it completly,
we move it near the output actions.

This way, we also keep SKB metadata through recirculation, and for
non-recirculation rules, the resulting tc rules should remain the same.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:
V3->V4:
Fix: Set flower tunnel attribute to true, if any of the tunnel
 attributes exists (we used to rely on unset action).
V2->V3:
Actually removed old tunnel set if tunnel exists (instead of just
adding a new one before a port)
Moved patch earlier to help git bisect
---
 lib/tc.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/lib/tc.c b/lib/tc.c
index b2d8ca7..7a4acce 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -665,6 +665,12 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct 
tc_flower *flower)
 flower->mask.tunnel.ttl =
 nl_attr_get_u8(attrs[TCA_FLOWER_KEY_ENC_IP_TTL_MASK]);
 }
+
+if (!is_all_zeros(>mask.tunnel, sizeof flower->mask.tunnel) ||
+!is_all_zeros(>key.tunnel, sizeof flower->key.tunnel)) {
+flower->tunnel = true;
+}
+
 if (attrs[TCA_FLOWER_KEY_ENC_OPTS] &&
 attrs[TCA_FLOWER_KEY_ENC_OPTS_MASK]) {
  err = nl_parse_flower_tunnel_opts(attrs[TCA_FLOWER_KEY_ENC_OPTS],
@@ -2091,24 +2097,17 @@ nl_msg_put_flower_rewrite_pedits(struct ofpbuf *request,
 static int
 nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower)
 {
+bool ingress, released = false;
 size_t offset;
 size_t act_offset;
 uint16_t act_index = 1;
 struct tc_action *action;
 int i, ifindex = 0;
-bool ingress;
 
 offset = nl_msg_start_nested(request, TCA_FLOWER_ACT);
 {
 int error;
 
-if (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);
-}
-
 action = flower->actions;
 for (i = 0; i < flower->action_count; i++, action++) {
 switch (action->type) {
@@ -2185,6 +2184,13 @@ 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_end_nested(request, act_offset);
+released = true;
+}
+
 ingress = action->out.ingress;
 ifindex = action->out.ifindex_out;
 if (ifindex < 1) {
-- 
1.8.3.1

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

[ovs-dev] [PATCH v5 03/10] tc: Introduce tcf_id to specify a tc filter

[Paul Blakey]

Move all that is needed to identify a tc filter to a
new structure, tcf_id. This removes a lot of duplication
in accessing/creating tc filters.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:
V3->V4:
Fix accidently removed Block_id in flow_put()
V2->V3:
Renamed *tc_id* -> *tcf_id*
Renamed make_tc_id -> tc_make_tcf_id to be consistent with tc_make_* helpers

V1->V2:
In tc_del_matchall_policer - reverse xmas param order
Added and used helper is_tc_id_eq(id1, id2) in find_ufid
In netdev_tc_flow_dump_next - use make_tc_id() instead of manualy filling id
In netdev_tc_flow_put - use if (get_ufid_tc_mapping(ufid, ) == 0) to be 
mor explict we found the mapping not failed to get
In make_tc_id - fill id explictily and removed memset.
Moved make_tc_id to be static inline in tc.h
---
 lib/netdev-linux.c  |   6 +-
 lib/netdev-offload-tc.c | 205 
 lib/tc.c| 109 -
 lib/tc.h|  51 +---
 4 files changed, 159 insertions(+), 212 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index f8e59ba..8a62f9d 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -2356,7 +2356,9 @@ tc_add_matchall_policer(struct netdev *netdev, uint32_t 
kbits_rate,
 static int
 tc_del_matchall_policer(struct netdev *netdev)
 {
+int prio = TC_RESERVED_PRIORITY_POLICE;
 uint32_t block_id = 0;
+struct tcf_id id;
 int ifindex;
 int err;
 
@@ -2365,8 +2367,8 @@ tc_del_matchall_policer(struct netdev *netdev)
 return err;
 }
 
-err = tc_del_filter(ifindex, TC_RESERVED_PRIORITY_POLICE, 1, block_id,
-TC_INGRESS);
+id = tc_make_tcf_id(ifindex, block_id, prio, TC_INGRESS);
+err = tc_del_filter();
 if (err) {
 return err;
 }
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 1adbb32..460c27f 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -145,20 +145,16 @@ static struct ovs_mutex ufid_lock = OVS_MUTEX_INITIALIZER;
 /**
  * struct ufid_tc_data - data entry for ufid_tc hmap.
  * @ufid_node: Element in @ufid_tc hash table by ufid key.
- * @tc_node: Element in @ufid_tc hash table by prio/handle/ifindex key.
+ * @tc_node: Element in @ufid_tc hash table by tcf_id key.
  * @ufid: ufid assigned to the flow
- * @prio: tc priority
- * @handle: tc handle
- * @ifindex: netdev ifindex.
+ * @id: tc filter id (tcf_id)
  * @netdev: netdev associated with the tc rule
  */
 struct ufid_tc_data {
 struct hmap_node ufid_node;
 struct hmap_node tc_node;
 ovs_u128 ufid;
-uint16_t prio;
-uint32_t handle;
-int ifindex;
+struct tcf_id id;
 struct netdev *netdev;
 };
 
@@ -190,32 +186,27 @@ del_ufid_tc_mapping(const ovs_u128 *ufid)
 
 /* Wrapper function to delete filter and ufid tc mapping */
 static int
-del_filter_and_ufid_mapping(int ifindex, int prio, int handle,
-uint32_t block_id, const ovs_u128 *ufid,
-enum tc_qdisc_hook hook)
+del_filter_and_ufid_mapping(struct tcf_id *id, const ovs_u128 *ufid)
 {
 int err;
 
-err = tc_del_filter(ifindex, prio, handle, block_id, hook);
+err = tc_del_filter(id);
 del_ufid_tc_mapping(ufid);
-
 return err;
 }
 
 /* Add ufid entry to ufid_tc hashmap. */
 static void
-add_ufid_tc_mapping(const ovs_u128 *ufid, int prio, int handle,
-struct netdev *netdev, int ifindex)
+add_ufid_tc_mapping(struct netdev *netdev, const ovs_u128 *ufid,
+struct tcf_id *id)
 {
 size_t ufid_hash = hash_bytes(ufid, sizeof *ufid, 0);
-size_t tc_hash = hash_int(hash_int(prio, handle), ifindex);
+size_t tc_hash = hash_int(hash_int(id->prio, id->handle), id->ifindex);
 struct ufid_tc_data *new_data = xzalloc(sizeof *new_data);
 
 new_data->ufid = *ufid;
-new_data->prio = prio;
-new_data->handle = handle;
+new_data->id = *id;
 new_data->netdev = netdev_ref(netdev);
-new_data->ifindex = ifindex;
 
 ovs_mutex_lock(_lock);
 hmap_insert(_tc, _data->ufid_node, ufid_hash);
@@ -223,56 +214,44 @@ add_ufid_tc_mapping(const ovs_u128 *ufid, int prio, int 
handle,
 ovs_mutex_unlock(_lock);
 }
 
-/* Get ufid from ufid_tc hashmap.
+/* Get tc id from ufid_tc hashmap.
  *
- * If netdev output param is not NULL then the function will return
- * associated netdev on success and a refcount is taken on that netdev.
- * The caller is then responsible to close the netdev.
- *
- * Returns handle if successful and fill prio and netdev for that ufid.
- * Otherwise returns 0.
+ * Returns 0 if successful and fills id.
+ * Otherwise returns the error.
  */
 static int
-get_ufid_tc_mapping(const ovs_u128 *ufid, int *prio, struct netdev **netdev)
+get_ufid_tc_mapping(const ovs_u128 *ufid, struct tcf_id *id)
 {
 size_t ufid_hash = hash_bytes(ufid, sizeof *ufid, 0);
 struct ufid_tc_data 

[ovs-dev] [PATCH v5 07/10] netdev-offload-tc: Add recirculation support via tc chains

[Paul Blakey]

Each recirculation id will create a tc chain, and we translate
the recirculation action to a tc goto chain action.

We check for kernel support for this by probing OvS Datapath for the
tc recirc id sharing feature. If supported, we can offload rules
that match on recirc_id, and recirculation action safely.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:
V4->V5:
Always enable recirc_id sharing to avoid bugs with opening an
existing datapath, and enabling offload later.
V3->V4:
Always try to enable recirc_id sharing (on dpif_open) if hardware offload 
is enabled.
V2->V3:
Merged part of probe for recirc_id support in here to help future git 
bisect.
Added tunnel released check to avoid bug with mirroring
Removed cascading condition in netdev_tc_flow_put() check of recirc_id 
support

V1->V2:
moved make_tc_id_chain helper to tc.h as static inline
updated is_tc_id_eq with chain compare instead of find_ufid
---
 lib/dpif-netlink.c  | 14 ++
 lib/netdev-offload-tc.c | 35 +--
 lib/tc.c| 49 +++--
 lib/tc.h| 18 +-
 4 files changed, 99 insertions(+), 17 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index ef06dd4..5d785e4 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -110,6 +110,8 @@ static int dpif_netlink_dp_transact(const struct 
dpif_netlink_dp *request,
 static int dpif_netlink_dp_get(const struct dpif *,
struct dpif_netlink_dp *reply,
struct ofpbuf **bufp);
+static int
+dpif_netlink_set_features(struct dpif *dpif_, uint32_t new_features);
 
 struct dpif_netlink_flow {
 /* Generic Netlink header. */
@@ -363,7 +365,9 @@ dpif_netlink_open(const struct dpif_class *class 
OVS_UNUSED, const char *name,
 }
 
 error = open_dpif(, dpifp);
+dpif_netlink_set_features(*dpifp, OVS_DP_F_TC_RECIRC_SHARING);
 ofpbuf_delete(buf);
+
 return error;
 }
 
@@ -1638,6 +1642,7 @@ dpif_netlink_netdev_match_to_dpif_flow(struct match 
*match,
 .mask = >wc.masks,
 .support = {
 .max_vlan_headers = 2,
+.recirc = true,
 },
 };
 size_t offset;
@@ -2037,6 +2042,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
dpif_flow_put *put)
 struct offload_info info;
 ovs_be16 dst_port = 0;
 uint8_t csum_on = false;
+bool recirc_act;
 int err;
 
 if (put->flags & DPIF_FP_PROBE) {
@@ -2076,9 +2082,17 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
dpif_flow_put *put)
 csum_on = tnl_cfg->csum;
 }
 netdev_close(outdev);
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_RECIRC) {
+recirc_act = true;
 }
 }
 
+if ((recirc_act || match.flow.recirc_id)
+&& !(dpif->user_features & OVS_DP_F_TC_RECIRC_SHARING)) {
+err = EOPNOTSUPP;
+goto out;
+}
+
 info.dpif_class = dpif_class;
 info.tp_dst_port = dst_port;
 info.tunnel_csum_on = csum_on;
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 15b39e6..e3b0415 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -38,6 +38,7 @@
 #include "tc.h"
 #include "unaligned.h"
 #include "util.h"
+#include "dpif-provider.h"
 
 VLOG_DEFINE_THIS_MODULE(netdev_offload_tc);
 
@@ -206,9 +207,12 @@ static void
 add_ufid_tc_mapping(struct netdev *netdev, const ovs_u128 *ufid,
 struct tcf_id *id)
 {
-size_t ufid_hash = hash_bytes(ufid, sizeof *ufid, 0);
-size_t tc_hash = hash_int(hash_int(id->prio, id->handle), id->ifindex);
 struct ufid_tc_data *new_data = xzalloc(sizeof *new_data);
+size_t ufid_hash = hash_bytes(ufid, sizeof *ufid, 0);
+size_t tc_hash;
+
+tc_hash = hash_int(hash_int(id->prio, id->handle), id->ifindex);
+tc_hash = hash_int(id->chain, tc_hash);
 
 new_data->ufid = *ufid;
 new_data->id = *id;
@@ -252,8 +256,11 @@ get_ufid_tc_mapping(const ovs_u128 *ufid, struct tcf_id 
*id)
 static bool
 find_ufid(struct netdev *netdev, struct tcf_id *id, ovs_u128 *ufid)
 {
-size_t tc_hash = hash_int(hash_int(id->prio, id->handle), id->ifindex);
 struct ufid_tc_data *data;
+size_t tc_hash;
+
+tc_hash = hash_int(hash_int(id->prio, id->handle), id->ifindex);
+tc_hash = hash_int(id->chain, tc_hash);
 
 ovs_mutex_lock(_lock);
 HMAP_FOR_EACH_WITH_HASH (data, tc_to_ufid_node, tc_hash,  _to_ufid) {
@@ -739,6 +746,10 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 nl_msg_put_u32(buf, OVS_ACTION_ATTR_OUTPUT, 
odp_to_u32(outport));
 }
 break;
+case TC_ACT_GOTO: {
+nl_msg_put_u32(buf, OVS_ACTION_ATTR_RECIRC, action->chain);
+}
+break;
 }
 }
 }
@@ -799,6 +810,7 @@ netdev_tc_flow_dump_next(struct 

[ovs-dev] [PATCH v5 00/10] Add support for offloading CT datapath rules to TC

[Paul Blakey]

The following patchset introduces hardware offload of OVS connection
tracking datapath rules.

OVS uses ct() and recirc() (recirculation) actions and recirc_id()/ct_state()
matches to support connection tracking.

The datapath rules are in the form of:

recirc_id(0),in_port(dev1),eth_type(0x0800),ct_state(-trk) 
actions:ct(),recirc(2)
recirc_id(2),in_port(dev1),eth_type(0x0800),ct_state(+trk+est) actions:4

This patchset will translate ct_state() and recirc_id() matches to tc 
ct_state and chain matches respectively. The datapath actions ct() and recirc()
will be translated to tc actions ct and goto chain respectively.

The tc equivalent commands for the above rules are:

$ tc filter add dev dev1 ingress \
prio 1 chain 0 proto ip \
flower tcp ct_state -trk \
action ct pipe \
action goto chain 2

$ tc filter add dev dev1 ingress \
prio 1 chain 2 proto ip \
flower tcp ct_state +trk+est \
action mirred egress redirect dev dev2

Thanks,
Paul

Paul Blakey (10):
  match: Add match_set_ct_zone_masked helper
  compat: Add tc ct action and flower matches defines for older kernels
  tc: Introduce tcf_id to specify a tc filter
  netdev-offload-tc: Implement netdev tc flush via tc filter del
  dpif: Add support to set user features
  tc: Move tunnel_key unset action before output ports
  netdev-offload-tc: Add recirculation support via tc chains
  netdev-offload-tc: Add conntrack support
  netdev-offload-tc: Add conntrack label and mark support
  netdev-offload-tc: Add conntrack nat support

 datapath/linux/compat/include/linux/openvswitch.h |   3 +
 include/linux/automake.mk |   3 +-
 include/linux/pkt_cls.h   |  46 +-
 include/linux/tc_act/tc_ct.h  |  41 ++
 include/openvswitch/match.h   |   2 +
 lib/dpif-netdev.c |   1 +
 lib/dpif-netlink.c|  70 ++-
 lib/dpif-provider.h   |   2 +
 lib/dpif.c|   9 +
 lib/dpif.h|   2 +
 lib/match.c   |  10 +-
 lib/netdev-linux.c|   6 +-
 lib/netdev-offload-tc.c   | 600 +++---
 lib/tc.c  | 448 
 lib/tc.h  | 112 +++-
 15 files changed, 1067 insertions(+), 288 deletions(-)
 create mode 100644 include/linux/tc_act/tc_ct.h

-- 
1.8.3.1

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

[ovs-dev] [PATCH v5 09/10] netdev-offload-tc: Add conntrack label and mark support

[Paul Blakey]

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:
V2->V3:
Added missing match on ct_mark.
---
 lib/dpif-netlink.c  |  2 ++
 lib/netdev-offload-tc.c | 66 +
 lib/tc.c| 53 +++
 lib/tc.h|  6 +
 4 files changed, 117 insertions(+), 10 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index cbd459a..f8eab40 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1645,6 +1645,8 @@ dpif_netlink_netdev_match_to_dpif_flow(struct match 
*match,
 .recirc = true,
 .ct_state = true,
 .ct_zone = true,
+.ct_mark = true,
+.ct_label = true,
 },
 };
 size_t offset;
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 6d2bcbe..b2c2c91 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -624,6 +624,8 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 }
 
 match_set_ct_zone_masked(match, key->ct_zone, mask->ct_zone);
+match_set_ct_mark_masked(match, key->ct_mark, mask->ct_mark);
+match_set_ct_label_masked(match, key->ct_label, mask->ct_label);
 }
 
 if (flower->tunnel) {
@@ -793,6 +795,26 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 nl_msg_put_u16(buf, OVS_CT_ATTR_ZONE, action->ct.zone);
 }
 
+if (action->ct.mark_mask) {
+uint32_t mark_and_mask[2] = { action->ct.mark,
+  action->ct.mark_mask };
+nl_msg_put_unspec(buf, OVS_CT_ATTR_MARK, _and_mask,
+  sizeof mark_and_mask);
+}
+
+if (!ovs_u128_is_zero(action->ct.label_mask)) {
+struct {
+ovs_u128 key;
+ovs_u128 mask;
+} *ct_label;
+
+ct_label = nl_msg_put_unspec_uninit(buf,
+OVS_CT_ATTR_LABELS,
+sizeof *ct_label);
+ct_label->key = action->ct.label;
+ct_label->mask = action->ct.label_mask;
+}
+
 nl_msg_end_nested(buf, ct_offset);
 }
 break;
@@ -903,6 +925,28 @@ parse_put_flow_ct_action(struct tc_flower *flower,
 action->ct.zone = nl_attr_get_u16(ct_attr);
 }
 break;
+case OVS_CT_ATTR_MARK: {
+const struct {
+uint32_t key;
+uint32_t mask;
+} *ct_mark;
+
+ct_mark = nl_attr_get_unspec(ct_attr, sizeof *ct_mark);
+action->ct.mark = ct_mark->key;
+action->ct.mark_mask = ct_mark->mask;
+}
+break;
+case OVS_CT_ATTR_LABELS: {
+const struct {
+ovs_u128 key;
+ovs_u128 mask;
+} *ct_label;
+
+ct_label = nl_attr_get_unspec(ct_attr, sizeof *ct_label);
+action->ct.label = ct_label->key;
+action->ct.label_mask = ct_label->mask;
+}
+break;
 }
 }
 
@@ -1093,22 +1137,12 @@ test_key_and_mask(struct match *match)
 return EOPNOTSUPP;
 }
 
-if (mask->ct_mark) {
-VLOG_DBG_RL(, "offloading attribute ct_mark isn't supported");
-return EOPNOTSUPP;
-}
-
 if (mask->packet_type && key->packet_type) {
 VLOG_DBG_RL(, "offloading attribute packet_type isn't supported");
 return EOPNOTSUPP;
 }
 mask->packet_type = 0;
 
-if (!ovs_u128_is_zero(mask->ct_label)) {
-VLOG_DBG_RL(, "offloading attribute ct_label isn't supported");
-return EOPNOTSUPP;
-}
-
 for (int i = 0; i < FLOW_N_REGS; i++) {
 if (mask->regs[i]) {
 VLOG_DBG_RL(,
@@ -1467,6 +1501,18 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 mask->ct_zone = 0;
 }
 
+if (mask->ct_mark) {
+flower.key.ct_mark = key->ct_mark;
+flower.mask.ct_mark = mask->ct_mark;
+mask->ct_mark = 0;
+}
+
+if (!ovs_u128_is_zero(mask->ct_label)) {
+flower.key.ct_label = key->ct_label;
+flower.mask.ct_label = mask->ct_label;
+mask->ct_label = OVS_U128_ZERO;
+}
+
 err = test_key_and_mask(match);
 if (err) {
 return err;
diff --git a/lib/tc.c b/lib/tc.c
index 4358cb7..de09c49 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -404,6 +404,11 @@ static const struct nl_policy tca_flower_policy[] = {
 [TCA_FLOWER_KEY_CT_STATE_MASK] = { .type = NL_A_U16, .optional = true, },
 

[ovs-dev] [PATCH v5 02/10] compat: Add tc ct action and flower matches defines for older kernels

[Paul Blakey]

Update kernel UAPI to support conntrack matches, and the
tc actions ct and goto chain.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
 include/linux/automake.mk|  3 ++-
 include/linux/pkt_cls.h  | 46 +---
 include/linux/tc_act/tc_ct.h | 41 +++
 3 files changed, 86 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/tc_act/tc_ct.h

diff --git a/include/linux/automake.mk b/include/linux/automake.mk
index c759186..8f063f4 100644
--- a/include/linux/automake.mk
+++ b/include/linux/automake.mk
@@ -6,4 +6,5 @@ noinst_HEADERS += \
include/linux/tc_act/tc_pedit.h \
include/linux/tc_act/tc_skbedit.h \
include/linux/tc_act/tc_tunnel_key.h \
-   include/linux/tc_act/tc_vlan.h
+   include/linux/tc_act/tc_vlan.h \
+   include/linux/tc_act/tc_ct.h
diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
index 55f3ef1..b0a5ce8 100644
--- a/include/linux/pkt_cls.h
+++ b/include/linux/pkt_cls.h
@@ -44,7 +44,21 @@ enum {
 #define TC_ACT_QUEUED  5
 #define TC_ACT_REPEAT  6
 #define TC_ACT_REDIRECT7
-#define TC_ACT_JUMP0x1000
+
+/* There is a special kind of actions called "extended actions",
+ * which need a value parameter. These have a local opcode located in
+ * the highest nibble, starting from 1. The rest of the bits
+ * are used to carry the value. These two parts together make
+ * a combined opcode.
+ */
+#define __TC_ACT_EXT_SHIFT 28
+#define __TC_ACT_EXT(local) ((local) << __TC_ACT_EXT_SHIFT)
+#define TC_ACT_EXT_VAL_MASK ((1 << __TC_ACT_EXT_SHIFT) - 1)
+#define TC_ACT_EXT_CMP(combined, opcode) \
+   (((combined) & (~TC_ACT_EXT_VAL_MASK)) == opcode)
+
+#define TC_ACT_JUMP __TC_ACT_EXT(1)
+#define TC_ACT_GOTO_CHAIN __TC_ACT_EXT(2)
 
 struct tc_police {
__u32   index;
@@ -207,16 +221,42 @@ enum {
TCA_FLOWER_KEY_CVLAN_PRIO,  /* u8   */
TCA_FLOWER_KEY_CVLAN_ETH_TYPE,  /* be16 */
 
-   TCA_FLOWER_KEY_ENC_IP_TOS,  /* u8 */
+   TCA_FLOWER_KEY_ENC_IP_TOS,  /* u8 */
TCA_FLOWER_KEY_ENC_IP_TOS_MASK, /* u8 */
-   TCA_FLOWER_KEY_ENC_IP_TTL,  /* u8 */
+   TCA_FLOWER_KEY_ENC_IP_TTL,  /* u8 */
TCA_FLOWER_KEY_ENC_IP_TTL_MASK, /* u8 */
+
TCA_FLOWER_KEY_ENC_OPTS,
TCA_FLOWER_KEY_ENC_OPTS_MASK,
 
+   TCA_FLOWER_IN_HW_COUNT,
+
+   TCA_FLOWER_KEY_PORT_SRC_MIN,/* be16 */
+   TCA_FLOWER_KEY_PORT_SRC_MAX,/* be16 */
+   TCA_FLOWER_KEY_PORT_DST_MIN,/* be16 */
+   TCA_FLOWER_KEY_PORT_DST_MAX,/* be16 */
+
+   TCA_FLOWER_KEY_CT_STATE,/* u16 */
+   TCA_FLOWER_KEY_CT_STATE_MASK,   /* u16 */
+   TCA_FLOWER_KEY_CT_ZONE, /* u16 */
+   TCA_FLOWER_KEY_CT_ZONE_MASK,/* u16 */
+   TCA_FLOWER_KEY_CT_MARK, /* u32 */
+   TCA_FLOWER_KEY_CT_MARK_MASK,/* u32 */
+   TCA_FLOWER_KEY_CT_LABELS,   /* u128 */
+   TCA_FLOWER_KEY_CT_LABELS_MASK,  /* u128 */
+
__TCA_FLOWER_MAX,
 };
 
+#define TCA_FLOWER_MAX (__TCA_FLOWER_MAX - 1)
+
+enum {
+   TCA_FLOWER_KEY_CT_FLAGS_NEW = 1 << 0, /* Beginning of a new connection. 
*/
+   TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED = 1 << 1, /* Part of an existing 
connection. */
+   TCA_FLOWER_KEY_CT_FLAGS_RELATED = 1 << 2, /* Related to an established 
connection. */
+   TCA_FLOWER_KEY_CT_FLAGS_TRACKED = 1 << 3, /* Conntrack has occurred. */
+};
+
 enum {
TCA_FLOWER_KEY_ENC_OPTS_UNSPEC,
TCA_FLOWER_KEY_ENC_OPTS_GENEVE, /* Nested
diff --git a/include/linux/tc_act/tc_ct.h b/include/linux/tc_act/tc_ct.h
new file mode 100644
index 000..5fb1d7a
--- /dev/null
+++ b/include/linux/tc_act/tc_ct.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef __UAPI_TC_CT_H
+#define __UAPI_TC_CT_H
+
+#include 
+#include 
+
+enum {
+   TCA_CT_UNSPEC,
+   TCA_CT_PARMS,
+   TCA_CT_TM,
+   TCA_CT_ACTION,  /* u16 */
+   TCA_CT_ZONE,/* u16 */
+   TCA_CT_MARK,/* u32 */
+   TCA_CT_MARK_MASK,   /* u32 */
+   TCA_CT_LABELS,  /* u128 */
+   TCA_CT_LABELS_MASK, /* u128 */
+   TCA_CT_NAT_IPV4_MIN,/* be32 */
+   TCA_CT_NAT_IPV4_MAX,/* be32 */
+   TCA_CT_NAT_IPV6_MIN,/* struct in6_addr */
+   TCA_CT_NAT_IPV6_MAX,/* struct in6_addr */
+   TCA_CT_NAT_PORT_MIN,/* be16 */
+   TCA_CT_NAT_PORT_MAX,/* be16 */
+   TCA_CT_PAD,
+   __TCA_CT_MAX
+};
+
+#define TCA_CT_MAX (__TCA_CT_MAX - 1)
+
+#define TCA_CT_ACT_COMMIT  (1 << 0)
+#define TCA_CT_ACT_FORCE   (1 << 1)
+#define TCA_CT_ACT_CLEAR   (1 << 2)
+#define TCA_CT_ACT_NAT (1 << 3)
+#define TCA_CT_ACT_NAT_SRC (1 << 4)
+#define TCA_CT_ACT_NAT_DST (1 << 5)
+
+struct tc_ct {
+   tc_gen;
+};
+
+#endif /* __UAPI_TC_CT_H */
-- 
1.8.3.1

[ovs-dev] [PATCH v5 05/10] dpif: Add support to set user features

[Paul Blakey]

This enables user features on the kernel datapath via the DP_CMD_SET
command, and also retrieves them to check for actual support and
not just an older kernel ignoring the requested features.

This will be used in next patch to enable recirc_id sharing with tc.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:

V3->V4:
Removed dpif pointer passing in offload info (not needed, and
compilation issue fix)
V2->V3:
Refactored commit, to move it earlier
Renamed commit from "netdev-offloads-tc: Probe recirc tc sharing feature on 
first recirc_id rule"
---
 datapath/linux/compat/include/linux/openvswitch.h |  3 ++
 lib/dpif-netdev.c |  1 +
 lib/dpif-netlink.c| 52 +--
 lib/dpif-provider.h   |  2 +
 lib/dpif.c|  9 
 lib/dpif.h|  2 +
 6 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 778827f..b9a7faa 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -143,6 +143,9 @@ struct ovs_vport_stats {
 /* Allow datapath to associate multiple Netlink PIDs to each vport */
 #define OVS_DP_F_VPORT_PIDS(1 << 1)
 
+/* Allow tc offload recirc sharing */
+#define OVS_DP_F_TC_RECIRC_SHARING  (1 << 2)
+
 /* Fixed logical ports. */
 #define OVSP_LOCAL  ((__u32)0)
 
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 3f21211..fd8280a 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7627,6 +7627,7 @@ const struct dpif_class dpif_netdev_class = {
 dpif_netdev_run,
 dpif_netdev_wait,
 dpif_netdev_get_stats,
+NULL,  /* set_features */
 dpif_netdev_port_add,
 dpif_netdev_port_del,
 dpif_netdev_port_set_config,
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index e9a6887..ef06dd4 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -193,6 +193,7 @@ struct dpif_handler {
 struct dpif_netlink {
 struct dpif dpif;
 int dp_ifindex;
+uint32_t user_features;
 
 /* Upcall messages. */
 struct fat_rwlock upcall_lock;
@@ -334,15 +335,26 @@ dpif_netlink_open(const struct dpif_class *class 
OVS_UNUSED, const char *name,
 
 /* Create or look up datapath. */
 dpif_netlink_dp_init(_request);
+upcall_pid = 0;
+dp_request.upcall_pid = _pid;
+dp_request.name = name;
+
 if (create) {
 dp_request.cmd = OVS_DP_CMD_NEW;
-upcall_pid = 0;
-dp_request.upcall_pid = _pid;
 } else {
+dp_request.cmd = OVS_DP_CMD_GET;
+
+error = dpif_netlink_dp_transact(_request, , );
+if (error) {
+return error;
+}
+dp_request.user_features = dp.user_features;
+ofpbuf_delete(buf);
+
 /* Use OVS_DP_CMD_SET to report user features */
 dp_request.cmd = OVS_DP_CMD_SET;
 }
-dp_request.name = name;
+
 dp_request.user_features |= OVS_DP_F_UNALIGNED;
 dp_request.user_features |= OVS_DP_F_VPORT_PIDS;
 error = dpif_netlink_dp_transact(_request, , );
@@ -368,6 +380,7 @@ open_dpif(const struct dpif_netlink_dp *dp, struct dpif 
**dpifp)
   dp->dp_ifindex, dp->dp_ifindex);
 
 dpif->dp_ifindex = dp->dp_ifindex;
+dpif->user_features = dp->user_features;
 *dpifp = >dpif;
 
 return 0;
@@ -664,6 +677,31 @@ dpif_netlink_get_stats(const struct dpif *dpif_, struct 
dpif_dp_stats *stats)
 return error;
 }
 
+static int
+dpif_netlink_set_features(struct dpif *dpif_, uint32_t new_features)
+{
+struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
+struct dpif_netlink_dp request, reply;
+struct ofpbuf *bufp;
+int error;
+
+dpif_netlink_dp_init();
+request.cmd = OVS_DP_CMD_SET;
+request.dp_ifindex = dpif->dp_ifindex;
+request.user_features = dpif->user_features | new_features;
+
+error = dpif_netlink_dp_transact(, , );
+if (!error) {
+dpif->user_features = reply.user_features;
+ofpbuf_delete(bufp);
+if (!(dpif->user_features & new_features)) {
+return -EOPNOTSUPP;
+}
+}
+
+return error;
+}
+
 static const char *
 get_vport_type(const struct dpif_netlink_vport *vport)
 {
@@ -3885,6 +3923,7 @@ const struct dpif_class dpif_netlink_class = {
 dpif_netlink_run,
 NULL,   /* wait */
 dpif_netlink_get_stats,
+dpif_netlink_set_features,
 dpif_netlink_port_add,
 dpif_netlink_port_del,
 NULL,   /* port_set_config */
@@ -4202,6 +4241,9 @@ dpif_netlink_dp_from_ofpbuf(struct dpif_netlink_dp *dp, 
const struct ofpbuf *buf
 [OVS_DP_ATTR_MEGAFLOW_STATS] = {
 NL_POLICY_FOR(struct ovs_dp_megaflow_stats),
 .optional = true 

Re: [ovs-dev] [PATCH] netdev-afxdp: Add pcap dump support.

[Ilya Maximets]

On 16.12.2019 15:27, William Tu wrote:
> On Mon, Dec 16, 2019 at 5:57 AM Ilya Maximets  wrote:
>>
>> On 16.12.2019 14:43, William Tu wrote:
>>> On Mon, Dec 16, 2019 at 1:41 AM Ilya Maximets  wrote:

 On 13.12.2019 21:59, William Tu wrote:
> Debugging netdev-afxdp is hard because tcpdump does not work
> at all, even for generic mode.  ovs-tcpdump which uses port
> mirroring also does not work for netdev-afxdp.

 Hmm.  Why ovs-tcpdump doesn't work for you?  It should not depend
 on port type.  If it doesn't work we need to investigate this
 case and fix it because it's a very important tool.
>>>
>>> Because ovs-tcpdump still uses 'tcpdump' tool to capture packets,
>>> (dump-cmd=tcpdump) and forward to mirror port.  Since tcpdump
>>> sees no packet at all when type=afxdp, there is no packets to mirror.
>>
>> ovs-tcpdump creates simple tap interface and mirrors all the traffic
>> to it.  tcpdump command is started on that tap interface (no xdp here).
>> There is no difference from which port types you're mirroring the traffic.
>> It works for DPDK based ports and should work for afxdp.
> 
> I got it working using ovs-tcpdump now, thanks!
> ex: ovs-tcpdump -i afxdp-p0 -n -l -w /tmp/p0.pcap

OK.  So, this patch is not needed.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] system-ovn.at: Fix failing system tests.

[Mark Michelson]

Acked-by: Mark Michelson 

On 12/6/19 10:14 AM, Dumitru Ceara wrote:

Upstream OVS commit:
 commit 03ccfe482ffdc6c1132aeb62467e0fbe6768f25d
 Author: William Tu 
 Date:   Tue Jun 25 14:52:38 2019 -0700

 vswitchd: Separate disable system and route.

 Previously, '--disable-system' disables both system dp and the system
 routing table.  The patch makes '--disable-system' only disable system
 dp and adds '--disable-system-route' for disabling the route table.
 This fixes failures when 'make check-system-userspace' for tunnel cases.

 As a consequence, hitting errors due to OVS userspace parses the IGMP 
packet
 but its datapaths do not, so odp_flow_key_to_flow() return 
ODP_FIT_TOO_LITTLE.
 commit c645550bb249 ("odp-util: Always report ODP_FIT_TOO_LITTLE for 
IGMP.")
 Fix it by filtering out the IGMP-related error message.

 Signed-off-by: William Tu 
 Signed-off-by: Yi-Hung Wei 
 Co-authored-by: Yi-Hung Wei 
 Signed-off-by: Ben Pfaff 

This OVS change was performed in the OVS repo after the OVS-OVN split
and after the OVN tests/system-userspace-*.at files were created by
commit c17862ff04c3b315e5793c9c7fb04a79a892e6f3 and needs to be ported
to OVN too.

CC: Numan Siddique 
Signed-off-by: Dumitru Ceara 
---
  tests/ofproto-macros.at  | 2 +-
  tests/system-userspace-macros.at | 4 +++-
  2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index 1fd546a..6c4ff60 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -196,7 +196,7 @@ m4_define([_OVS_VSWITCHD_START],
  # 'vswitchd-aux-args' provides a way to pass extra command line arguments
  # to ovs-vswitchd
  m4_define([OVS_VSWITCHD_START],
-  [_OVS_VSWITCHD_START([--enable-dummy$3 --disable-system $4])
+  [_OVS_VSWITCHD_START([--enable-dummy$3 --disable-system 
--disable-system-route $4])
 AT_CHECK([add_of_br 0 $1 m4_if([$2], [], [], [| uuidfilt])], [0], [$2])
  ])
  
diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at

index d8cc686..86ed226 100644
--- a/tests/system-userspace-macros.at
+++ b/tests/system-userspace-macros.at
@@ -36,7 +36,9 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START],
  m4_define([OVS_TRAFFIC_VSWITCHD_STOP],
[OVS_VSWITCHD_STOP([dnl
  $1";/netdev_linux.*obtaining netdev stats via vport failed/d
-/dpif_netlink.*Generic Netlink family 'ovs_datapath' does not exist. The Open 
vSwitch kernel module is probably not loaded./d"])
+/dpif_netlink.*Generic Netlink family 'ovs_datapath' does not exist. The Open 
vSwitch kernel module is probably not loaded./d
+/dpif_netdev(revalidator.*)|ERR|internal error parsing flow key.*proto=2.*/d
+/dpif(revalidator.*)|WARN|netdev@ovs-netdev: failed to.*proto=2.*/d"])
 AT_CHECK([:; $2])
])
  



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

[ovs-dev] [PATCH V4 03/17] netdev-offload-dpdk: Dynamically allocate pattern items

[Eli Britstein]

Instead of statically allocated pattern items on the stack, dynamically
allocate only the required items while parsing the matches, to simplify
the parsing and make it self-contained, without need of external types,
making it easier to support more matches in the future.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-offload-dpdk.c | 206 ++
 1 file changed, 118 insertions(+), 88 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 89828224a..a5bc4ad33 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -374,6 +374,24 @@ add_flow_action(struct flow_actions *actions, enum 
rte_flow_action_type type,
 actions->cnt++;
 }
 
+static void
+free_flow_patterns(struct flow_patterns *patterns)
+{
+int i;
+
+for (i = 0; i < patterns->cnt; i++) {
+if (patterns->items[i].spec) {
+free(CONST_CAST(void *, patterns->items[i].spec));
+}
+if (patterns->items[i].mask) {
+free(CONST_CAST(void *, patterns->items[i].mask));
+}
+}
+free(patterns->items);
+patterns->items = NULL;
+patterns->cnt = 0;
+}
+
 static void
 free_flow_actions(struct flow_actions *actions)
 {
@@ -389,39 +407,30 @@ free_flow_actions(struct flow_actions *actions)
 actions->cnt = 0;
 }
 
-struct flow_items {
-struct rte_flow_item_eth  eth;
-struct rte_flow_item_vlan vlan;
-struct rte_flow_item_ipv4 ipv4;
-union {
-struct rte_flow_item_tcp  tcp;
-struct rte_flow_item_udp  udp;
-struct rte_flow_item_sctp sctp;
-struct rte_flow_item_icmp icmp;
-};
-};
-
 static int
 parse_flow_match(struct flow_patterns *patterns,
- struct flow_items *spec,
- struct flow_items *mask,
  const struct match *match)
 {
+uint8_t *next_proto_mask = NULL;
 uint8_t proto = 0;
 
 /* Eth */
 if (!eth_addr_is_zero(match->wc.masks.dl_src) ||
 !eth_addr_is_zero(match->wc.masks.dl_dst)) {
-memcpy(>eth.dst, >flow.dl_dst, sizeof spec->eth.dst);
-memcpy(>eth.src, >flow.dl_src, sizeof spec->eth.src);
-spec->eth.type = match->flow.dl_type;
+struct rte_flow_item_eth *spec, *mask;
+
+spec = xzalloc(sizeof *spec);
+mask = xzalloc(sizeof *mask);
+
+memcpy(>dst, >flow.dl_dst, sizeof spec->dst);
+memcpy(>src, >flow.dl_src, sizeof spec->src);
+spec->type = match->flow.dl_type;
 
-memcpy(>eth.dst, >wc.masks.dl_dst, sizeof mask->eth.dst);
-memcpy(>eth.src, >wc.masks.dl_src, sizeof mask->eth.src);
-mask->eth.type = match->wc.masks.dl_type;
+memcpy(>dst, >wc.masks.dl_dst, sizeof mask->dst);
+memcpy(>src, >wc.masks.dl_src, sizeof mask->src);
+mask->type = match->wc.masks.dl_type;
 
-add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH,
- >eth, >eth);
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, mask);
 } else {
 /*
  * If user specifies a flow (like UDP flow) without L2 patterns,
@@ -435,36 +444,45 @@ parse_flow_match(struct flow_patterns *patterns,
 
 /* VLAN */
 if (match->wc.masks.vlans[0].tci && match->flow.vlans[0].tci) {
-spec->vlan.tci  = match->flow.vlans[0].tci & ~htons(VLAN_CFI);
-mask->vlan.tci  = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI);
+struct rte_flow_item_vlan *spec, *mask;
+
+spec = xzalloc(sizeof *spec);
+mask = xzalloc(sizeof *mask);
+
+spec->tci = match->flow.vlans[0].tci & ~htons(VLAN_CFI);
+mask->tci = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI);
 
 /* Match any protocols. */
-mask->vlan.inner_type = 0;
+mask->inner_type = 0;
 
-add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_VLAN,
- >vlan, >vlan);
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_VLAN, spec, mask);
 }
 
 /* IP v4 */
 if (match->flow.dl_type == htons(ETH_TYPE_IP)) {
-spec->ipv4.hdr.type_of_service = match->flow.nw_tos;
-spec->ipv4.hdr.time_to_live= match->flow.nw_ttl;
-spec->ipv4.hdr.next_proto_id   = match->flow.nw_proto;
-spec->ipv4.hdr.src_addr= match->flow.nw_src;
-spec->ipv4.hdr.dst_addr= match->flow.nw_dst;
+struct rte_flow_item_ipv4 *spec, *mask;
+
+spec = xzalloc(sizeof *spec);
+mask = xzalloc(sizeof *mask);
+
+spec->hdr.type_of_service = match->flow.nw_tos;
+spec->hdr.time_to_live= match->flow.nw_ttl;
+spec->hdr.next_proto_id   = match->flow.nw_proto;
+spec->hdr.src_addr= match->flow.nw_src;
+spec->hdr.dst_addr= match->flow.nw_dst;
 
-mask->ipv4.hdr.type_of_service = match->wc.masks.nw_tos;
-mask->ipv4.hdr.time_to_live= match->wc.masks.nw_ttl;
-mask->ipv4.hdr.next_proto_id   = 

[ovs-dev] [PATCH V4 15/17] netdev-offload-dpdk: Support offload of set MAC actions

[Eli Britstein]

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 Documentation/howto/dpdk.rst |   1 +
 NEWS |   3 +-
 lib/netdev-offload-dpdk.c| 114 +++
 3 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index d9de7bedd..0e9939ddd 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -392,6 +392,7 @@ Supported actions for hardware offload are:
 
 - Output.
 - Drop.
+- Modification of Ethernet (mod_dl_src/mod_dl_dst).
 
 Further Reading
 ---
diff --git a/NEWS b/NEWS
index d019e066f..3ade86d49 100644
--- a/NEWS
+++ b/NEWS
@@ -26,7 +26,8 @@ Post-v2.12.0
  * DPDK ring ports (dpdkr) are deprecated and will be removed in next
releases.
  * Add support for DPDK 19.11.
- * Add hardware offload support for output and drop actions (experimental).
+ * Add hardware offload support for output, drop and set MAC actions
+   (experimental).
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index df7460773..83fb7a2ed 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -367,6 +367,21 @@ dump_flow_action(struct ds *s, const struct 
rte_flow_action *actions)
 }
 } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) {
 ds_put_cstr(s, "rte flow drop action\n");
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ||
+   actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST) {
+const struct rte_flow_action_set_mac *set_mac = actions->conf;
+
+char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST
+   ? "dst" : "src";
+
+ds_put_format(s, "rte flow set-mac-%s action:\n", dirstr);
+if (set_mac) {
+ds_put_format(s,
+  "  Set-mac-%s: "ETH_ADDR_FMT"\n", dirstr,
+  ETH_ADDR_BYTES_ARGS(set_mac->mac_addr));
+} else {
+ds_put_format(s, "  Set-mac-%s = null\n", dirstr);
+}
 } else {
 ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
 }
@@ -794,6 +809,95 @@ add_output_action(struct netdev *netdev,
 return ret;
 }
 
+static int
+add_set_flow_action(struct flow_actions *actions,
+const void *value,
+const void *mask,
+const size_t size,
+const int attr)
+{
+void *spec;
+
+if (mask) {
+/* DPDK does not support partially masked set actions. In such
+ * case, fail the offload.
+ */
+if (is_all_zeros(mask, size)) {
+return 0;
+}
+if (!is_all_ones(mask, size)) {
+VLOG_DBG_RL(_rl,
+"Partial mask is not supported");
+return -1;
+}
+}
+
+spec = xzalloc(size);
+memcpy(spec, value, size);
+add_flow_action(actions, attr, spec);
+
+return 0;
+}
+
+/* Mask is at the midpoint of the data. */
+#define get_mask(a, type) ((const type *)(const void *)(a + 1) + 1)
+
+static int
+parse_set_actions(struct flow_actions *actions,
+  const struct nlattr *set_actions,
+  const size_t set_actions_len,
+  bool masked)
+{
+const struct nlattr *sa;
+unsigned int sleft;
+
+NL_ATTR_FOR_EACH_UNSAFE (sa, sleft, set_actions, set_actions_len) {
+if (nl_attr_type(sa) == OVS_KEY_ATTR_ETHERNET) {
+const struct ovs_key_ethernet *key = nl_attr_get(sa);
+const struct ovs_key_ethernet *mask = masked ?
+get_mask(sa, struct ovs_key_ethernet) : NULL;
+struct ovs_key_ethernet consumed_mask;
+
+if (masked) {
+memcpy(_mask, mask, sizeof consumed_mask);
+} else {
+memset(_mask, 0, sizeof consumed_mask);
+}
+
+BUILD_ASSERT(sizeof(struct rte_flow_action_set_mac) ==
+ sizeof key->eth_src);
+if (add_set_flow_action(actions, >eth_src,
+mask ? >eth_src : NULL,
+sizeof key->eth_src,
+RTE_FLOW_ACTION_TYPE_SET_MAC_SRC)) {
+return -1;
+}
+memset(_mask.eth_src, 0, sizeof consumed_mask.eth_src);
+
+BUILD_ASSERT(sizeof(struct rte_flow_action_set_mac) ==
+ sizeof key->eth_dst);
+if (add_set_flow_action(actions, >eth_dst,
+mask ? >eth_dst : NULL,
+sizeof key->eth_dst,
+RTE_FLOW_ACTION_TYPE_SET_MAC_DST)) {
+return -1;
+}
+memset(_mask.eth_dst, 0, sizeof consumed_mask.eth_dst);
+
+if 

[ovs-dev] [PATCH V4 07/17] netdev-offload-dpdk: Implement flow get method

[Eli Britstein]

Implement the flow get method for DPDK, to get the statistics of the
provided ufid, towards reading statistics of fully offloaded flows.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-offload-dpdk.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 536071ac4..79a7d33fb 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -56,6 +56,7 @@ struct ufid_to_rte_flow_data {
 struct cmap_node node;
 ovs_u128 ufid;
 struct rte_flow *rte_flow;
+struct dpif_flow_stats stats;
 };
 
 /* Find rte_flow with @ufid. */
@@ -885,9 +886,46 @@ netdev_offload_dpdk_init_flow_api(struct netdev *netdev)
 return netdev_dpdk_flow_api_supported(netdev) ? 0 : EOPNOTSUPP;
 }
 
+static int
+netdev_offload_dpdk_flow_get(struct netdev *netdev,
+ struct match *match OVS_UNUSED,
+ struct nlattr **actions OVS_UNUSED,
+ const ovs_u128 *ufid,
+ struct dpif_flow_stats *stats,
+ struct dpif_flow_attrs *attrs OVS_UNUSED,
+ struct ofpbuf *buf OVS_UNUSED)
+{
+struct rte_flow_query_count query = { .reset = 1 };
+struct ufid_to_rte_flow_data *rte_flow_data;
+struct rte_flow_error error;
+int ret = 0;
+
+rte_flow_data = ufid_to_rte_flow_data_find(ufid);
+if (!rte_flow_data || !rte_flow_data->rte_flow) {
+ret = -1;
+goto out;
+}
+
+ret = netdev_dpdk_rte_flow_query_count(netdev, rte_flow_data->rte_flow,
+   , );
+if (ret) {
+goto out;
+}
+rte_flow_data->stats.n_packets += (query.hits_set) ? query.hits : 0;
+rte_flow_data->stats.n_bytes += (query.bytes_set) ? query.bytes : 0;
+if (query.hits_set && query.hits) {
+rte_flow_data->stats.used = time_usec() / 1000;
+}
+memcpy(stats, _flow_data->stats, sizeof *stats);
+
+out:
+return ret;
+}
+
 const struct netdev_flow_api netdev_offload_dpdk = {
 .type = "dpdk_flow_api",
 .flow_put = netdev_offload_dpdk_flow_put,
 .flow_del = netdev_offload_dpdk_flow_del,
 .init_flow_api = netdev_offload_dpdk_init_flow_api,
+.flow_get = netdev_offload_dpdk_flow_get,
 };
-- 
2.14.5

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

[ovs-dev] [PATCH V4 09/17] dpif-netdev: Populate dpif class field in offload struct

[Eli Britstein]

Populate dpif class field in offload struct to be used in offloading
flow put.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/dpif-netdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index be3205a7c..b4fed6416 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2373,6 +2373,7 @@ static int
 dp_netdev_flow_offload_put(struct dp_flow_offload_item *offload)
 {
 struct dp_netdev_pmd_thread *pmd = offload->pmd;
+const struct dpif_class *dpif_class = pmd->dp->class;
 struct dp_netdev_flow *flow = offload->flow;
 odp_port_t in_port = flow->flow.in_port.odp_port;
 bool modification = offload->op == DP_NETDEV_FLOW_OFFLOAD_OP_MOD;
@@ -2410,6 +2411,7 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item 
*offload)
 }
 }
 info.flow_mark = mark;
+info.dpif_class = dpif_class;
 
 port = netdev_ports_get(in_port, pmd->dp->class);
 if (!port || netdev_vport_is_vport_class(port->netdev_class)) {
-- 
2.14.5

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

[ovs-dev] [PATCH V4 08/17] dpif-netdev: Update offloaded flows statistics

[Eli Britstein]

From: Ophir Munk 

In case a flow is HW offloaded, packets do not reach the SW, thus not
counted for statistics. Use netdev flow get API in order to update the
statistics of flows by the HW statistics.

Co-authored-by: Eli Britstein 
Signed-off-by: Ophir Munk 
Reviewed-by: Oz Shlomo 
Signed-off-by: Eli Britstein 
---
 lib/dpif-netdev.c | 81 ++-
 1 file changed, 68 insertions(+), 13 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 7ebf4ef87..be3205a7c 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -530,6 +530,7 @@ struct dp_netdev_flow {
 
 /* Statistics. */
 struct dp_netdev_flow_stats stats;
+struct dp_netdev_flow_stats hw_stats;
 
 /* Actions. */
 OVSRCU_TYPE(struct dp_netdev_actions *) actions;
@@ -3002,8 +3003,9 @@ dp_netdev_pmd_find_flow(const struct dp_netdev_pmd_thread 
*pmd,
 
 static void
 get_dpif_flow_stats(const struct dp_netdev_flow *netdev_flow_,
-struct dpif_flow_stats *stats)
+struct dpif_flow_stats *stats, bool hw)
 {
+struct dp_netdev_flow_stats *flow_stats;
 struct dp_netdev_flow *netdev_flow;
 unsigned long long n;
 long long used;
@@ -3011,13 +3013,15 @@ get_dpif_flow_stats(const struct dp_netdev_flow 
*netdev_flow_,
 
 netdev_flow = CONST_CAST(struct dp_netdev_flow *, netdev_flow_);
 
-atomic_read_relaxed(_flow->stats.packet_count, );
+flow_stats = (hw) ? _flow->hw_stats : _flow->stats;
+
+atomic_read_relaxed(_stats->packet_count, );
 stats->n_packets = n;
-atomic_read_relaxed(_flow->stats.byte_count, );
+atomic_read_relaxed(_stats->byte_count, );
 stats->n_bytes = n;
-atomic_read_relaxed(_flow->stats.used, );
+atomic_read_relaxed(_stats->used, );
 stats->used = used;
-atomic_read_relaxed(_flow->stats.tcp_flags, );
+atomic_read_relaxed(_stats->tcp_flags, );
 stats->tcp_flags = flags;
 }
 
@@ -3028,8 +3032,10 @@ get_dpif_flow_stats(const struct dp_netdev_flow 
*netdev_flow_,
 static void
 dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow *netdev_flow,
 struct ofpbuf *key_buf, struct ofpbuf *mask_buf,
-struct dpif_flow *flow, bool terse)
+struct dpif_flow *flow, bool terse, bool offloaded)
 {
+struct dpif_flow_stats hw_stats;
+
 if (terse) {
 memset(flow, 0, sizeof *flow);
 } else {
@@ -3069,10 +3075,14 @@ dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow 
*netdev_flow,
 flow->ufid = netdev_flow->ufid;
 flow->ufid_present = true;
 flow->pmd_id = netdev_flow->pmd_id;
-get_dpif_flow_stats(netdev_flow, >stats);
+get_dpif_flow_stats(netdev_flow, >stats, false);
+get_dpif_flow_stats(netdev_flow, _stats, true);
+flow->stats.n_packets += hw_stats.n_packets;
+flow->stats.n_bytes += hw_stats.n_bytes;
+flow->stats.used = MAX(flow->stats.used, hw_stats.used);
 
-flow->attrs.offloaded = false;
-flow->attrs.dp_layer = "ovs";
+flow->attrs.offloaded = offloaded;
+flow->attrs.dp_layer = flow->attrs.offloaded ? "in_hw" : "ovs";
 }
 
 static int
@@ -3142,6 +3152,44 @@ dpif_netdev_flow_from_nlattrs(const struct nlattr *key, 
uint32_t key_len,
 return 0;
 }
 
+static bool
+dpif_netdev_offload_stats(struct dp_netdev_flow *netdev_flow,
+  struct dp_netdev *dp)
+{
+struct dpif_flow_stats stats;
+struct dpif_flow_attrs attrs;
+struct nlattr *actions;
+struct ofpbuf wbuffer;
+struct netdev *netdev;
+struct match match;
+
+int ret = 0;
+
+if (!netdev_is_flow_api_enabled()) {
+return false;
+}
+
+netdev = netdev_ports_get(netdev_flow->flow.in_port.odp_port, dp->class);
+if (!netdev) {
+return false;
+}
+/* Taking a global 'port_mutex' to fulfill thread safety
+ * restrictions for the netdev-offload-dpdk module. */
+ovs_mutex_lock(>port_mutex);
+ret = netdev_flow_get(netdev, , , _flow->mega_ufid,
+  , , );
+ovs_mutex_unlock(>port_mutex);
+netdev_close(netdev);
+if (ret) {
+return false;
+}
+atomic_store_relaxed(_flow->hw_stats.packet_count, stats.n_packets);
+atomic_store_relaxed(_flow->hw_stats.byte_count, stats.n_bytes);
+atomic_store_relaxed(_flow->hw_stats.used, stats.used);
+
+return true;
+}
+
 static int
 dpif_netdev_flow_get(const struct dpif *dpif, const struct dpif_flow_get *get)
 {
@@ -3175,8 +3223,9 @@ dpif_netdev_flow_get(const struct dpif *dpif, const 
struct dpif_flow_get *get)
 netdev_flow = dp_netdev_pmd_find_flow(pmd, get->ufid, get->key,
   get->key_len);
 if (netdev_flow) {
+bool offloaded = dpif_netdev_offload_stats(netdev_flow, pmd->dp);
 dp_netdev_flow_to_dpif_flow(netdev_flow, get->buffer, get->buffer,
-get->flow, false);

[ovs-dev] [PATCH V4 02/17] netdev-offload-dpdk: Refactor action items freeing scheme

[Eli Britstein]

Action item data structures are pointed by rte_flow_action items.
Refactor the code to free the data structures when freeing the
rte_flow_action items, allowing simpler future actions simpler to add to
the code.

Signed-off-by: Eli Britstein 
---
 lib/netdev-offload-dpdk.c | 92 ++-
 1 file changed, 52 insertions(+), 40 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 71da12aab..89828224a 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -374,40 +374,19 @@ add_flow_action(struct flow_actions *actions, enum 
rte_flow_action_type type,
 actions->cnt++;
 }
 
-struct action_rss_data {
-struct rte_flow_action_rss conf;
-uint16_t queue[0];
-};
-
-static struct action_rss_data *
-add_flow_rss_action(struct flow_actions *actions,
-struct netdev *netdev)
+static void
+free_flow_actions(struct flow_actions *actions)
 {
 int i;
-struct action_rss_data *rss_data;
 
-rss_data = xmalloc(sizeof *rss_data +
-   netdev_n_rxq(netdev) * sizeof rss_data->queue[0]);
-*rss_data = (struct action_rss_data) {
-.conf = (struct rte_flow_action_rss) {
-.func = RTE_ETH_HASH_FUNCTION_DEFAULT,
-.level = 0,
-.types = 0,
-.queue_num = netdev_n_rxq(netdev),
-.queue = rss_data->queue,
-.key_len = 0,
-.key  = NULL
-},
-};
-
-/* Override queue array with default. */
-for (i = 0; i < netdev_n_rxq(netdev); i++) {
-   rss_data->queue[i] = i;
+for (i = 0; i < actions->cnt; i++) {
+if (actions->actions[i].conf) {
+free(CONST_CAST(void *, actions->actions[i].conf));
+}
 }
-
-add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, _data->conf);
-
-return rss_data;
+free(actions->actions);
+actions->actions = NULL;
+actions->cnt = 0;
 }
 
 struct flow_items {
@@ -569,6 +548,47 @@ parse_flow_match(struct flow_patterns *patterns,
 return 0;
 }
 
+static void
+add_flow_mark_rss_actions(struct flow_actions *actions,
+  uint32_t flow_mark,
+  const struct netdev *netdev)
+{
+struct rte_flow_action_mark *mark;
+struct action_rss_data {
+struct rte_flow_action_rss conf;
+uint16_t queue[0];
+} *rss_data;
+BUILD_ASSERT_DECL(offsetof(struct action_rss_data, conf) == 0);
+int i;
+
+mark = xzalloc(sizeof *mark);
+
+mark->id = flow_mark;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_MARK, mark);
+
+rss_data = xmalloc(sizeof *rss_data +
+   netdev_n_rxq(netdev) * sizeof rss_data->queue[0]);
+*rss_data = (struct action_rss_data) {
+.conf = (struct rte_flow_action_rss) {
+.func = RTE_ETH_HASH_FUNCTION_DEFAULT,
+.level = 0,
+.types = 0,
+.queue_num = netdev_n_rxq(netdev),
+.queue = rss_data->queue,
+.key_len = 0,
+.key  = NULL
+},
+};
+
+/* Override queue array with default. */
+for (i = 0; i < netdev_n_rxq(netdev); i++) {
+   rss_data->queue[i] = i;
+}
+
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, _data->conf);
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
+}
+
 static int
 netdev_offload_dpdk_add_flow(struct netdev *netdev,
  const struct match *match,
@@ -598,20 +618,12 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
 goto out;
 }
 
-struct rte_flow_action_mark mark;
-struct action_rss_data *rss;
-
-mark.id = info->flow_mark;
-add_flow_action(, RTE_FLOW_ACTION_TYPE_MARK, );
-
-rss = add_flow_rss_action(, netdev);
-add_flow_action(, RTE_FLOW_ACTION_TYPE_END, NULL);
+add_flow_mark_rss_actions(, info->flow_mark, netdev);
 
 flow = netdev_dpdk_rte_flow_create(netdev, _attr,
patterns.items,
actions.actions, );
 
-free(rss);
 if (!flow) {
 VLOG_ERR("%s: rte flow creat error: %u : message : %s\n",
  netdev_get_name(netdev), error.type, error.message);
@@ -624,7 +636,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
 
 out:
 free(patterns.items);
-free(actions.actions);
+free_flow_actions();
 return ret;
 }
 
-- 
2.14.5

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

[ovs-dev] [PATCH V4 16/17] netdev-offload-dpdk: Support offload of set IPv4 actions

[Eli Britstein]

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 Documentation/howto/dpdk.rst |  1 +
 NEWS |  4 +--
 lib/netdev-offload-dpdk.c| 69 
 3 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 0e9939ddd..c440bf28e 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -393,6 +393,7 @@ Supported actions for hardware offload are:
 - Output.
 - Drop.
 - Modification of Ethernet (mod_dl_src/mod_dl_dst).
+- Modification of IPv4 (mod_nw_src/mod_nw_dst/mod_nw_ttl).
 
 Further Reading
 ---
diff --git a/NEWS b/NEWS
index 3ade86d49..297ca6fff 100644
--- a/NEWS
+++ b/NEWS
@@ -26,8 +26,8 @@ Post-v2.12.0
  * DPDK ring ports (dpdkr) are deprecated and will be removed in next
releases.
  * Add support for DPDK 19.11.
- * Add hardware offload support for output, drop and set MAC actions
-   (experimental).
+ * Add hardware offload support for output, drop and set actions of
+   MAC and IPv4 (experimental).
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 83fb7a2ed..0f0685f4d 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -382,6 +382,29 @@ dump_flow_action(struct ds *s, const struct 
rte_flow_action *actions)
 } else {
 ds_put_format(s, "  Set-mac-%s = null\n", dirstr);
 }
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ||
+   actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_DST) {
+const struct rte_flow_action_set_ipv4 *set_ipv4 = actions->conf;
+char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_DST
+   ? "dst" : "src";
+
+ds_put_format(s, "rte flow set-ipv4-%s action:\n", dirstr);
+if (set_ipv4) {
+ds_put_format(s,
+  "  Set-ipv4-%s: "IP_FMT"\n", dirstr,
+  IP_ARGS(set_ipv4->ipv4_addr));
+} else {
+ds_put_format(s, "  Set-ipv4-%s = null\n", dirstr);
+}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_TTL) {
+const struct rte_flow_action_set_ttl *set_ttl = actions->conf;
+
+ds_put_cstr(s, "rte flow set-ttl action:\n");
+if (set_ttl) {
+ds_put_format(s, "  Set-ttl: %d\n", set_ttl->ttl_value);
+} else {
+ds_put_cstr(s, "  Set-ttl = null\n");
+}
 } else {
 ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
 }
@@ -888,6 +911,52 @@ parse_set_actions(struct flow_actions *actions,
 VLOG_DBG_RL(_rl, "Unsupported ETHERNET set action");
 return -1;
 }
+} else if (nl_attr_type(sa) == OVS_KEY_ATTR_IPV4) {
+const struct ovs_key_ipv4 *key = nl_attr_get(sa);
+const struct ovs_key_ipv4 *mask = masked ?
+get_mask(sa, struct ovs_key_ipv4) : NULL;
+struct ovs_key_ipv4 consumed_mask;
+
+if (masked) {
+memcpy(_mask, mask, sizeof consumed_mask);
+} else {
+memset(_mask, 0, sizeof consumed_mask);
+}
+
+BUILD_ASSERT(sizeof(struct rte_flow_action_set_ipv4) ==
+ sizeof key->ipv4_src);
+if (add_set_flow_action(actions, >ipv4_src,
+mask ? >ipv4_src : NULL,
+sizeof key->ipv4_src,
+RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC)) {
+return -1;
+}
+memset(_mask.ipv4_src, 0, sizeof consumed_mask.ipv4_src);
+
+BUILD_ASSERT(sizeof(struct rte_flow_action_set_ipv4) ==
+ sizeof key->ipv4_dst);
+if (add_set_flow_action(actions, >ipv4_dst,
+mask ? >ipv4_dst : NULL,
+sizeof key->ipv4_dst,
+RTE_FLOW_ACTION_TYPE_SET_IPV4_DST)) {
+return -1;
+}
+memset(_mask.ipv4_dst, 0, sizeof consumed_mask.ipv4_dst);
+
+BUILD_ASSERT(sizeof(struct rte_flow_action_set_ttl) ==
+ sizeof key->ipv4_ttl);
+if (add_set_flow_action(actions, >ipv4_ttl,
+mask ? >ipv4_ttl : NULL,
+sizeof key->ipv4_ttl,
+RTE_FLOW_ACTION_TYPE_SET_TTL)) {
+return -1;
+}
+memset(_mask.ipv4_ttl, 0, sizeof consumed_mask.ipv4_ttl);
+
+if (!is_all_zeros(_mask, sizeof consumed_mask)) {
+VLOG_DBG_RL(_rl, "Unsupported IPv4 set action");
+return -1;
+}
 } else {
 VLOG_DBG_RL(_rl,
  

[ovs-dev] [PATCH V4 14/17] netdev-offload-dpdk: Support offload of drop action

[Eli Britstein]

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 Documentation/howto/dpdk.rst | 1 +
 NEWS | 2 +-
 lib/netdev-offload-dpdk.c| 6 +++---
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index f0fe2bef7..d9de7bedd 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -391,6 +391,7 @@ Supported protocols for hardware offload matches are:
 Supported actions for hardware offload are:
 
 - Output.
+- Drop.
 
 Further Reading
 ---
diff --git a/NEWS b/NEWS
index c430999a0..d019e066f 100644
--- a/NEWS
+++ b/NEWS
@@ -26,7 +26,7 @@ Post-v2.12.0
  * DPDK ring ports (dpdkr) are deprecated and will be removed in next
releases.
  * Add support for DPDK 19.11.
- * Add hardware offload support for output actions (experimental).
+ * Add hardware offload support for output and drop actions (experimental).
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 2be36731a..df7460773 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -365,6 +365,8 @@ dump_flow_action(struct ds *s, const struct rte_flow_action 
*actions)
 } else {
 ds_put_cstr(s, "  Port-id = null\n");
 }
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) {
+ds_put_cstr(s, "rte flow drop action\n");
 } else {
 ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
 }
@@ -816,9 +818,7 @@ parse_flow_actions(struct netdev *netdev,
 }
 
 if (nl_actions_len == 0) {
-VLOG_DBG_RL(_rl,
-"Unsupported action type drop");
-return -1;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_DROP, NULL);
 }
 
 add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
-- 
2.14.5

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

[ovs-dev] [PATCH V4 12/17] netdev-offload-dpdk: Framework for actions offload

[Eli Britstein]

Currently HW offload is accelerating only the rule matching sequence.
Introduce a framework for offloading rule actions as a pre-step for
processing the rule actions in HW. In case of a failure, fallback to the
legacy partial offload scheme.

Note: a flow will be fully offloaded only if it can process all its
actions in HW.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-offload-dpdk.c | 104 --
 1 file changed, 91 insertions(+), 13 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 79a7d33fb..a60d68d2c 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -691,6 +691,89 @@ add_flow_mark_rss_actions(struct flow_actions *actions,
 add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
 }
 
+static struct rte_flow *
+netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
+ struct netdev *netdev,
+ uint32_t flow_mark)
+{
+struct flow_actions actions = { .actions = NULL, .cnt = 0 };
+const struct rte_flow_attr flow_attr = {
+.group = 0,
+.priority = 0,
+.ingress = 1,
+.egress = 0
+};
+struct rte_flow_error error;
+struct rte_flow *flow;
+
+add_flow_mark_rss_actions(, flow_mark, netdev);
+
+flow = netdev_offload_dpdk_flow_create(netdev, _attr, patterns->items,
+   actions.actions, );
+
+if (!flow) {
+VLOG_ERR("%s: Failed to create flow: %s (%u)\n",
+ netdev_get_name(netdev), error.message, error.type);
+}
+
+free_flow_actions();
+return flow;
+}
+
+static int
+parse_flow_actions(struct netdev *netdev OVS_UNUSED,
+   struct flow_actions *actions,
+   struct nlattr *nl_actions,
+   size_t nl_actions_len,
+   struct offload_info *info OVS_UNUSED)
+{
+struct nlattr *nla;
+size_t left;
+
+NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
+VLOG_DBG_RL(_rl,
+"Unsupported action type %d", nl_attr_type(nla));
+return -1;
+}
+
+if (nl_actions_len == 0) {
+VLOG_DBG_RL(_rl,
+"Unsupported action type drop");
+return -1;
+}
+
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
+return 0;
+}
+
+static struct rte_flow *
+netdev_offload_dpdk_actions(struct netdev *netdev,
+struct flow_patterns *patterns,
+struct nlattr *nl_actions,
+size_t actions_len,
+struct offload_info *info)
+{
+const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1 };
+struct flow_actions actions = { .actions = NULL, .cnt = 0 };
+struct rte_flow *flow = NULL;
+struct rte_flow_error error;
+int ret;
+
+ret = parse_flow_actions(netdev, , nl_actions, actions_len, info);
+if (ret) {
+goto out;
+}
+flow = netdev_offload_dpdk_flow_create(netdev, _attr, patterns->items,
+   actions.actions, );
+if (!flow) {
+VLOG_ERR("%s: Failed to create flow: %s (%u)\n",
+ netdev_get_name(netdev), error.message, error.type);
+}
+out:
+free_flow_actions();
+return flow;
+}
+
 static int
 netdev_offload_dpdk_add_flow(struct netdev *netdev,
  const struct match *match,
@@ -699,16 +782,8 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
  const ovs_u128 *ufid,
  struct offload_info *info)
 {
-const struct rte_flow_attr flow_attr = {
-.group = 0,
-.priority = 0,
-.ingress = 1,
-.egress = 0
-};
 struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
-struct flow_actions actions = { .actions = NULL, .cnt = 0 };
 struct rte_flow *flow;
-struct rte_flow_error error;
 int ret = 0;
 
 ret = parse_flow_match(, match);
@@ -716,10 +791,14 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
 goto out;
 }
 
-add_flow_mark_rss_actions(, info->flow_mark, netdev);
-
-flow = netdev_offload_dpdk_flow_create(netdev, _attr, patterns.items,
-   actions.actions, );
+flow = netdev_offload_dpdk_actions(netdev, , nl_actions,
+   actions_len, info);
+if (!flow) {
+/* if we failed to offload the rule actions fallback to mark rss
+ * actions.
+ */
+flow = netdev_offload_dpdk_mark_rss(, netdev, 
info->flow_mark);
+}
 
 if (!flow) {
 ret = -1;
@@ -731,7 +810,6 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
 
 out:
 free_flow_patterns();
-free_flow_actions();
 return ret;
 }
 
-- 
2.14.5

[ovs-dev] [PATCH V4 04/17] netdev-offload-dpdk: Improve HW offload flow debuggability

[Eli Britstein]

Add debug prints when creating and destroying rte flows, with all the
flow details (attributes, patterns, actions).

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-offload-dpdk.c | 200 ++
 1 file changed, 130 insertions(+), 70 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index a5bc4ad33..7cb24ddcd 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -28,6 +28,7 @@
 #include "uuid.h"
 
 VLOG_DEFINE_THIS_MODULE(netdev_offload_dpdk);
+static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(100, 5);
 
 /* Thread-safety
  * =
@@ -132,72 +133,70 @@ struct flow_actions {
 };
 
 static void
-dump_flow_pattern(struct rte_flow_item *item)
+dump_flow_attr(struct ds *s, const struct rte_flow_attr *attr)
 {
-struct ds s;
-
-if (!VLOG_IS_DBG_ENABLED() || item->type == RTE_FLOW_ITEM_TYPE_END) {
-return;
-}
-
-ds_init();
+ds_put_format(s,
+  "  Attributes: "
+  "ingress=%d, egress=%d, prio=%d, group=%d, transfer=%d\n",
+  attr->ingress, attr->egress, attr->priority, attr->group,
+  attr->transfer);
+}
 
+static void
+dump_flow_pattern(struct ds *s, const struct rte_flow_item *item)
+{
 if (item->type == RTE_FLOW_ITEM_TYPE_ETH) {
 const struct rte_flow_item_eth *eth_spec = item->spec;
 const struct rte_flow_item_eth *eth_mask = item->mask;
 
-ds_put_cstr(, "rte flow eth pattern:\n");
+ds_put_cstr(s, "rte flow eth pattern:\n");
 if (eth_spec) {
-ds_put_format(,
+ds_put_format(s,
   "  Spec: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
   "type=0x%04" PRIx16"\n",
   ETH_ADDR_BYTES_ARGS(eth_spec->src.addr_bytes),
   ETH_ADDR_BYTES_ARGS(eth_spec->dst.addr_bytes),
   ntohs(eth_spec->type));
 } else {
-ds_put_cstr(, "  Spec = null\n");
+ds_put_cstr(s, "  Spec = null\n");
 }
 if (eth_mask) {
-ds_put_format(,
+ds_put_format(s,
   "  Mask: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
   "type=0x%04"PRIx16"\n",
   ETH_ADDR_BYTES_ARGS(eth_mask->src.addr_bytes),
   ETH_ADDR_BYTES_ARGS(eth_mask->dst.addr_bytes),
   ntohs(eth_mask->type));
 } else {
-ds_put_cstr(, "  Mask = null\n");
+ds_put_cstr(s, "  Mask = null\n");
 }
-}
-
-if (item->type == RTE_FLOW_ITEM_TYPE_VLAN) {
+} else if (item->type == RTE_FLOW_ITEM_TYPE_VLAN) {
 const struct rte_flow_item_vlan *vlan_spec = item->spec;
 const struct rte_flow_item_vlan *vlan_mask = item->mask;
 
-ds_put_cstr(, "rte flow vlan pattern:\n");
+ds_put_cstr(s, "rte flow vlan pattern:\n");
 if (vlan_spec) {
-ds_put_format(,
+ds_put_format(s,
   "  Spec: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n",
   ntohs(vlan_spec->inner_type), ntohs(vlan_spec->tci));
 } else {
-ds_put_cstr(, "  Spec = null\n");
+ds_put_cstr(s, "  Spec = null\n");
 }
 
 if (vlan_mask) {
-ds_put_format(,
+ds_put_format(s,
   "  Mask: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n",
   ntohs(vlan_mask->inner_type), ntohs(vlan_mask->tci));
 } else {
-ds_put_cstr(, "  Mask = null\n");
+ds_put_cstr(s, "  Mask = null\n");
 }
-}
-
-if (item->type == RTE_FLOW_ITEM_TYPE_IPV4) {
+} else if (item->type == RTE_FLOW_ITEM_TYPE_IPV4) {
 const struct rte_flow_item_ipv4 *ipv4_spec = item->spec;
 const struct rte_flow_item_ipv4 *ipv4_mask = item->mask;
 
-ds_put_cstr(, "rte flow ipv4 pattern:\n");
+ds_put_cstr(s, "rte flow ipv4 pattern:\n");
 if (ipv4_spec) {
-ds_put_format(,
+ds_put_format(s,
   "  Spec: tos=0x%"PRIx8", ttl=%"PRIx8
   ", proto=0x%"PRIx8
   ", src="IP_FMT", dst="IP_FMT"\n",
@@ -207,10 +206,10 @@ dump_flow_pattern(struct rte_flow_item *item)
   IP_ARGS(ipv4_spec->hdr.src_addr),
   IP_ARGS(ipv4_spec->hdr.dst_addr));
 } else {
-ds_put_cstr(, "  Spec = null\n");
+ds_put_cstr(s, "  Spec = null\n");
 }
 if (ipv4_mask) {
-ds_put_format(,
+ds_put_format(s,
   "  Mask: tos=0x%"PRIx8", ttl=%"PRIx8
   ", proto=0x%"PRIx8
   ", src="IP_FMT", dst="IP_FMT"\n",
@@ -220,89 +219,81 @@ 

[ovs-dev] [PATCH V4 01/17] netdev-offload-dpdk: Refactor flow patterns

[Eli Britstein]

Refactor the flow patterns code to a helper function for better
readability and towards supporting more matches.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-offload-dpdk.c | 207 +-
 1 file changed, 111 insertions(+), 96 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 96794dc4d..71da12aab 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -410,54 +410,39 @@ add_flow_rss_action(struct flow_actions *actions,
 return rss_data;
 }
 
+struct flow_items {
+struct rte_flow_item_eth  eth;
+struct rte_flow_item_vlan vlan;
+struct rte_flow_item_ipv4 ipv4;
+union {
+struct rte_flow_item_tcp  tcp;
+struct rte_flow_item_udp  udp;
+struct rte_flow_item_sctp sctp;
+struct rte_flow_item_icmp icmp;
+};
+};
+
 static int
-netdev_offload_dpdk_add_flow(struct netdev *netdev,
- const struct match *match,
- struct nlattr *nl_actions OVS_UNUSED,
- size_t actions_len OVS_UNUSED,
- const ovs_u128 *ufid,
- struct offload_info *info)
+parse_flow_match(struct flow_patterns *patterns,
+ struct flow_items *spec,
+ struct flow_items *mask,
+ const struct match *match)
 {
-const struct rte_flow_attr flow_attr = {
-.group = 0,
-.priority = 0,
-.ingress = 1,
-.egress = 0
-};
-struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
-struct flow_actions actions = { .actions = NULL, .cnt = 0 };
-struct rte_flow *flow;
-struct rte_flow_error error;
 uint8_t proto = 0;
-int ret = 0;
-struct flow_items {
-struct rte_flow_item_eth  eth;
-struct rte_flow_item_vlan vlan;
-struct rte_flow_item_ipv4 ipv4;
-union {
-struct rte_flow_item_tcp  tcp;
-struct rte_flow_item_udp  udp;
-struct rte_flow_item_sctp sctp;
-struct rte_flow_item_icmp icmp;
-};
-} spec, mask;
-
-memset(, 0, sizeof spec);
-memset(, 0, sizeof mask);
 
 /* Eth */
 if (!eth_addr_is_zero(match->wc.masks.dl_src) ||
 !eth_addr_is_zero(match->wc.masks.dl_dst)) {
-memcpy(, >flow.dl_dst, sizeof spec.eth.dst);
-memcpy(, >flow.dl_src, sizeof spec.eth.src);
-spec.eth.type = match->flow.dl_type;
+memcpy(>eth.dst, >flow.dl_dst, sizeof spec->eth.dst);
+memcpy(>eth.src, >flow.dl_src, sizeof spec->eth.src);
+spec->eth.type = match->flow.dl_type;
 
-memcpy(, >wc.masks.dl_dst, sizeof mask.eth.dst);
-memcpy(, >wc.masks.dl_src, sizeof mask.eth.src);
-mask.eth.type = match->wc.masks.dl_type;
+memcpy(>eth.dst, >wc.masks.dl_dst, sizeof mask->eth.dst);
+memcpy(>eth.src, >wc.masks.dl_src, sizeof mask->eth.src);
+mask->eth.type = match->wc.masks.dl_type;
 
-add_flow_pattern(, RTE_FLOW_ITEM_TYPE_ETH,
- , );
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH,
+ >eth, >eth);
 } else {
 /*
  * If user specifies a flow (like UDP flow) without L2 patterns,
@@ -466,41 +451,41 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
  * NIC (such as XL710) doesn't support that. Below is a workaround,
  * which simply matches any L2 pkts.
  */
-add_flow_pattern(, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);
 }
 
 /* VLAN */
 if (match->wc.masks.vlans[0].tci && match->flow.vlans[0].tci) {
-spec.vlan.tci  = match->flow.vlans[0].tci & ~htons(VLAN_CFI);
-mask.vlan.tci  = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI);
+spec->vlan.tci  = match->flow.vlans[0].tci & ~htons(VLAN_CFI);
+mask->vlan.tci  = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI);
 
 /* Match any protocols. */
-mask.vlan.inner_type = 0;
+mask->vlan.inner_type = 0;
 
-add_flow_pattern(, RTE_FLOW_ITEM_TYPE_VLAN,
- , );
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_VLAN,
+ >vlan, >vlan);
 }
 
 /* IP v4 */
 if (match->flow.dl_type == htons(ETH_TYPE_IP)) {
-spec.ipv4.hdr.type_of_service = match->flow.nw_tos;
-spec.ipv4.hdr.time_to_live= match->flow.nw_ttl;
-spec.ipv4.hdr.next_proto_id   = match->flow.nw_proto;
-spec.ipv4.hdr.src_addr= match->flow.nw_src;
-spec.ipv4.hdr.dst_addr= match->flow.nw_dst;
+spec->ipv4.hdr.type_of_service = match->flow.nw_tos;
+spec->ipv4.hdr.time_to_live= match->flow.nw_ttl;
+spec->ipv4.hdr.next_proto_id   = match->flow.nw_proto;
+spec->ipv4.hdr.src_addr= 

[ovs-dev] [PATCH V4 06/17] netdev-offload-dpdk: Return UFID-rte_flow entry in find method

[Eli Britstein]

Change the find method to return the whole entry of UFID-rte_flow
association instead of only the rte_flow field in it, as a pre-step
towards adding and using more fields into that map entry.

Signed-off-by: Eli Britstein 
---
 lib/netdev-offload-dpdk.c | 29 ++---
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 7cb24ddcd..536071ac4 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -59,15 +59,15 @@ struct ufid_to_rte_flow_data {
 };
 
 /* Find rte_flow with @ufid. */
-static struct rte_flow *
-ufid_to_rte_flow_find(const ovs_u128 *ufid)
+static struct ufid_to_rte_flow_data *
+ufid_to_rte_flow_data_find(const ovs_u128 *ufid)
 {
 size_t hash = hash_bytes(ufid, sizeof *ufid, 0);
 struct ufid_to_rte_flow_data *data;
 
 CMAP_FOR_EACH_WITH_HASH (data, node, hash, _to_rte_flow) {
 if (ovs_u128_equals(*ufid, data->ufid)) {
-return data->rte_flow;
+return data;
 }
 }
 
@@ -80,6 +80,7 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid,
 {
 size_t hash = hash_bytes(ufid, sizeof *ufid, 0);
 struct ufid_to_rte_flow_data *data = xzalloc(sizeof *data);
+struct ufid_to_rte_flow_data *data_prev;
 
 /*
  * We should not simply overwrite an existing rte flow.
@@ -87,7 +88,10 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid,
  * Thus, if following assert triggers, something is wrong:
  * the rte_flow is not destroyed.
  */
-ovs_assert(ufid_to_rte_flow_find(ufid) == NULL);
+data_prev = ufid_to_rte_flow_data_find(ufid);
+if (data_prev) {
+ovs_assert(data_prev->rte_flow == NULL);
+}
 
 data->ufid = *ufid;
 data->rte_flow = rte_flow;
@@ -829,16 +833,17 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, 
struct match *match,
  const ovs_u128 *ufid, struct offload_info *info,
  struct dpif_flow_stats *stats)
 {
-struct rte_flow *rte_flow;
+struct ufid_to_rte_flow_data *rte_flow_data;
 int ret;
 
 /*
  * If an old rte_flow exists, it means it's a flow modification.
  * Here destroy the old rte flow first before adding a new one.
  */
-rte_flow = ufid_to_rte_flow_find(ufid);
-if (rte_flow) {
-ret = netdev_offload_dpdk_destroy_flow(netdev, ufid, rte_flow);
+rte_flow_data = ufid_to_rte_flow_data_find(ufid);
+if (rte_flow_data && rte_flow_data->rte_flow) {
+ret = netdev_offload_dpdk_destroy_flow(netdev, ufid,
+   rte_flow_data->rte_flow);
 if (ret < 0) {
 return ret;
 }
@@ -860,16 +865,18 @@ static int
 netdev_offload_dpdk_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
  struct dpif_flow_stats *stats)
 {
-struct rte_flow *rte_flow = ufid_to_rte_flow_find(ufid);
+struct ufid_to_rte_flow_data *rte_flow_data;
 
-if (!rte_flow) {
+rte_flow_data = ufid_to_rte_flow_data_find(ufid);
+if (!rte_flow_data || !rte_flow_data->rte_flow) {
 return -1;
 }
 
 if (stats) {
 memset(stats, 0, sizeof *stats);
 }
-return netdev_offload_dpdk_destroy_flow(netdev, ufid, rte_flow);
+return netdev_offload_dpdk_destroy_flow(netdev, ufid,
+rte_flow_data->rte_flow);
 }
 
 static int
-- 
2.14.5

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

[ovs-dev] [PATCH V4 05/17] netdev-dpdk: Introduce rte flow query function

[Eli Britstein]

Introduce a rte flow query function as a pre-step towards reading HW
statistics of fully offloaded flows.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-dpdk.c | 30 ++
 lib/netdev-dpdk.h |  6 ++
 2 files changed, 36 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 89c73a29b..87d005062 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4477,6 +4477,36 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
 return flow;
 }
 
+int
+netdev_dpdk_rte_flow_query_count(struct netdev *netdev,
+ struct rte_flow *rte_flow,
+ struct rte_flow_query_count *query,
+ struct rte_flow_error *error)
+{
+struct rte_flow_action_count count = { .shared = 0, .id = 0 };
+const struct rte_flow_action actions[] = {
+{
+.type = RTE_FLOW_ACTION_TYPE_COUNT,
+.conf = ,
+},
+{
+.type = RTE_FLOW_ACTION_TYPE_END,
+},
+};
+struct netdev_dpdk *dev;
+int ret;
+
+if (!is_dpdk_class(netdev->netdev_class)) {
+return -1;
+}
+
+dev = netdev_dpdk_cast(netdev);
+ovs_mutex_lock(>mutex);
+ret = rte_flow_query(dev->port_id, rte_flow, actions, query, error);
+ovs_mutex_unlock(>mutex);
+return ret;
+}
+
 #define NETDEV_DPDK_CLASS_COMMON\
 .is_pmd = true, \
 .alloc = netdev_dpdk_alloc, \
diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
index 60631c4f0..59919a89a 100644
--- a/lib/netdev-dpdk.h
+++ b/lib/netdev-dpdk.h
@@ -31,6 +31,7 @@ struct rte_flow_error;
 struct rte_flow_attr;
 struct rte_flow_item;
 struct rte_flow_action;
+struct rte_flow_query_count;
 
 void netdev_dpdk_register(void);
 void free_dpdk_buf(struct dp_packet *);
@@ -47,6 +48,11 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
 const struct rte_flow_item *items,
 const struct rte_flow_action *actions,
 struct rte_flow_error *error);
+int
+netdev_dpdk_rte_flow_query_count(struct netdev *netdev,
+ struct rte_flow *rte_flow,
+ struct rte_flow_query_count *query,
+ struct rte_flow_error *error);
 
 #else
 
-- 
2.14.5

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

[ovs-dev] [PATCH V4 11/17] netdev-offload: Introduce a function to validate same flow api handle

[Eli Britstein]

Signed-off-by: Eli Britstein 
---
 lib/netdev-offload-provider.h |  1 +
 lib/netdev-offload.c  | 12 
 2 files changed, 13 insertions(+)

diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 4e1c4251d..5a809c0cd 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -89,6 +89,7 @@ struct netdev_flow_api {
 
 int netdev_register_flow_api_provider(const struct netdev_flow_api *);
 int netdev_unregister_flow_api_provider(const char *type);
+bool netdev_flow_api_equals(const struct netdev *, const struct netdev *);
 
 #ifdef __linux__
 extern const struct netdev_flow_api netdev_offload_tc;
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index ae01acda6..32eab5910 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -156,6 +156,18 @@ netdev_unregister_flow_api_provider(const char *type)
 return error;
 }
 
+bool
+netdev_flow_api_equals(const struct netdev *netdev1,
+   const struct netdev *netdev2)
+{
+const struct netdev_flow_api *netdev_flow_api1 =
+ovsrcu_get(const struct netdev_flow_api *, >flow_api);
+const struct netdev_flow_api *netdev_flow_api2 =
+ovsrcu_get(const struct netdev_flow_api *, >flow_api);
+
+return netdev_flow_api1 == netdev_flow_api2;
+}
+
 static int
 netdev_assign_flow_api(struct netdev *netdev)
 {
-- 
2.14.5

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

[ovs-dev] [PATCH V4 13/17] netdev-offload-dpdk: Support offload of output action

[Eli Britstein]

Support offload of output action, also configuring count action for
allowing query statistics of HW offloaded flows.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 Documentation/howto/dpdk.rst | 17 +++--
 NEWS |  1 +
 lib/netdev-offload-dpdk.c| 89 +---
 3 files changed, 98 insertions(+), 9 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 766a7950c..f0fe2bef7 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -370,19 +370,28 @@ The flow hardware offload is disabled by default and can 
be enabled by::
 
 $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
 
-So far only partial flow offload is implemented. Moreover, it only works
-with PMD drivers have the rte_flow action "MARK + RSS" support.
+Matches and actions are programmed into HW to achieve full offload of
+the flow. If not all actions are supported, fallback to partial flow
+offload (offloading matches only). Moreover, it only works with PMD
+drivers that support the configured rte_flow actions.
+Partial flow offload requires support of "MARK + RSS" actions. Full
+hardware offload requires support of the actions listed below.
 
 The validated NICs are:
 
 - Mellanox (ConnectX-4, ConnectX-4 Lx, ConnectX-5)
 - Napatech (NT200B01)
 
-Supported protocols for hardware offload are:
+Supported protocols for hardware offload matches are:
+
 - L2: Ethernet, VLAN
-- L3: IPv4, IPv6
+- L3: IPv4
 - L4: TCP, UDP, SCTP, ICMP
 
+Supported actions for hardware offload are:
+
+- Output.
+
 Further Reading
 ---
 
diff --git a/NEWS b/NEWS
index 2acde3fe8..c430999a0 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ Post-v2.12.0
  * DPDK ring ports (dpdkr) are deprecated and will be removed in next
releases.
  * Add support for DPDK 19.11.
+ * Add hardware offload support for output actions (experimental).
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index a60d68d2c..2be36731a 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -345,6 +345,26 @@ dump_flow_action(struct ds *s, const struct 
rte_flow_action *actions)
 } else {
 ds_put_cstr(s, "  RSS = null\n");
 }
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
+const struct rte_flow_action_count *count = actions->conf;
+
+ds_put_cstr(s, "rte flow count action:\n");
+if (count) {
+ds_put_format(s, "  Count: shared=%d, id=%d\n", count->shared,
+  count->id);
+} else {
+ds_put_cstr(s, "  Count = null\n");
+}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
+const struct rte_flow_action_port_id *port_id = actions->conf;
+
+ds_put_cstr(s, "rte flow port-id action:\n");
+if (port_id) {
+ds_put_format(s, "  Port-id: original=%d, id=%d\n",
+  port_id->original, port_id->id);
+} else {
+ds_put_cstr(s, "  Port-id = null\n");
+}
 } else {
 ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
 }
@@ -720,20 +740,79 @@ netdev_offload_dpdk_mark_rss(struct flow_patterns 
*patterns,
 return flow;
 }
 
+static void
+add_count_action(struct flow_actions *actions)
+{
+struct rte_flow_action_count *count = xzalloc(sizeof *count);
+
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
+}
+
+static int
+add_port_id_action(struct flow_actions *actions,
+   struct netdev *outdev)
+{
+struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
+int outdev_id;
+
+outdev_id = netdev_dpdk_get_port_id(outdev);
+if (outdev_id < 0) {
+return -1;
+}
+port_id->id = outdev_id;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
+return 0;
+}
+
+static int
+add_output_action(struct netdev *netdev,
+  struct flow_actions *actions,
+  const struct nlattr *nla,
+  struct offload_info *info)
+{
+struct netdev *outdev;
+odp_port_t port;
+int ret = 0;
+
+port = nl_attr_get_odp_port(nla);
+outdev = netdev_ports_get(port, info->dpif_class);
+if (outdev == NULL) {
+VLOG_DBG_RL(_rl,
+"Cannot find netdev for odp port %d", port);
+return -1;
+}
+if (!netdev_flow_api_equals(netdev, outdev) ||
+add_port_id_action(actions, outdev)) {
+VLOG_DBG_RL(_rl,
+"Output to %s cannot be offloaded",
+netdev_get_name(outdev));
+ret = -1;
+}
+netdev_close(outdev);
+return ret;
+}
+
 static int
-parse_flow_actions(struct netdev *netdev OVS_UNUSED,
+parse_flow_actions(struct netdev *netdev,
struct flow_actions *actions,
  

[ovs-dev] [PATCH V4 10/17] netdev-dpdk: Getter function for dpdk port id API

[Eli Britstein]

Add a getter function for using the dpdk port id outside the scope of
netdev-dpdk.c to be used for HW offload.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-dpdk.c | 18 ++
 lib/netdev-dpdk.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 87d005062..6c196e8dd 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4426,6 +4426,24 @@ unlock:
 return err;
 }
 
+int
+netdev_dpdk_get_port_id(struct netdev *netdev)
+{
+struct netdev_dpdk *dev;
+int ret = -1;
+
+if (!is_dpdk_class(netdev->netdev_class)) {
+goto out;
+}
+
+dev = netdev_dpdk_cast(netdev);
+ovs_mutex_lock(>mutex);
+ret = dev->port_id;
+ovs_mutex_unlock(>mutex);
+out:
+return ret;
+}
+
 bool
 netdev_dpdk_flow_api_supported(struct netdev *netdev)
 {
diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
index 59919a89a..848346cb4 100644
--- a/lib/netdev-dpdk.h
+++ b/lib/netdev-dpdk.h
@@ -53,6 +53,8 @@ netdev_dpdk_rte_flow_query_count(struct netdev *netdev,
  struct rte_flow *rte_flow,
  struct rte_flow_query_count *query,
  struct rte_flow_error *error);
+int
+netdev_dpdk_get_port_id(struct netdev *netdev);
 
 #else
 
-- 
2.14.5

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

[ovs-dev] [PATCH V4 17/17] netdev-offload-dpdk: Support offload of set TCP/UDP ports actions

[Eli Britstein]

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 Documentation/howto/dpdk.rst |  1 +
 NEWS |  4 +--
 lib/netdev-offload-dpdk.c| 85 
 3 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index c440bf28e..be950d7ce 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -394,6 +394,7 @@ Supported actions for hardware offload are:
 - Drop.
 - Modification of Ethernet (mod_dl_src/mod_dl_dst).
 - Modification of IPv4 (mod_nw_src/mod_nw_dst/mod_nw_ttl).
+- Modification of TCP/UDP (mod_tp_src/mod_tp_dst).
 
 Further Reading
 ---
diff --git a/NEWS b/NEWS
index 297ca6fff..b611ed76d 100644
--- a/NEWS
+++ b/NEWS
@@ -26,8 +26,8 @@ Post-v2.12.0
  * DPDK ring ports (dpdkr) are deprecated and will be removed in next
releases.
  * Add support for DPDK 19.11.
- * Add hardware offload support for output, drop and set actions of
-   MAC and IPv4 (experimental).
+ * Add hardware offload support for output, drop, set of MAC, IPv4 and
+   TCP/UDP ports actions (experimental).
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 0f0685f4d..0ec840ce1 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -405,6 +405,19 @@ dump_flow_action(struct ds *s, const struct 
rte_flow_action *actions)
 } else {
 ds_put_cstr(s, "  Set-ttl = null\n");
 }
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_TP_SRC ||
+   actions->type == RTE_FLOW_ACTION_TYPE_SET_TP_DST) {
+const struct rte_flow_action_set_tp *set_tp = actions->conf;
+char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_TP_DST
+   ? "dst" : "src";
+
+ds_put_format(s, "rte flow set-tcp/udp-port-%s action:\n", dirstr);
+if (set_tp) {
+ds_put_format(s, "  Set-%s-tcp/udp-port: %"PRIu16"\n", dirstr,
+  ntohs(set_tp->port));
+} else {
+ds_put_format(s, "  Set-%s-tcp/udp-port = null\n", dirstr);
+}
 } else {
 ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
 }
@@ -957,6 +970,78 @@ parse_set_actions(struct flow_actions *actions,
 VLOG_DBG_RL(_rl, "Unsupported IPv4 set action");
 return -1;
 }
+} else if (nl_attr_type(sa) == OVS_KEY_ATTR_TCP) {
+const struct ovs_key_tcp *key = nl_attr_get(sa);
+const struct ovs_key_tcp *mask = masked ?
+get_mask(sa, struct ovs_key_tcp) : NULL;
+struct ovs_key_tcp consumed_mask;
+
+if (masked) {
+memcpy(_mask, mask, sizeof consumed_mask);
+} else {
+memset(_mask, 0, sizeof consumed_mask);
+}
+
+BUILD_ASSERT(sizeof(struct rte_flow_action_set_tp) ==
+ sizeof key->tcp_src);
+if (add_set_flow_action(actions, >tcp_src,
+mask ? >tcp_src : NULL,
+sizeof key->tcp_src,
+RTE_FLOW_ACTION_TYPE_SET_TP_SRC)) {
+return -1;
+}
+memset(_mask.tcp_src, 0, sizeof consumed_mask.tcp_src);
+
+BUILD_ASSERT(sizeof(struct rte_flow_action_set_tp) ==
+ sizeof key->tcp_dst);
+if (add_set_flow_action(actions, >tcp_dst,
+mask ? >tcp_dst : NULL,
+sizeof key->tcp_dst,
+RTE_FLOW_ACTION_TYPE_SET_TP_DST)) {
+return -1;
+}
+memset(_mask.tcp_dst, 0, sizeof consumed_mask.tcp_dst);
+
+if (!is_all_zeros(_mask, sizeof consumed_mask)) {
+VLOG_DBG_RL(_rl, "Unsupported TCP set action");
+return -1;
+}
+} else if (nl_attr_type(sa) == OVS_KEY_ATTR_UDP) {
+const struct ovs_key_udp *key = nl_attr_get(sa);
+const struct ovs_key_udp *mask = masked ?
+get_mask(sa, struct ovs_key_udp) : NULL;
+struct ovs_key_udp consumed_mask;
+
+if (masked) {
+memcpy(_mask, mask, sizeof consumed_mask);
+} else {
+memset(_mask, 0, sizeof consumed_mask);
+}
+
+BUILD_ASSERT(sizeof(struct rte_flow_action_set_tp) ==
+ sizeof key->udp_src);
+if (add_set_flow_action(actions, >udp_src,
+mask ? >udp_src : NULL,
+sizeof key->udp_src,
+RTE_FLOW_ACTION_TYPE_SET_TP_SRC)) {
+return -1;
+}
+memset(_mask.udp_src, 

[ovs-dev] [PATCH V4 00/17] netdev datapath actions offload

[Eli Britstein]

Currently, netdev datapath offload only accelerates the flow match
sequence by associating a mark per flow. This series introduces the full
offload of netdev datapath flows by having the HW also perform the flow
actions.

This series adds HW offload for output, drop, set MAC, set IPv4, set
TCP/UDP ports and tunnel push actions using the DPDK rte_flow API.

v1 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/614511472
v2 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/619262148
v3 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/622253870
v4 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/625654160

v1-v2:
- fallback to mark/rss in case of any failure of action offload.
- dp_layer changed to "in_hw" in case the flow is offloaded.
- using netdev_ports_get instead of dp_netdev_lookup_port in stats reading.
- using flow_dump_next API instead of a new API.
- validity checks for last action of output and clone.
- count action should be done for drop as well.
- validity check for output action.
- added doc in Documentation/howto/dpdk.rst.
v2-v3:
- removed refactoring for netdev-offload-dpdk-flow.c file.
- dropped flush commits.
- dbg prints rate-limited, and outside lock.
- added validity check for output action - same flow_api handle for both 
netdevs.
- added a mutex to protect possible concurrent deletion/querying of a flow.
- statistics are collected using flow_get API.
- added more documentation in Documentation/howto/dpdk.rst.
v3-v4:
- debug prints moved to netdev-offload-dpdk.c.
- flow get returns full stats, not diff.
- removed additional fields from offload_info and dp_netdev_flow.
- read HW stats during dpif_netdev_flow_get as well as
  dpif_netdev_flow_dump_next.
- moved actions offload framework just before support commit.
- fixed memory leak in rewrite code.
- dropped clone commit - will be posted in next series.

Eli Britstein (16):
  netdev-offload-dpdk: Refactor flow patterns
  netdev-offload-dpdk: Refactor action items freeing scheme
  netdev-offload-dpdk: Dynamically allocate pattern items
  netdev-offload-dpdk: Improve HW offload flow debuggability
  netdev-dpdk: Introduce rte flow query function
  netdev-offload-dpdk: Return UFID-rte_flow entry in find method
  netdev-offload-dpdk: Implement flow get method
  dpif-netdev: Populate dpif class field in offload struct
  netdev-dpdk: Getter function for dpdk port id API
  netdev-offload: Introduce a function to validate same flow api handle
  netdev-offload-dpdk: Framework for actions offload
  netdev-offload-dpdk: Support offload of output action
  netdev-offload-dpdk: Support offload of drop action
  netdev-offload-dpdk: Support offload of set MAC actions
  netdev-offload-dpdk: Support offload of set IPv4 actions
  netdev-offload-dpdk: Support offload of set TCP/UDP ports actions

Ophir Munk (1):
  dpif-netdev: Update offloaded flows statistics

 Documentation/howto/dpdk.rst  |   21 +-
 NEWS  |2 +
 lib/dpif-netdev.c |   83 +++-
 lib/netdev-dpdk.c |   48 ++
 lib/netdev-dpdk.h |8 +
 lib/netdev-offload-dpdk.c | 1033 -
 lib/netdev-offload-provider.h |1 +
 lib/netdev-offload.c  |   12 +
 8 files changed, 968 insertions(+), 240 deletions(-)

-- 
2.14.5

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

Re: [ovs-dev] [PATCH] netdev-afxdp: Add pcap dump support.

[William Tu]

On Mon, Dec 16, 2019 at 5:57 AM Ilya Maximets  wrote:
>
> On 16.12.2019 14:43, William Tu wrote:
> > On Mon, Dec 16, 2019 at 1:41 AM Ilya Maximets  wrote:
> >>
> >> On 13.12.2019 21:59, William Tu wrote:
> >>> Debugging netdev-afxdp is hard because tcpdump does not work
> >>> at all, even for generic mode.  ovs-tcpdump which uses port
> >>> mirroring also does not work for netdev-afxdp.
> >>
> >> Hmm.  Why ovs-tcpdump doesn't work for you?  It should not depend
> >> on port type.  If it doesn't work we need to investigate this
> >> case and fix it because it's a very important tool.
> >
> > Because ovs-tcpdump still uses 'tcpdump' tool to capture packets,
> > (dump-cmd=tcpdump) and forward to mirror port.  Since tcpdump
> > sees no packet at all when type=afxdp, there is no packets to mirror.
>
> ovs-tcpdump creates simple tap interface and mirrors all the traffic
> to it.  tcpdump command is started on that tap interface (no xdp here).
> There is no difference from which port types you're mirroring the traffic.
> It works for DPDK based ports and should work for afxdp.

I got it working using ovs-tcpdump now, thanks!
ex: ovs-tcpdump -i afxdp-p0 -n -l -w /tmp/p0.pcap

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

[ovs-dev] [PATCH] ovs-thread: Avoid huge alignment on a base spinlock structure.

[Ilya Maximets]

Marking the structure as 64 bytes aligned forces compiler to produce
big holes in the containing structures in order to fulfill this
requirement.  Also, any structure that contains this one as a member
automatically inherits this huge alignment making resulted memory
layout not efficient.  For example, 'struct umem_pool' currently
uses 3 full cache lines (192 bytes) with only 32 bytes of actual data:

  struct umem_pool {
intindex;/*  0   4 */
unsigned int   size; /*  4   4 */

/* XXX 56 bytes hole, try to pack */

/* --- cacheline 1 boundary (64 bytes) --- */
struct ovs_spin lock __attribute__((__aligned__(64))); /* 64  64 */

/* XXX last struct has 48 bytes of padding */

/* --- cacheline 2 boundary (128 bytes) --- */
void * *   array;/* 128  8 */

/* size: 192, cachelines: 3, members: 4 */
/* sum members: 80, holes: 1, sum holes: 56 */
/* padding: 56 */
/* paddings: 1, sum paddings: 48 */
/* forced alignments: 1, forced holes: 1, sum forced holes: 56 */
  } __attribute__((__aligned__(64)));

Actual alignment of a spin lock is required only for Tx queue locks
inside netdev-afxdp to avoid false sharing, in all other cases
alignment only produces inefficient memory usage.

Also, CACHE_LINE_SIZE macro should be used instead of 64 as different
platforms may have different cache line sizes.

Using PADDED_MEMBERS to avoid alignment inheritance.

CC: William Tu 
Fixes: ae36d63d7e3c ("ovs-thread: Make struct spin lock cache aligned.")
Signed-off-by: Ilya Maximets 
---
 include/openvswitch/thread.h |  2 +-
 lib/netdev-afxdp.c   | 20 ++--
 lib/netdev-afxdp.h   | 13 +++--
 lib/netdev-linux-private.h   |  2 +-
 4 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/include/openvswitch/thread.h b/include/openvswitch/thread.h
index 5053cb309..acc822904 100644
--- a/include/openvswitch/thread.h
+++ b/include/openvswitch/thread.h
@@ -34,7 +34,7 @@ struct OVS_LOCKABLE ovs_mutex {
 };
 
 #ifdef HAVE_PTHREAD_SPIN_LOCK
-OVS_ALIGNED_STRUCT(64, OVS_LOCKABLE ovs_spin) {
+struct OVS_LOCKABLE ovs_spin {
 pthread_spinlock_t lock;
 const char *where;  /* NULL if and only if uninitialized. */
 };
diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index ca2dfd005..c59a671e7 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -40,6 +40,7 @@
 #include "openvswitch/compiler.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/list.h"
+#include "openvswitch/thread.h"
 #include "openvswitch/vlog.h"
 #include "packets.h"
 #include "socket-util.h"
@@ -158,6 +159,13 @@ struct xsk_socket_info {
 atomic_uint64_t tx_dropped;
 };
 
+struct netdev_afxdp_tx_lock {
+/* Padding to make netdev_afxdp_tx_lock exactly one cache line long. */
+PADDED_MEMBERS(CACHE_LINE_SIZE,
+struct ovs_spin lock;
+);
+};
+
 #ifdef HAVE_XDP_NEED_WAKEUP
 static inline void
 xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
@@ -512,10 +520,10 @@ xsk_configure_all(struct netdev *netdev)
 }
 
 n_txq = netdev_n_txq(netdev);
-dev->tx_locks = xcalloc(n_txq, sizeof *dev->tx_locks);
+dev->tx_locks = xzalloc_cacheline(n_txq * sizeof *dev->tx_locks);
 
 for (i = 0; i < n_txq; i++) {
-ovs_spin_init(>tx_locks[i]);
+ovs_spin_init(>tx_locks[i].lock);
 }
 
 return 0;
@@ -577,9 +585,9 @@ xsk_destroy_all(struct netdev *netdev)
 
 if (dev->tx_locks) {
 for (i = 0; i < netdev_n_txq(netdev); i++) {
-ovs_spin_destroy(>tx_locks[i]);
+ovs_spin_destroy(>tx_locks[i].lock);
 }
-free(dev->tx_locks);
+free_cacheline(dev->tx_locks);
 dev->tx_locks = NULL;
 }
 }
@@ -1076,9 +1084,9 @@ netdev_afxdp_batch_send(struct netdev *netdev, int qid,
 dev = netdev_linux_cast(netdev);
 qid = qid % netdev_n_txq(netdev);
 
-ovs_spin_lock(>tx_locks[qid]);
+ovs_spin_lock(>tx_locks[qid].lock);
 ret = __netdev_afxdp_batch_send(netdev, qid, batch);
-ovs_spin_unlock(>tx_locks[qid]);
+ovs_spin_unlock(>tx_locks[qid].lock);
 } else {
 ret = __netdev_afxdp_batch_send(netdev, qid, batch);
 }
diff --git a/lib/netdev-afxdp.h b/lib/netdev-afxdp.h
index 8188bc669..ae6971efd 100644
--- a/lib/netdev-afxdp.h
+++ b/lib/netdev-afxdp.h
@@ -34,15 +34,16 @@ enum afxdp_mode {
 OVS_AF_XDP_MODE_MAX,
 };
 
-struct netdev;
-struct xsk_socket_info;
-struct xdp_umem;
-struct dp_packet_batch;
-struct smap;
 struct dp_packet;
+struct dp_packet_batch;
+struct netdev;
+struct netdev_afxdp_tx_lock;
+struct netdev_custom_stats;
 struct netdev_rxq;
 struct netdev_stats;
-struct netdev_custom_stats;
+struct smap;
+struct xdp_umem;
+struct xsk_socket_info;
 
 int netdev_afxdp_rxq_construct(struct netdev_rxq *rxq_);
 void netdev_afxdp_rxq_destruct(struct netdev_rxq *rxq_);
diff --git 

[ovs-dev] [PATCH] Documentation: Fix ovs-tcpdump options.

[William Tu]

Signed-off-by: William Tu 
---
 Documentation/ref/ovs-tcpdump.8.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/ref/ovs-tcpdump.8.rst 
b/Documentation/ref/ovs-tcpdump.8.rst
index 048b70f23daf..b9f8cdf6f786 100644
--- a/Documentation/ref/ovs-tcpdump.8.rst
+++ b/Documentation/ref/ovs-tcpdump.8.rst
@@ -5,7 +5,7 @@ ovs-tcpdump
 Synopsis
 
 
-``ovs-tcpdump -i  tcpdump ...``
+``ovs-tcpdump -i  ...``
 
 Description
 ===
-- 
2.7.4

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

Re: [ovs-dev] [PATCH] netdev-afxdp: Add pcap dump support.

[Ilya Maximets]

On 16.12.2019 14:43, William Tu wrote:
> On Mon, Dec 16, 2019 at 1:41 AM Ilya Maximets  wrote:
>>
>> On 13.12.2019 21:59, William Tu wrote:
>>> Debugging netdev-afxdp is hard because tcpdump does not work
>>> at all, even for generic mode.  ovs-tcpdump which uses port
>>> mirroring also does not work for netdev-afxdp.
>>
>> Hmm.  Why ovs-tcpdump doesn't work for you?  It should not depend
>> on port type.  If it doesn't work we need to investigate this
>> case and fix it because it's a very important tool.
> 
> Because ovs-tcpdump still uses 'tcpdump' tool to capture packets,
> (dump-cmd=tcpdump) and forward to mirror port.  Since tcpdump
> sees no packet at all when type=afxdp, there is no packets to mirror.

ovs-tcpdump creates simple tap interface and mirrors all the traffic
to it.  tcpdump command is started on that tap interface (no xdp here).
There is no difference from which port types you're mirroring the traffic.
It works for DPDK based ports and should work for afxdp.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-afxdp: Add pcap dump support.

[William Tu]

On Mon, Dec 16, 2019 at 1:41 AM Ilya Maximets  wrote:
>
> On 13.12.2019 21:59, William Tu wrote:
> > Debugging netdev-afxdp is hard because tcpdump does not work
> > at all, even for generic mode.  ovs-tcpdump which uses port
> > mirroring also does not work for netdev-afxdp.
>
> Hmm.  Why ovs-tcpdump doesn't work for you?  It should not depend
> on port type.  If it doesn't work we need to investigate this
> case and fix it because it's a very important tool.

Because ovs-tcpdump still uses 'tcpdump' tool to capture packets,
(dump-cmd=tcpdump) and forward to mirror port.  Since tcpdump
sees no packet at all when type=afxdp, there is no packets to mirror.

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

Re: [ovs-dev] [PATCH v4 08/10] netdev-offload-tc: Add conntrack support

[0-day Robot]

Bleep bloop.  Greetings Paul Blakey, 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:
/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/dpif-netlink-rtnl.lo -MD -MP -MF $depbase.Tpo -c -o 
lib/dpif-netlink-rtnl.lo lib/dpif-netlink-rtnl.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/dpif-netlink-rtnl.lo -MD -MP -MF 
lib/.deps/dpif-netlink-rtnl.Tpo -c lib/dpif-netlink-rtnl.c -o 
lib/dpif-netlink-rtnl.o
depbase=`echo lib/if-notifier.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/if-notifier.lo -MD -MP -MF $depbase.Tpo -c -o lib/if-notifier.lo 
lib/if-notifier.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/if-notifier.lo -MD -MP -MF 
lib/.deps/if-notifier.Tpo -c lib/if-notifier.c -o lib/if-notifier.o
depbase=`echo lib/netdev-linux.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/netdev-linux.lo -MD -MP -MF $depbase.Tpo -c -o lib/netdev-linux.lo 
lib/netdev-linux.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/netdev-linux.lo -MD -MP -MF 
lib/.deps/netdev-linux.Tpo -c lib/netdev-linux.c -o lib/netdev-linux.o
depbase=`echo lib/netdev-offload-tc.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/netdev-offload-tc.lo -MD -MP -MF $depbase.Tpo -c -o 
lib/netdev-offload-tc.lo lib/netdev-offload-tc.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/netdev-offload-tc.lo -MD -MP -MF 
lib/.deps/netdev-offload-tc.Tpo -c lib/netdev-offload-tc.c -o 
lib/netdev-offload-tc.o
lib/netdev-offload-tc.c: In function 'parse_put_flow_ct_action':
lib/netdev-offload-tc.c:895:13: error: unused variable 'err' 

Re: [ovs-dev] [PATCH v4 07/10] netdev-offload-tc: Add recirculation support via tc chains

[0-day Robot]

Bleep bloop.  Greetings Paul Blakey, 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: Line is 92 characters long (recommended limit is 79)
#46 FILE: lib/dpif-netlink.c:376:
VLOG_INFO("dpif_netlink: tc recirc id sharing with OvS datapath is 
supported.");

Lines checked: 392, 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] Why did you authorized Mrs. Lyndia Paulson to receive your funds $4.800.000 us dollars from this office?,

[Mrs. ALAN UDE, Official Director.Money Gram-Benin]

MONEY GRAM.
AROPORT INTL DE COTONOU COTONOU.
Office of Mrs. ALAN UDE.

Attn,Dear Funds beneficiary.

I am Mrs. ALAN UDE., Official Director.Money Gram-Benin
Confirm to us urgent,
Why  did you authorized Mrs. Lyndia Paulson to receive your funds
$4.800.000 us dollars from this office?, I need your urgent response
now because this woman contacted us again this morning with all her
mailing address stating that you are very ill, meanwhile you have
advised her to claim the funds on your behalf, i am real confuse now,
and i need to hear from you urgent before our office will release your
transfer to this woman,
Here is the address she forward to us this morning where your funds
will be transfer to her.Please do you know this address?

Full name, Mrs. Lyndia Paulson
Address. 21644 Vaca Dr.
Eckert Colorado 81418
Country. USA

Also i want you to know that we have cut down the transfer fees to
$23.00 only for your help, to enable you afford it,
this is because we need all our real customers to receive their funds
before the end of this year 2019, due after this physical year 2019,
all remaining and unclaimed funds in our office will be cancelled, so
you are advised to try and send the remaining $23.00 today so that you
can pick up your first $US5000.00 immediately today,
I promise you with all of my life, no more fees, this $23.00 is the
last fee you will pay to receive your transfer now, once i receive it,
you must pick up first $US5000.00 at your Money Gram today,
and i will send you another US$5000.00 tomorrow morning, i just plan
to make sure that you receive at least $100,000.00 US Dollars before
the Christmas day, to enable you celebrate a good Christmas with your
family. Note Iam only here to help you out and make sure you did not
lose your transfer total amount of $4.8m us dollars to Mrs. Lyndia
Paulson ok.
So try and send the $23.00 today once you receive this email ok. God
bless you, it is your time to rejoice and be happy forever.
Send the transfer fee $23.00 to us by Money Gram.

Receiver's NameAlan Ude
Country--Benin
City address-Cotonou
Amount--23.00 dollars Only
Text  Question-Honest
Answer-Trust

Thanks
I wait for your urgent response
Mrs. ALAN UDE., Official Director.Money Gram-Benin
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v4 00/10] Add support for offloading CT datapath rules to TC

[Paul Blakey]

The following patchset introduces hardware offload of OVS connection
tracking datapath rules.

OVS uses ct() and recirc() (recirculation) actions and recirc_id()/ct_state()
matches to support connection tracking.

The datapath rules are in the form of:

recirc_id(0),in_port(dev1),eth_type(0x0800),ct_state(-trk) 
actions:ct(),recirc(2)
recirc_id(2),in_port(dev1),eth_type(0x0800),ct_state(+trk+est) actions:4

This patchset will translate ct_state() and recirc_id() matches to tc 
ct_state and chain matches respectively. The datapath actions ct() and recirc()
will be translated to tc actions ct and goto chain respectively.

The tc equivalent commands for the above rules are:

$ tc filter add dev dev1 ingress \
prio 1 chain 0 proto ip \
flower tcp ct_state -trk \
action ct pipe \
action goto chain 2

$ tc filter add dev dev1 ingress \
prio 1 chain 2 proto ip \
flower tcp ct_state +trk+est \
action mirred egress redirect dev dev2

Thanks,
Paul

Paul Blakey (10):
  match: Add match_set_ct_zone_masked helper
  compat: Add tc ct action and flower matches defines for older kernels
  tc: Introduce tcf_id to specify a tc filter
  netdev-offload-tc: Implement netdev tc flush via tc filter del
  dpif: Add support to set user features
  tc: Move tunnel_key unset action before output ports
  netdev-offload-tc: Add recirculation support via tc chains
  netdev-offload-tc: Add conntrack support
  netdev-offload-tc: Add conntrack label and mark support
  netdev-offload-tc: Add conntrack nat support

 datapath/linux/compat/include/linux/openvswitch.h |   3 +
 include/linux/automake.mk |   3 +-
 include/linux/pkt_cls.h   |  46 +-
 include/linux/tc_act/tc_ct.h  |  41 ++
 include/openvswitch/match.h   |   2 +
 lib/dpif-netdev.c |   1 +
 lib/dpif-netlink.c|  78 ++-
 lib/dpif-provider.h   |   2 +
 lib/dpif.c|   9 +
 lib/dpif.h|   2 +
 lib/match.c   |  10 +-
 lib/netdev-linux.c|   6 +-
 lib/netdev-offload-tc.c   | 600 +++---
 lib/tc.c  | 448 
 lib/tc.h  | 112 +++-
 15 files changed, 1075 insertions(+), 288 deletions(-)
 create mode 100644 include/linux/tc_act/tc_ct.h

-- 
1.8.3.1

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

[ovs-dev] [PATCH v4 06/10] tc: Move tunnel_key unset action before output ports

[Paul Blakey]

Since OvS datapath gets packets already decapsulated from tunnel devices,
it doesn't explicitly decapsulate them. So in a recirculation setup,
the tunnel matching continues in the recirculation as the tunnel
metadata still exists on the SKB.

Tunnel key unset action unsets this metadata. Some drivers might rely
on this explicit tunnel key unset to know when to decapsulate the packet
instead of the device type. So instead of removing it completly,
we move it near the output actions.

This way, we also keep SKB metadata through recirculation, and for
non-recirculation rules, the resulting tc rules should remain the same.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:
V3->V4:
Fix: Set flower tunnel attribute to true, if any of the tunnel
 attributes exists (we used to rely on unset action).
V2->V3:
Actually removed old tunnel set if tunnel exists (instead of just
adding a new one before a port)
Moved patch earlier to help git bisect
---
 lib/tc.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/lib/tc.c b/lib/tc.c
index b2d8ca7..7a4acce 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -665,6 +665,12 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct 
tc_flower *flower)
 flower->mask.tunnel.ttl =
 nl_attr_get_u8(attrs[TCA_FLOWER_KEY_ENC_IP_TTL_MASK]);
 }
+
+if (!is_all_zeros(>mask.tunnel, sizeof flower->mask.tunnel) ||
+!is_all_zeros(>key.tunnel, sizeof flower->key.tunnel)) {
+flower->tunnel = true;
+}
+
 if (attrs[TCA_FLOWER_KEY_ENC_OPTS] &&
 attrs[TCA_FLOWER_KEY_ENC_OPTS_MASK]) {
  err = nl_parse_flower_tunnel_opts(attrs[TCA_FLOWER_KEY_ENC_OPTS],
@@ -2091,24 +2097,17 @@ nl_msg_put_flower_rewrite_pedits(struct ofpbuf *request,
 static int
 nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower)
 {
+bool ingress, released = false;
 size_t offset;
 size_t act_offset;
 uint16_t act_index = 1;
 struct tc_action *action;
 int i, ifindex = 0;
-bool ingress;
 
 offset = nl_msg_start_nested(request, TCA_FLOWER_ACT);
 {
 int error;
 
-if (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);
-}
-
 action = flower->actions;
 for (i = 0; i < flower->action_count; i++, action++) {
 switch (action->type) {
@@ -2185,6 +2184,13 @@ 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_end_nested(request, act_offset);
+released = true;
+}
+
 ingress = action->out.ingress;
 ifindex = action->out.ifindex_out;
 if (ifindex < 1) {
-- 
1.8.3.1

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

[ovs-dev] [PATCH v4 07/10] netdev-offload-tc: Add recirculation support via tc chains

[Paul Blakey]

Each recirculation id will create a tc chain, and we translate
the recirculation action to a tc goto chain action.

We check for kernel support for this by probing OvS Datapath for the
tc recirc id sharing feature. If supported, we can offload rules
that match on recirc_id, and recirculation action safely.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:
V3->V4:
Always try to enable recirc_id support (on dpif_open) if hardware offload 
is enabled.
V2->V3:
Merged part of probe for recirc_id support in here to help future git 
bisect.
Added tunnel released check to avoid bug with mirroring
Removed cascading condition in netdev_tc_flow_put() check of recirc_id 
support

V1->V2:
moved make_tc_id_chain helper to tc.h as static inline
updated is_tc_id_eq with chain compare instead of find_ufid
---
 lib/dpif-netlink.c  | 22 ++
 lib/netdev-offload-tc.c | 35 +--
 lib/tc.c| 49 +++--
 lib/tc.h| 18 +-
 4 files changed, 107 insertions(+), 17 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index ef06dd4..89f5688 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -110,6 +110,8 @@ static int dpif_netlink_dp_transact(const struct 
dpif_netlink_dp *request,
 static int dpif_netlink_dp_get(const struct dpif *,
struct dpif_netlink_dp *reply,
struct ofpbuf **bufp);
+static int
+dpif_netlink_set_features(struct dpif *dpif_, uint32_t new_features);
 
 struct dpif_netlink_flow {
 /* Generic Netlink header. */
@@ -364,6 +366,16 @@ dpif_netlink_open(const struct dpif_class *class 
OVS_UNUSED, const char *name,
 
 error = open_dpif(, dpifp);
 ofpbuf_delete(buf);
+
+if (!error && create && netdev_is_flow_api_enabled()) {
+int set_err;
+
+set_err = dpif_netlink_set_features(*dpifp,
+OVS_DP_F_TC_RECIRC_SHARING);
+if (!set_err) {
+VLOG_INFO("dpif_netlink: tc recirc id sharing with OvS datapath is 
supported.");
+}
+}
 return error;
 }
 
@@ -1638,6 +1650,7 @@ dpif_netlink_netdev_match_to_dpif_flow(struct match 
*match,
 .mask = >wc.masks,
 .support = {
 .max_vlan_headers = 2,
+.recirc = true,
 },
 };
 size_t offset;
@@ -2037,6 +2050,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
dpif_flow_put *put)
 struct offload_info info;
 ovs_be16 dst_port = 0;
 uint8_t csum_on = false;
+bool recirc_act;
 int err;
 
 if (put->flags & DPIF_FP_PROBE) {
@@ -2076,9 +2090,17 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
dpif_flow_put *put)
 csum_on = tnl_cfg->csum;
 }
 netdev_close(outdev);
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_RECIRC) {
+recirc_act = true;
 }
 }
 
+if ((recirc_act || match.flow.recirc_id)
+&& !(dpif->user_features & OVS_DP_F_TC_RECIRC_SHARING)) {
+err = EOPNOTSUPP;
+goto out;
+}
+
 info.dpif_class = dpif_class;
 info.tp_dst_port = dst_port;
 info.tunnel_csum_on = csum_on;
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 15b39e6..e3b0415 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -38,6 +38,7 @@
 #include "tc.h"
 #include "unaligned.h"
 #include "util.h"
+#include "dpif-provider.h"
 
 VLOG_DEFINE_THIS_MODULE(netdev_offload_tc);
 
@@ -206,9 +207,12 @@ static void
 add_ufid_tc_mapping(struct netdev *netdev, const ovs_u128 *ufid,
 struct tcf_id *id)
 {
-size_t ufid_hash = hash_bytes(ufid, sizeof *ufid, 0);
-size_t tc_hash = hash_int(hash_int(id->prio, id->handle), id->ifindex);
 struct ufid_tc_data *new_data = xzalloc(sizeof *new_data);
+size_t ufid_hash = hash_bytes(ufid, sizeof *ufid, 0);
+size_t tc_hash;
+
+tc_hash = hash_int(hash_int(id->prio, id->handle), id->ifindex);
+tc_hash = hash_int(id->chain, tc_hash);
 
 new_data->ufid = *ufid;
 new_data->id = *id;
@@ -252,8 +256,11 @@ get_ufid_tc_mapping(const ovs_u128 *ufid, struct tcf_id 
*id)
 static bool
 find_ufid(struct netdev *netdev, struct tcf_id *id, ovs_u128 *ufid)
 {
-size_t tc_hash = hash_int(hash_int(id->prio, id->handle), id->ifindex);
 struct ufid_tc_data *data;
+size_t tc_hash;
+
+tc_hash = hash_int(hash_int(id->prio, id->handle), id->ifindex);
+tc_hash = hash_int(id->chain, tc_hash);
 
 ovs_mutex_lock(_lock);
 HMAP_FOR_EACH_WITH_HASH (data, tc_to_ufid_node, tc_hash,  _to_ufid) {
@@ -739,6 +746,10 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 nl_msg_put_u32(buf, OVS_ACTION_ATTR_OUTPUT, 
odp_to_u32(outport));
 }
 break;
+case TC_ACT_GOTO: {
+nl_msg_put_u32(buf, 

[ovs-dev] [PATCH v4 08/10] netdev-offload-tc: Add conntrack support

[Paul Blakey]

Zone and ct_state first.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
 lib/dpif-netlink.c  |   2 +
 lib/netdev-offload-tc.c | 136 
 lib/tc.c| 122 +++
 lib/tc.h|  11 
 4 files changed, 261 insertions(+), 10 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 89f5688..c9dd193 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1651,6 +1651,8 @@ dpif_netlink_netdev_match_to_dpif_flow(struct match 
*match,
 .support = {
 .max_vlan_headers = 2,
 .recirc = true,
+.ct_state = true,
+.ct_zone = true,
 },
 };
 size_t offset;
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index e3b0415..7a346e3 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -595,6 +595,35 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 match_set_tp_dst_masked(match, key->sctp_dst, mask->sctp_dst);
 match_set_tp_src_masked(match, key->sctp_src, mask->sctp_src);
 }
+
+if (mask->ct_state) {
+uint8_t ct_statev = 0, ct_statem = 0;
+
+if (mask->ct_state & TCA_FLOWER_KEY_CT_FLAGS_NEW) {
+if (key->ct_state & TCA_FLOWER_KEY_CT_FLAGS_NEW) {
+ct_statev |= OVS_CS_F_NEW;
+}
+ct_statem |= OVS_CS_F_NEW;
+}
+
+if (mask->ct_state & TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED) {
+if (key->ct_state & TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED) {
+ct_statev |= OVS_CS_F_ESTABLISHED;
+}
+ct_statem |= OVS_CS_F_ESTABLISHED;
+}
+
+if (mask->ct_state & TCA_FLOWER_KEY_CT_FLAGS_TRACKED) {
+if (key->ct_state & TCA_FLOWER_KEY_CT_FLAGS_TRACKED) {
+ct_statev |= OVS_CS_F_TRACKED;
+}
+ct_statem |= OVS_CS_F_TRACKED;
+}
+
+match_set_ct_state_masked(match, ct_statev, ct_statem);
+}
+
+match_set_ct_zone_masked(match, key->ct_zone, mask->ct_zone);
 }
 
 if (flower->tunnel) {
@@ -746,6 +775,27 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 nl_msg_put_u32(buf, OVS_ACTION_ATTR_OUTPUT, 
odp_to_u32(outport));
 }
 break;
+case TC_ACT_CT: {
+size_t ct_offset;
+
+if (action->ct.clear) {
+nl_msg_put_flag(buf, OVS_ACTION_ATTR_CT_CLEAR);
+break;
+}
+
+ct_offset = nl_msg_start_nested(buf, OVS_ACTION_ATTR_CT);
+
+if (action->ct.commit) {
+nl_msg_put_flag(buf, OVS_CT_ATTR_COMMIT);
+}
+
+if (action->ct.zone) {
+nl_msg_put_u16(buf, OVS_CT_ATTR_ZONE, action->ct.zone);
+}
+
+nl_msg_end_nested(buf, ct_offset);
+}
+break;
 case TC_ACT_GOTO: {
 nl_msg_put_u32(buf, OVS_ACTION_ATTR_RECIRC, action->chain);
 }
@@ -835,6 +885,34 @@ parse_mpls_set_action(struct tc_flower *flower, struct 
tc_action *action,
 }
 
 static int
+parse_put_flow_ct_action(struct tc_flower *flower,
+ struct tc_action *action,
+ const struct nlattr *ct,
+ size_t ct_len)
+{
+const struct nlattr *ct_attr;
+size_t ct_left;
+int err;
+
+NL_ATTR_FOR_EACH_UNSAFE (ct_attr, ct_left, ct, ct_len) {
+switch (nl_attr_type(ct_attr)) {
+case OVS_CT_ATTR_COMMIT: {
+action->ct.commit = true;
+}
+break;
+case OVS_CT_ATTR_ZONE: {
+action->ct.zone = nl_attr_get_u16(ct_attr);
+}
+break;
+}
+}
+
+action->type = TC_ACT_CT;
+flower->action_count++;
+return 0;
+}
+
+static int
 parse_put_flow_set_masked_action(struct tc_flower *flower,
  struct tc_action *action,
  const struct nlattr *set,
@@ -1016,16 +1094,6 @@ test_key_and_mask(struct match *match)
 return EOPNOTSUPP;
 }
 
-if (mask->ct_state) {
-VLOG_DBG_RL(, "offloading attribute ct_state isn't supported");
-return EOPNOTSUPP;
-}
-
-if (mask->ct_zone) {
-VLOG_DBG_RL(, "offloading attribute ct_zone isn't supported");
-return EOPNOTSUPP;
-}
-
 if (mask->ct_mark) {
 VLOG_DBG_RL(, "offloading attribute ct_mark isn't supported");
 return EOPNOTSUPP;
@@ -1364,6 +1432,42 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 }
 }
 
+if (mask->ct_state) {
+if (mask->ct_state & 

[ovs-dev] [PATCH v4 02/10] compat: Add tc ct action and flower matches defines for older kernels

[Paul Blakey]

Update kernel UAPI to support conntrack matches, and the
tc actions ct and goto chain.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
 include/linux/automake.mk|  3 ++-
 include/linux/pkt_cls.h  | 46 +---
 include/linux/tc_act/tc_ct.h | 41 +++
 3 files changed, 86 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/tc_act/tc_ct.h

diff --git a/include/linux/automake.mk b/include/linux/automake.mk
index c759186..8f063f4 100644
--- a/include/linux/automake.mk
+++ b/include/linux/automake.mk
@@ -6,4 +6,5 @@ noinst_HEADERS += \
include/linux/tc_act/tc_pedit.h \
include/linux/tc_act/tc_skbedit.h \
include/linux/tc_act/tc_tunnel_key.h \
-   include/linux/tc_act/tc_vlan.h
+   include/linux/tc_act/tc_vlan.h \
+   include/linux/tc_act/tc_ct.h
diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
index 55f3ef1..b0a5ce8 100644
--- a/include/linux/pkt_cls.h
+++ b/include/linux/pkt_cls.h
@@ -44,7 +44,21 @@ enum {
 #define TC_ACT_QUEUED  5
 #define TC_ACT_REPEAT  6
 #define TC_ACT_REDIRECT7
-#define TC_ACT_JUMP0x1000
+
+/* There is a special kind of actions called "extended actions",
+ * which need a value parameter. These have a local opcode located in
+ * the highest nibble, starting from 1. The rest of the bits
+ * are used to carry the value. These two parts together make
+ * a combined opcode.
+ */
+#define __TC_ACT_EXT_SHIFT 28
+#define __TC_ACT_EXT(local) ((local) << __TC_ACT_EXT_SHIFT)
+#define TC_ACT_EXT_VAL_MASK ((1 << __TC_ACT_EXT_SHIFT) - 1)
+#define TC_ACT_EXT_CMP(combined, opcode) \
+   (((combined) & (~TC_ACT_EXT_VAL_MASK)) == opcode)
+
+#define TC_ACT_JUMP __TC_ACT_EXT(1)
+#define TC_ACT_GOTO_CHAIN __TC_ACT_EXT(2)
 
 struct tc_police {
__u32   index;
@@ -207,16 +221,42 @@ enum {
TCA_FLOWER_KEY_CVLAN_PRIO,  /* u8   */
TCA_FLOWER_KEY_CVLAN_ETH_TYPE,  /* be16 */
 
-   TCA_FLOWER_KEY_ENC_IP_TOS,  /* u8 */
+   TCA_FLOWER_KEY_ENC_IP_TOS,  /* u8 */
TCA_FLOWER_KEY_ENC_IP_TOS_MASK, /* u8 */
-   TCA_FLOWER_KEY_ENC_IP_TTL,  /* u8 */
+   TCA_FLOWER_KEY_ENC_IP_TTL,  /* u8 */
TCA_FLOWER_KEY_ENC_IP_TTL_MASK, /* u8 */
+
TCA_FLOWER_KEY_ENC_OPTS,
TCA_FLOWER_KEY_ENC_OPTS_MASK,
 
+   TCA_FLOWER_IN_HW_COUNT,
+
+   TCA_FLOWER_KEY_PORT_SRC_MIN,/* be16 */
+   TCA_FLOWER_KEY_PORT_SRC_MAX,/* be16 */
+   TCA_FLOWER_KEY_PORT_DST_MIN,/* be16 */
+   TCA_FLOWER_KEY_PORT_DST_MAX,/* be16 */
+
+   TCA_FLOWER_KEY_CT_STATE,/* u16 */
+   TCA_FLOWER_KEY_CT_STATE_MASK,   /* u16 */
+   TCA_FLOWER_KEY_CT_ZONE, /* u16 */
+   TCA_FLOWER_KEY_CT_ZONE_MASK,/* u16 */
+   TCA_FLOWER_KEY_CT_MARK, /* u32 */
+   TCA_FLOWER_KEY_CT_MARK_MASK,/* u32 */
+   TCA_FLOWER_KEY_CT_LABELS,   /* u128 */
+   TCA_FLOWER_KEY_CT_LABELS_MASK,  /* u128 */
+
__TCA_FLOWER_MAX,
 };
 
+#define TCA_FLOWER_MAX (__TCA_FLOWER_MAX - 1)
+
+enum {
+   TCA_FLOWER_KEY_CT_FLAGS_NEW = 1 << 0, /* Beginning of a new connection. 
*/
+   TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED = 1 << 1, /* Part of an existing 
connection. */
+   TCA_FLOWER_KEY_CT_FLAGS_RELATED = 1 << 2, /* Related to an established 
connection. */
+   TCA_FLOWER_KEY_CT_FLAGS_TRACKED = 1 << 3, /* Conntrack has occurred. */
+};
+
 enum {
TCA_FLOWER_KEY_ENC_OPTS_UNSPEC,
TCA_FLOWER_KEY_ENC_OPTS_GENEVE, /* Nested
diff --git a/include/linux/tc_act/tc_ct.h b/include/linux/tc_act/tc_ct.h
new file mode 100644
index 000..5fb1d7a
--- /dev/null
+++ b/include/linux/tc_act/tc_ct.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef __UAPI_TC_CT_H
+#define __UAPI_TC_CT_H
+
+#include 
+#include 
+
+enum {
+   TCA_CT_UNSPEC,
+   TCA_CT_PARMS,
+   TCA_CT_TM,
+   TCA_CT_ACTION,  /* u16 */
+   TCA_CT_ZONE,/* u16 */
+   TCA_CT_MARK,/* u32 */
+   TCA_CT_MARK_MASK,   /* u32 */
+   TCA_CT_LABELS,  /* u128 */
+   TCA_CT_LABELS_MASK, /* u128 */
+   TCA_CT_NAT_IPV4_MIN,/* be32 */
+   TCA_CT_NAT_IPV4_MAX,/* be32 */
+   TCA_CT_NAT_IPV6_MIN,/* struct in6_addr */
+   TCA_CT_NAT_IPV6_MAX,/* struct in6_addr */
+   TCA_CT_NAT_PORT_MIN,/* be16 */
+   TCA_CT_NAT_PORT_MAX,/* be16 */
+   TCA_CT_PAD,
+   __TCA_CT_MAX
+};
+
+#define TCA_CT_MAX (__TCA_CT_MAX - 1)
+
+#define TCA_CT_ACT_COMMIT  (1 << 0)
+#define TCA_CT_ACT_FORCE   (1 << 1)
+#define TCA_CT_ACT_CLEAR   (1 << 2)
+#define TCA_CT_ACT_NAT (1 << 3)
+#define TCA_CT_ACT_NAT_SRC (1 << 4)
+#define TCA_CT_ACT_NAT_DST (1 << 5)
+
+struct tc_ct {
+   tc_gen;
+};
+
+#endif /* __UAPI_TC_CT_H */
-- 
1.8.3.1

[ovs-dev] [PATCH v4 05/10] dpif: Add support to set user features

[Paul Blakey]

This enables user features on the kernel datapath via the DP_CMD_SET
command, and also retrieves them to check for actual support and
not just an older kernel ignoring the requested features.

This will be used in next patch to enable recirc_id sharing with tc.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:

V3->V4:
Removed dpif pointer passing in offload info (not needed, and
compilation issue fix)
V2->V3:
Refactored commit, to move it earlier
Renamed commit from "netdev-offloads-tc: Probe recirc tc sharing feature on 
first recirc_id rule"
---
 datapath/linux/compat/include/linux/openvswitch.h |  3 ++
 lib/dpif-netdev.c |  1 +
 lib/dpif-netlink.c| 52 +--
 lib/dpif-provider.h   |  2 +
 lib/dpif.c|  9 
 lib/dpif.h|  2 +
 6 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 778827f..b9a7faa 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -143,6 +143,9 @@ struct ovs_vport_stats {
 /* Allow datapath to associate multiple Netlink PIDs to each vport */
 #define OVS_DP_F_VPORT_PIDS(1 << 1)
 
+/* Allow tc offload recirc sharing */
+#define OVS_DP_F_TC_RECIRC_SHARING  (1 << 2)
+
 /* Fixed logical ports. */
 #define OVSP_LOCAL  ((__u32)0)
 
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 3f21211..fd8280a 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7627,6 +7627,7 @@ const struct dpif_class dpif_netdev_class = {
 dpif_netdev_run,
 dpif_netdev_wait,
 dpif_netdev_get_stats,
+NULL,  /* set_features */
 dpif_netdev_port_add,
 dpif_netdev_port_del,
 dpif_netdev_port_set_config,
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index e9a6887..ef06dd4 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -193,6 +193,7 @@ struct dpif_handler {
 struct dpif_netlink {
 struct dpif dpif;
 int dp_ifindex;
+uint32_t user_features;
 
 /* Upcall messages. */
 struct fat_rwlock upcall_lock;
@@ -334,15 +335,26 @@ dpif_netlink_open(const struct dpif_class *class 
OVS_UNUSED, const char *name,
 
 /* Create or look up datapath. */
 dpif_netlink_dp_init(_request);
+upcall_pid = 0;
+dp_request.upcall_pid = _pid;
+dp_request.name = name;
+
 if (create) {
 dp_request.cmd = OVS_DP_CMD_NEW;
-upcall_pid = 0;
-dp_request.upcall_pid = _pid;
 } else {
+dp_request.cmd = OVS_DP_CMD_GET;
+
+error = dpif_netlink_dp_transact(_request, , );
+if (error) {
+return error;
+}
+dp_request.user_features = dp.user_features;
+ofpbuf_delete(buf);
+
 /* Use OVS_DP_CMD_SET to report user features */
 dp_request.cmd = OVS_DP_CMD_SET;
 }
-dp_request.name = name;
+
 dp_request.user_features |= OVS_DP_F_UNALIGNED;
 dp_request.user_features |= OVS_DP_F_VPORT_PIDS;
 error = dpif_netlink_dp_transact(_request, , );
@@ -368,6 +380,7 @@ open_dpif(const struct dpif_netlink_dp *dp, struct dpif 
**dpifp)
   dp->dp_ifindex, dp->dp_ifindex);
 
 dpif->dp_ifindex = dp->dp_ifindex;
+dpif->user_features = dp->user_features;
 *dpifp = >dpif;
 
 return 0;
@@ -664,6 +677,31 @@ dpif_netlink_get_stats(const struct dpif *dpif_, struct 
dpif_dp_stats *stats)
 return error;
 }
 
+static int
+dpif_netlink_set_features(struct dpif *dpif_, uint32_t new_features)
+{
+struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
+struct dpif_netlink_dp request, reply;
+struct ofpbuf *bufp;
+int error;
+
+dpif_netlink_dp_init();
+request.cmd = OVS_DP_CMD_SET;
+request.dp_ifindex = dpif->dp_ifindex;
+request.user_features = dpif->user_features | new_features;
+
+error = dpif_netlink_dp_transact(, , );
+if (!error) {
+dpif->user_features = reply.user_features;
+ofpbuf_delete(bufp);
+if (!(dpif->user_features & new_features)) {
+return -EOPNOTSUPP;
+}
+}
+
+return error;
+}
+
 static const char *
 get_vport_type(const struct dpif_netlink_vport *vport)
 {
@@ -3885,6 +3923,7 @@ const struct dpif_class dpif_netlink_class = {
 dpif_netlink_run,
 NULL,   /* wait */
 dpif_netlink_get_stats,
+dpif_netlink_set_features,
 dpif_netlink_port_add,
 dpif_netlink_port_del,
 NULL,   /* port_set_config */
@@ -4202,6 +4241,9 @@ dpif_netlink_dp_from_ofpbuf(struct dpif_netlink_dp *dp, 
const struct ofpbuf *buf
 [OVS_DP_ATTR_MEGAFLOW_STATS] = {
 NL_POLICY_FOR(struct ovs_dp_megaflow_stats),
 .optional = true 

[ovs-dev] [PATCH v4 10/10] netdev-offload-tc: Add conntrack nat support

[Paul Blakey]

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:
V2->V3:
Ipv6 nat dump fix (missing unspec type in policy)
Renamed tc range struct to be consistent with NL ATTRIBUTES and ovs

V1->V2:
Missing ntohs/htons on nat port range.
---
 lib/netdev-offload-tc.c | 104 
 lib/tc.c|  93 +++
 lib/tc.h|  28 +
 3 files changed, 225 insertions(+)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index e7c3463..66cd45b 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -815,6 +815,40 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 ct_label->mask = action->ct.label_mask;
 }
 
+if (action->ct.nat_type) {
+size_t nat_offset = nl_msg_start_nested(buf,
+OVS_CT_ATTR_NAT);
+
+if (action->ct.nat_type == TC_NAT_SRC) {
+nl_msg_put_flag(buf, OVS_NAT_ATTR_SRC);
+} else if (action->ct.nat_type == TC_NAT_DST) {
+nl_msg_put_flag(buf, OVS_NAT_ATTR_DST);
+}
+
+if (action->ct.range.ip_family == AF_INET) {
+nl_msg_put_be32(buf, OVS_NAT_ATTR_IP_MIN,
+action->ct.range.ipv4.min);
+nl_msg_put_be32(buf, OVS_NAT_ATTR_IP_MAX,
+action->ct.range.ipv4.max);
+} else if (action->ct.range.ip_family == AF_INET6) {
+nl_msg_put_in6_addr(buf, OVS_NAT_ATTR_IP_MIN,
+>ct.range.ipv6.min);
+nl_msg_put_in6_addr(buf, OVS_NAT_ATTR_IP_MAX,
+>ct.range.ipv6.max);
+}
+
+if (action->ct.range.port.min) {
+nl_msg_put_u16(buf, OVS_NAT_ATTR_PROTO_MIN,
+   ntohs(action->ct.range.port.min));
+if (action->ct.range.port.max) {
+nl_msg_put_u16(buf, OVS_NAT_ATTR_PROTO_MAX,
+   ntohs(action->ct.range.port.max));
+}
+}
+
+nl_msg_end_nested(buf, nat_offset);
+}
+
 nl_msg_end_nested(buf, ct_offset);
 }
 break;
@@ -907,6 +941,66 @@ parse_mpls_set_action(struct tc_flower *flower, struct 
tc_action *action,
 }
 
 static int
+parse_put_flow_nat_action(struct tc_action *action,
+  const struct nlattr *nat,
+  size_t nat_len)
+{
+const struct nlattr *nat_attr;
+size_t nat_left;
+
+action->ct.nat_type = TC_NAT_RESTORE;
+NL_ATTR_FOR_EACH_UNSAFE (nat_attr, nat_left, nat, nat_len) {
+switch (nl_attr_type(nat_attr)) {
+case OVS_NAT_ATTR_SRC: {
+action->ct.nat_type = TC_NAT_SRC;
+};
+break;
+case OVS_NAT_ATTR_DST: {
+action->ct.nat_type = TC_NAT_DST;
+};
+break;
+case OVS_NAT_ATTR_IP_MIN: {
+if (nl_attr_get_size(nat_attr) == sizeof(ovs_be32)) {
+ovs_be32 addr = nl_attr_get_be32(nat_attr);
+
+action->ct.range.ipv4.min = addr;
+action->ct.range.ip_family = AF_INET;
+} else {
+struct in6_addr addr = nl_attr_get_in6_addr(nat_attr);
+
+action->ct.range.ipv6.min = addr;
+action->ct.range.ip_family = AF_INET6;
+}
+};
+break;
+case OVS_NAT_ATTR_IP_MAX: {
+if (nl_attr_get_size(nat_attr) == sizeof(ovs_be32)) {
+ovs_be32 addr = nl_attr_get_be32(nat_attr);
+
+action->ct.range.ipv4.max = addr;
+action->ct.range.ip_family = AF_INET;
+} else {
+struct in6_addr addr = nl_attr_get_in6_addr(nat_attr);
+
+action->ct.range.ipv6.max = addr;
+action->ct.range.ip_family = AF_INET6;
+}
+};
+break;
+case OVS_NAT_ATTR_PROTO_MIN: {
+action->ct.range.port.min = htons(nl_attr_get_u16(nat_attr));
+};
+break;
+case OVS_NAT_ATTR_PROTO_MAX: {
+action->ct.range.port.max = htons(nl_attr_get_u16(nat_attr));
+};
+break;
+}
+}
+return 0;
+}
+
+static int
 parse_put_flow_ct_action(struct tc_flower *flower,
  struct tc_action *action,
  const struct nlattr 

[ovs-dev] [PATCH v4 04/10] netdev-offload-tc: Implement netdev tc flush via tc filter del

[Paul Blakey]

To be consistent with our tc-ufid mapping after flush, and to support tc
chains flushing in the next commit, implement flush operation via
deleting all the filters we actually added and delete their mappings.

This will also not delete the configured qos policing via matchall filters,
while old code did.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
 lib/netdev-offload-tc.c | 74 +++--
 1 file changed, 41 insertions(+), 33 deletions(-)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 460c27f..15b39e6 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -43,7 +43,8 @@ VLOG_DEFINE_THIS_MODULE(netdev_offload_tc);
 
 static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(60, 5);
 
-static struct hmap ufid_tc = HMAP_INITIALIZER(_tc);
+static struct hmap ufid_to_tc = HMAP_INITIALIZER(_to_tc);
+static struct hmap tc_to_ufid = HMAP_INITIALIZER(_to_ufid);
 static bool multi_mask_per_prio = false;
 static bool block_support = false;
 
@@ -143,44 +144,49 @@ static struct netlink_field set_flower_map[][4] = {
 static struct ovs_mutex ufid_lock = OVS_MUTEX_INITIALIZER;
 
 /**
- * struct ufid_tc_data - data entry for ufid_tc hmap.
- * @ufid_node: Element in @ufid_tc hash table by ufid key.
- * @tc_node: Element in @ufid_tc hash table by tcf_id key.
+ * struct ufid_tc_data - data entry for ufid-tc hashmaps.
+ * @ufid_to_tc_node: Element in @ufid_to_tc hash table by ufid key.
+ * @tc_to_ufid_node: Element in @tc_to_ufid hash table by tcf_id key.
  * @ufid: ufid assigned to the flow
  * @id: tc filter id (tcf_id)
  * @netdev: netdev associated with the tc rule
  */
 struct ufid_tc_data {
-struct hmap_node ufid_node;
-struct hmap_node tc_node;
+struct hmap_node ufid_to_tc_node;
+struct hmap_node tc_to_ufid_node;
 ovs_u128 ufid;
 struct tcf_id id;
 struct netdev *netdev;
 };
 
-/* Remove matching ufid entry from ufid_tc hashmap. */
 static void
-del_ufid_tc_mapping(const ovs_u128 *ufid)
+del_ufid_tc_mapping_unlocked(const ovs_u128 *ufid)
 {
 size_t ufid_hash = hash_bytes(ufid, sizeof *ufid, 0);
 struct ufid_tc_data *data;
 
-ovs_mutex_lock(_lock);
-HMAP_FOR_EACH_WITH_HASH(data, ufid_node, ufid_hash, _tc) {
+HMAP_FOR_EACH_WITH_HASH (data, ufid_to_tc_node, ufid_hash, _to_tc) {
 if (ovs_u128_equals(*ufid, data->ufid)) {
 break;
 }
 }
 
 if (!data) {
-ovs_mutex_unlock(_lock);
 return;
 }
 
-hmap_remove(_tc, >ufid_node);
-hmap_remove(_tc, >tc_node);
+hmap_remove(_to_tc, >ufid_to_tc_node);
+hmap_remove(_to_ufid, >tc_to_ufid_node);
 netdev_close(data->netdev);
 free(data);
+}
+
+/* Remove matching ufid entry from ufid-tc hashmaps. */
+static void
+del_ufid_tc_mapping(const ovs_u128 *ufid)
+{
+ovs_mutex_lock(_lock);
+del_ufid_tc_mapping_unlocked(ufid);
 ovs_mutex_unlock(_lock);
 }
 
@@ -195,7 +201,7 @@ del_filter_and_ufid_mapping(struct tcf_id *id, const 
ovs_u128 *ufid)
 return err;
 }
 
-/* Add ufid entry to ufid_tc hashmap. */
+/* Add ufid entry to ufid_to_tc hashmap. */
 static void
 add_ufid_tc_mapping(struct netdev *netdev, const ovs_u128 *ufid,
 struct tcf_id *id)
@@ -209,12 +215,12 @@ add_ufid_tc_mapping(struct netdev *netdev, const ovs_u128 
*ufid,
 new_data->netdev = netdev_ref(netdev);
 
 ovs_mutex_lock(_lock);
-hmap_insert(_tc, _data->ufid_node, ufid_hash);
-hmap_insert(_tc, _data->tc_node, tc_hash);
+hmap_insert(_to_tc, _data->ufid_to_tc_node, ufid_hash);
+hmap_insert(_to_ufid, _data->tc_to_ufid_node, tc_hash);
 ovs_mutex_unlock(_lock);
 }
 
-/* Get tc id from ufid_tc hashmap.
+/* Get tc id from ufid_to_tc hashmap.
  *
  * Returns 0 if successful and fills id.
  * Otherwise returns the error.
@@ -226,7 +232,7 @@ get_ufid_tc_mapping(const ovs_u128 *ufid, struct tcf_id *id)
 struct ufid_tc_data *data;
 
 ovs_mutex_lock(_lock);
-HMAP_FOR_EACH_WITH_HASH(data, ufid_node, ufid_hash, _tc) {
+HMAP_FOR_EACH_WITH_HASH (data, ufid_to_tc_node, ufid_hash, _to_tc) {
 if (ovs_u128_equals(*ufid, data->ufid)) {
 *id = data->id;
 ovs_mutex_unlock(_lock);
@@ -238,7 +244,7 @@ get_ufid_tc_mapping(const ovs_u128 *ufid, struct tcf_id *id)
 return ENOENT;
 }
 
-/* Find ufid entry in ufid_tc hashmap using tcf_id id.
+/* Find ufid entry in ufid_to_tc hashmap using tcf_id id.
  * The result is saved in ufid.
  *
  * Returns true on success.
@@ -250,7 +256,7 @@ find_ufid(struct netdev *netdev, struct tcf_id *id, 
ovs_u128 *ufid)
 struct ufid_tc_data *data;
 
 ovs_mutex_lock(_lock);
-HMAP_FOR_EACH_WITH_HASH(data, tc_node, tc_hash,  _tc) {
+HMAP_FOR_EACH_WITH_HASH (data, tc_to_ufid_node, tc_hash,  _to_ufid) {
 if (netdev == data->netdev && is_tcf_id_eq(>id, id)) {
 *ufid = data->ufid;
 break;
@@ -293,7 +299,7 @@ get_prio_for_tc_flower(struct tc_flower *flower)
 

[ovs-dev] [PATCH v4 03/10] tc: Introduce tcf_id to specify a tc filter

[Paul Blakey]

Move all that is needed to identify a tc filter to a
new structure, tcf_id. This removes a lot of duplication
in accessing/creating tc filters.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:
V3->V4:
Fix accidently removed Block_id in flow_put()
V2->V3:
Renamed *tc_id* -> *tcf_id*
Renamed make_tc_id -> tc_make_tcf_id to be consistent with tc_make_* helpers

V1->V2:
In tc_del_matchall_policer - reverse xmas param order
Added and used helper is_tc_id_eq(id1, id2) in find_ufid
In netdev_tc_flow_dump_next - use make_tc_id() instead of manualy filling id
In netdev_tc_flow_put - use if (get_ufid_tc_mapping(ufid, ) == 0) to be 
mor explict we found the mapping not failed to get
In make_tc_id - fill id explictily and removed memset.
Moved make_tc_id to be static inline in tc.h
---
 lib/netdev-linux.c  |   6 +-
 lib/netdev-offload-tc.c | 205 
 lib/tc.c| 109 -
 lib/tc.h|  51 +---
 4 files changed, 159 insertions(+), 212 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index f8e59ba..8a62f9d 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -2356,7 +2356,9 @@ tc_add_matchall_policer(struct netdev *netdev, uint32_t 
kbits_rate,
 static int
 tc_del_matchall_policer(struct netdev *netdev)
 {
+int prio = TC_RESERVED_PRIORITY_POLICE;
 uint32_t block_id = 0;
+struct tcf_id id;
 int ifindex;
 int err;
 
@@ -2365,8 +2367,8 @@ tc_del_matchall_policer(struct netdev *netdev)
 return err;
 }
 
-err = tc_del_filter(ifindex, TC_RESERVED_PRIORITY_POLICE, 1, block_id,
-TC_INGRESS);
+id = tc_make_tcf_id(ifindex, block_id, prio, TC_INGRESS);
+err = tc_del_filter();
 if (err) {
 return err;
 }
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 1adbb32..460c27f 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -145,20 +145,16 @@ static struct ovs_mutex ufid_lock = OVS_MUTEX_INITIALIZER;
 /**
  * struct ufid_tc_data - data entry for ufid_tc hmap.
  * @ufid_node: Element in @ufid_tc hash table by ufid key.
- * @tc_node: Element in @ufid_tc hash table by prio/handle/ifindex key.
+ * @tc_node: Element in @ufid_tc hash table by tcf_id key.
  * @ufid: ufid assigned to the flow
- * @prio: tc priority
- * @handle: tc handle
- * @ifindex: netdev ifindex.
+ * @id: tc filter id (tcf_id)
  * @netdev: netdev associated with the tc rule
  */
 struct ufid_tc_data {
 struct hmap_node ufid_node;
 struct hmap_node tc_node;
 ovs_u128 ufid;
-uint16_t prio;
-uint32_t handle;
-int ifindex;
+struct tcf_id id;
 struct netdev *netdev;
 };
 
@@ -190,32 +186,27 @@ del_ufid_tc_mapping(const ovs_u128 *ufid)
 
 /* Wrapper function to delete filter and ufid tc mapping */
 static int
-del_filter_and_ufid_mapping(int ifindex, int prio, int handle,
-uint32_t block_id, const ovs_u128 *ufid,
-enum tc_qdisc_hook hook)
+del_filter_and_ufid_mapping(struct tcf_id *id, const ovs_u128 *ufid)
 {
 int err;
 
-err = tc_del_filter(ifindex, prio, handle, block_id, hook);
+err = tc_del_filter(id);
 del_ufid_tc_mapping(ufid);
-
 return err;
 }
 
 /* Add ufid entry to ufid_tc hashmap. */
 static void
-add_ufid_tc_mapping(const ovs_u128 *ufid, int prio, int handle,
-struct netdev *netdev, int ifindex)
+add_ufid_tc_mapping(struct netdev *netdev, const ovs_u128 *ufid,
+struct tcf_id *id)
 {
 size_t ufid_hash = hash_bytes(ufid, sizeof *ufid, 0);
-size_t tc_hash = hash_int(hash_int(prio, handle), ifindex);
+size_t tc_hash = hash_int(hash_int(id->prio, id->handle), id->ifindex);
 struct ufid_tc_data *new_data = xzalloc(sizeof *new_data);
 
 new_data->ufid = *ufid;
-new_data->prio = prio;
-new_data->handle = handle;
+new_data->id = *id;
 new_data->netdev = netdev_ref(netdev);
-new_data->ifindex = ifindex;
 
 ovs_mutex_lock(_lock);
 hmap_insert(_tc, _data->ufid_node, ufid_hash);
@@ -223,56 +214,44 @@ add_ufid_tc_mapping(const ovs_u128 *ufid, int prio, int 
handle,
 ovs_mutex_unlock(_lock);
 }
 
-/* Get ufid from ufid_tc hashmap.
+/* Get tc id from ufid_tc hashmap.
  *
- * If netdev output param is not NULL then the function will return
- * associated netdev on success and a refcount is taken on that netdev.
- * The caller is then responsible to close the netdev.
- *
- * Returns handle if successful and fill prio and netdev for that ufid.
- * Otherwise returns 0.
+ * Returns 0 if successful and fills id.
+ * Otherwise returns the error.
  */
 static int
-get_ufid_tc_mapping(const ovs_u128 *ufid, int *prio, struct netdev **netdev)
+get_ufid_tc_mapping(const ovs_u128 *ufid, struct tcf_id *id)
 {
 size_t ufid_hash = hash_bytes(ufid, sizeof *ufid, 0);
 struct ufid_tc_data 

[ovs-dev] [PATCH v4 01/10] match: Add match_set_ct_zone_masked helper

[Paul Blakey]

Sets zone in match.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
 include/openvswitch/match.h |  2 ++
 lib/match.c | 10 --
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
index 05ecee7..eeabd5f 100644
--- a/include/openvswitch/match.h
+++ b/include/openvswitch/match.h
@@ -127,6 +127,8 @@ void match_set_pkt_mark_masked(struct match *, uint32_t 
pkt_mark, uint32_t mask)
 void match_set_ct_state(struct match *, uint32_t ct_state);
 void match_set_ct_state_masked(struct match *, uint32_t ct_state, uint32_t 
mask);
 void match_set_ct_zone(struct match *, uint16_t ct_zone);
+void match_set_ct_zone_masked(struct match *match, uint16_t ct_zone,
+  uint16_t mask);
 void match_set_ct_mark(struct match *, uint32_t ct_mark);
 void match_set_ct_mark_masked(struct match *, uint32_t ct_mark, uint32_t mask);
 void match_set_ct_label(struct match *, ovs_u128 ct_label);
diff --git a/lib/match.c b/lib/match.c
index ae56828..0d1ec31 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -417,8 +417,14 @@ match_set_ct_state_masked(struct match *match, uint32_t 
ct_state, uint32_t mask)
 void
 match_set_ct_zone(struct match *match, uint16_t ct_zone)
 {
-match->flow.ct_zone = ct_zone;
-match->wc.masks.ct_zone = UINT16_MAX;
+match_set_ct_zone_masked(match, ct_zone, UINT16_MAX);
+}
+
+void
+match_set_ct_zone_masked(struct match *match, uint16_t ct_zone, uint16_t mask)
+{
+match->flow.ct_zone = ct_zone & mask;
+match->wc.masks.ct_zone = mask;
 }
 
 void
-- 
1.8.3.1

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

[ovs-dev] [PATCH v4 09/10] netdev-offload-tc: Add conntrack label and mark support

[Paul Blakey]

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

---

Changelog:
V2->V3:
Added missing match on ct_mark.
---
 lib/dpif-netlink.c  |  2 ++
 lib/netdev-offload-tc.c | 66 +
 lib/tc.c| 53 +++
 lib/tc.h|  6 +
 4 files changed, 117 insertions(+), 10 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index c9dd193..483cafd 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1653,6 +1653,8 @@ dpif_netlink_netdev_match_to_dpif_flow(struct match 
*match,
 .recirc = true,
 .ct_state = true,
 .ct_zone = true,
+.ct_mark = true,
+.ct_label = true,
 },
 };
 size_t offset;
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 7a346e3..e7c3463 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -624,6 +624,8 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 }
 
 match_set_ct_zone_masked(match, key->ct_zone, mask->ct_zone);
+match_set_ct_mark_masked(match, key->ct_mark, mask->ct_mark);
+match_set_ct_label_masked(match, key->ct_label, mask->ct_label);
 }
 
 if (flower->tunnel) {
@@ -793,6 +795,26 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 nl_msg_put_u16(buf, OVS_CT_ATTR_ZONE, action->ct.zone);
 }
 
+if (action->ct.mark_mask) {
+uint32_t mark_and_mask[2] = { action->ct.mark,
+  action->ct.mark_mask };
+nl_msg_put_unspec(buf, OVS_CT_ATTR_MARK, _and_mask,
+  sizeof mark_and_mask);
+}
+
+if (!ovs_u128_is_zero(action->ct.label_mask)) {
+struct {
+ovs_u128 key;
+ovs_u128 mask;
+} *ct_label;
+
+ct_label = nl_msg_put_unspec_uninit(buf,
+OVS_CT_ATTR_LABELS,
+sizeof *ct_label);
+ct_label->key = action->ct.label;
+ct_label->mask = action->ct.label_mask;
+}
+
 nl_msg_end_nested(buf, ct_offset);
 }
 break;
@@ -904,6 +926,28 @@ parse_put_flow_ct_action(struct tc_flower *flower,
 action->ct.zone = nl_attr_get_u16(ct_attr);
 }
 break;
+case OVS_CT_ATTR_MARK: {
+const struct {
+uint32_t key;
+uint32_t mask;
+} *ct_mark;
+
+ct_mark = nl_attr_get_unspec(ct_attr, sizeof *ct_mark);
+action->ct.mark = ct_mark->key;
+action->ct.mark_mask = ct_mark->mask;
+}
+break;
+case OVS_CT_ATTR_LABELS: {
+const struct {
+ovs_u128 key;
+ovs_u128 mask;
+} *ct_label;
+
+ct_label = nl_attr_get_unspec(ct_attr, sizeof *ct_label);
+action->ct.label = ct_label->key;
+action->ct.label_mask = ct_label->mask;
+}
+break;
 }
 }
 
@@ -1094,22 +1138,12 @@ test_key_and_mask(struct match *match)
 return EOPNOTSUPP;
 }
 
-if (mask->ct_mark) {
-VLOG_DBG_RL(, "offloading attribute ct_mark isn't supported");
-return EOPNOTSUPP;
-}
-
 if (mask->packet_type && key->packet_type) {
 VLOG_DBG_RL(, "offloading attribute packet_type isn't supported");
 return EOPNOTSUPP;
 }
 mask->packet_type = 0;
 
-if (!ovs_u128_is_zero(mask->ct_label)) {
-VLOG_DBG_RL(, "offloading attribute ct_label isn't supported");
-return EOPNOTSUPP;
-}
-
 for (int i = 0; i < FLOW_N_REGS; i++) {
 if (mask->regs[i]) {
 VLOG_DBG_RL(,
@@ -1468,6 +1502,18 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 mask->ct_zone = 0;
 }
 
+if (mask->ct_mark) {
+flower.key.ct_mark = key->ct_mark;
+flower.mask.ct_mark = mask->ct_mark;
+mask->ct_mark = 0;
+}
+
+if (!ovs_u128_is_zero(mask->ct_label)) {
+flower.key.ct_label = key->ct_label;
+flower.mask.ct_label = mask->ct_label;
+mask->ct_label = OVS_U128_ZERO;
+}
+
 err = test_key_and_mask(match);
 if (err) {
 return err;
diff --git a/lib/tc.c b/lib/tc.c
index 4358cb7..de09c49 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -404,6 +404,11 @@ static const struct nl_policy tca_flower_policy[] = {
 [TCA_FLOWER_KEY_CT_STATE_MASK] = { .type = NL_A_U16, .optional = true, },
 

[ovs-dev] Reply

[Barrister. Adolf Robert]

Greetings Dear,

I write to know if you receive my previous email? I wait to hear from you
as a matter of urgency.

Best Regards,
Robert Adolf.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 07/10] netdev-offload-tc: Add recirculation, support via tc chains

[Ilya Maximets]

On 15.12.2019 14:35, Paul Blakey wrote:
> 
> On 12/13/2019 2:05 PM, Ilya Maximets wrote:
>>> Hi sorry for late reply, didn't get this email.
>>>
>>> On 03.12.2019 16:16, Ilya Maximets wrote:
>>>   > From: Ilya Maximets 
>>>   > On 03.12.2019 14:45, Roi Dayan wrote:
>>>   > > From: Paul Blakey 
>>>   > >
> 
> [...]
> 
>>>
>>> We are calling dpif_set_features, not accessing dpif internals.
>> Not here, but you're dereferencing dpif pointer later producing the build
>> failure:
>> lib/netdev-offload-tc.c:1371:64: error: using member 'dpif_class' in 
>> incomplete struct dpif
>>
>>> The thing is, that this sets a feature and not just probes for a feature.
>>>
>>> It enables a static branch on the kernel side which we might not want to
>>> enable any time,
>> Does it have any significant performance impact?  Looking at the kernel
>> code I don't see any special heavy operations.
> 
> I don't see any heavy operations as well, it was just suggested by 
> mailing list,
> 
> We just lookup the skb extension on vport recv. For normal packets this 
> will just incur a simple inline check of skb_ext_find (which just does 
> 'return skb->active_extensions & (1 << id)'),
> 
> which will return false.
> 
> 
>>
>>> and an offloading a recirc() action was our hook to know that this is
>>> wanted, as we talked
>>>
>>> about in the kernel patch.
>> I do not know what you've talked about while upstreaming kernel parts,
>> but current patch-set for OVS doesn't work for several reasons:
>>
>> 1. Obviously, it doesn't build successfully, because netdev-offload
>> implementation now requires access to the internals of 'struct dpif'.
> Right I will check that.
>>
>> 2. You're using ovsthread_once to not enable feature twice and this will
>> break CT offloading if you'll remove datapath and create it back.  You
>> will not be able to enable feature for the newly created datapath.
> 
> Thanks, will fix that.
> 
>>
>> 3. offloading module doesn't depend on datapath and could be theoretically
>> used from the userspace datapath at the same time and if userspace
>> datapath will reach feature enabling code first it will try to set
>> datapath features, will fail and kernel datapath will never try to
>> enable feature because of the same ovsthread_once.
>>
>> So, taking above issues into account, feature enabling should definitely
>> happen on datapath level and might or might not be controlled by the
>> ofproto.
>>
>>> I'm not sure how to do that from ofproto layer.
>> We may not involve the ofproto layer.  For example, you may try to enable
>> the feature in dpif_netlink_open() and fallback if not supported.  While
>> doing that you may remember the status of this feature and then send the
>> status to netdev-offload module via 'struct offload_info' or check directly
>> by implementing dpif_get_[user_]features().
> 
> 
> As the performance impact is very minor,
> 
> I'm for enabling/querying this feature at the start (probably via 
> dpif_set_features) after a successfully creation in dpif_netlink_open of 
> dpif-netlink,
> 
> will that be ok with you?

I'm not against enabling by default if it doesn't cause performance degradation.
Just be sure that we could fall back if the feature is not supported by kernel.

BTW, while offloading it might be better to use 'struct offload_info' to pass
the feature support status.  This way you'll not need to operate 
with/dereference
dpif pointers.

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

Re: [ovs-dev] [PATCH] netdev-afxdp: Add pcap dump support.

[Ilya Maximets]

On 13.12.2019 21:59, William Tu wrote:
> Debugging netdev-afxdp is hard because tcpdump does not work
> at all, even for generic mode.  ovs-tcpdump which uses port
> mirroring also does not work for netdev-afxdp.

Hmm.  Why ovs-tcpdump doesn't work for you?  It should not depend
on port type.  If it doesn't work we need to investigate this
case and fix it because it's a very important tool.

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

Re: [ovs-dev] [PATCH v17] Improved Packet Drop Statistics in OVS

[Ilya Maximets]

On 16.12.2019 05:07, Anju Thomas wrote:
> Thanks for the review Ilya,
> 
> For the below comment 
> 
> 
>> @@ -7086,10 +7105,16 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
>> *packets_,
>>   * the ownership of these packets. Thus, we can avoid performing
>>   * the action, because the caller will not use the result 
>> anyway.
>>   * Just break to free the batch. */
>> +COVERAGE_ADD(datapath_drop_tunnel_push_error,
>> + dp_packet_batch_size(packets_));
> This is not a tunnel push error.  We literally executed all the actions 
> without errors and that is not our fault that there are no more actions after 
> tnl_push.  We're just saving some time avoiding actual execution of a 
> pointless action.  So, this should not be accounted as error, and definitely 
> not as a tunnel push error.
> 
> I agree we need a better name .Does datapath_drop_incomplete_dp_action sound 
> good or do you have any other suggestions in mind?

datapath_drop_last_action_tnl_push ?

And we actually could just avoid counting in this case because:

1. It's a normal scenario of action execution.  If the code looked like this:

case OVS_ACTION_ATTR_TUNNEL_PUSH:
dp_packet_batch_apply_cutlen(packets_);
push_tnl_action(pmd, a, packets_)
break;

  you most probably wouldn't noticed this case because it's perfectly normal.

2. We're not tracking similar cases for all other actions.  What if push_vlan,
   set_ipv4_src or meter is the last action?

The only reason why we have a spacial case with big comment for tunnel push
is that we had dp_packet leak here and we don't really want to execute heavy
push action if we're going to drop these packets anyway.

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

Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action

[Sriharsha Basavapatna via dev]

On Sun, Dec 8, 2019 at 3:08 PM Eli Britstein  wrote:
>
>
> On 12/5/2019 6:46 PM, Sriharsha Basavapatna wrote:
> > On Wed, Dec 4, 2019 at 8:55 PM Eli Britstein  wrote:
> >>
> >> On 12/3/2019 5:19 PM, Sriharsha Basavapatna wrote:
> >>> On Wed, Nov 20, 2019 at 9:07 PM Eli Britstein  wrote:
>  Signed-off-by: Eli Britstein 
>  Reviewed-by: Oz Shlomo 
>  ---
> NEWS   |  2 +
> lib/netdev-offload-dpdk-flow.c | 87 
>  --
> 2 files changed, 85 insertions(+), 4 deletions(-)
> 
>  diff --git a/NEWS b/NEWS
>  index 88b818948..ca9c2b230 100644
>  --- a/NEWS
>  +++ b/NEWS
>  @@ -10,6 +10,8 @@ Post-v2.12.0
>    if supported by libbpf.
>  * Add option to enable, disable and query TCP sequence checking 
>  in
>    conntrack.
>  +   - DPDK:
>  + * Add hardware offload support for output actions.
> 
> v2.12.0 - 03 Sep 2019
> -
>  diff --git a/lib/netdev-offload-dpdk-flow.c 
>  b/lib/netdev-offload-dpdk-flow.c
>  index dbbc45e99..6e7efb315 100644
>  --- a/lib/netdev-offload-dpdk-flow.c
>  +++ b/lib/netdev-offload-dpdk-flow.c
>  @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct 
>  rte_flow_action *actions)
> } else {
> ds_put_cstr(s, "  RSS = null\n");
> }
>  +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
>  +const struct rte_flow_action_count *count = actions->conf;
>  +
>  +ds_put_cstr(s, "rte flow count action:\n");
>  +if (count) {
>  +ds_put_format(s,
>  +  "  Count: shared=%d, id=%d\n",
>  +  count->shared, count->id);
>  +} else {
>  +ds_put_cstr(s, "  Count = null\n");
>  +}
>  +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
>  +const struct rte_flow_action_port_id *port_id = actions->conf;
>  +
>  +ds_put_cstr(s, "rte flow port-id action:\n");
>  +if (port_id) {
>  +ds_put_format(s,
>  +  "  Port-id: original=%d, id=%d\n",
>  +  port_id->original, port_id->id);
>  +} else {
>  +ds_put_cstr(s, "  Port-id = null\n");
>  +}
> } else {
> ds_put_format(s, "unknown rte flow action (%d)\n", 
>  actions->type);
> }
>  @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns 
>  *patterns,
> return 0;
> }
> 
>  +static void
>  +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
>  +{
>  +struct rte_flow_action_count *count = xzalloc(sizeof *count);
>  +
>  +count->shared = 0;
>  +/* Each flow has a single count action, so no need of id */
>  +count->id = 0;
>  +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
>  +}
>  +
>  +static void
>  +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
>  +struct netdev *outdev)
>  +{
>  +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
>  +
>  +port_id->id = netdev_dpdk_get_port_id(outdev);
>  +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
>  +}
>  +
>  +static int
>  +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
>  +   const struct nlattr *nla,
>  +   struct offload_info *info)
>  +{
>  +struct netdev *outdev;
>  +odp_port_t port;
>  +
>  +port = nl_attr_get_odp_port(nla);
>  +outdev = netdev_ports_get(port, info->dpif_class);
>  +if (outdev == NULL) {
>  +VLOG_DBG_RL(_rl,
>  +"Cannot find netdev for odp port %d", port);
>  +return -1;
>  +}
>  +if (!netdev_dpdk_flow_api_supported(outdev)) {
>  +VLOG_DBG_RL(_rl,
>  +"Output to %s cannot be offloaded",
>  +netdev_get_name(outdev));
>  +return -1;
>  +}
>  +
>  +netdev_dpdk_flow_add_count_action(actions);
>  +netdev_dpdk_flow_add_port_id_action(actions, outdev);
>  +netdev_close(outdev);
>  +
>  +return 0;
>  +}
>  +
> int
> netdev_dpdk_flow_actions_add(struct flow_actions *actions,
>  struct nlattr *nl_actions,
>  size_t nl_actions_len,
>  - struct offload_info *info OVS_UNUSED)
>  + struct