[ovs-dev] [PATCH v2] dpif-netlink: convert packet_type netlink attribute

Yang, Yi Y yi.y.yang at intel.com
Fri Jul 7 02:16:12 UTC 2017


Eric, I verified this one, ptap unit tests and nsh tests passed without any change, so it seems it is better than the original one you did.

-----Original Message-----
From: Eric Garver [mailto:e at erig.me] 
Sent: Friday, July 7, 2017 3:44 AM
To: Zoltán Balogh <zoltan.balogh at ericsson.com>
Cc: Yang, Yi Y <yi.y.yang at intel.com>; 'dev at openvswitch.org' <dev at openvswitch.org>
Subject: Re: [ovs-dev] [PATCH v2] dpif-netlink: convert packet_type netlink attribute

On Thu, Jul 06, 2017 at 01:14:31PM +0000, Zoltán Balogh wrote:
> Hi Eric,
> 
> Thank you for clarifying. 
> 
>     ...
>     [OVS_FLOW_ATTR_KEY]
>         ...
>         [OVS_KEY_ATTR_IPV4] = ...
>         ...
>     [OVS_KEY_ATTR_ETHERTYPE] = ...  <--- the one inserted
> 
> I have not found any API that would extend a nested attribute. 
> Maybe I'm wrong, but I thought 'buf' holds a single nested attribute. If this is true, isn't possible to add the ethertype to it and increase its size?

I gave it a try and realized it doesn't matter.

During the upcall we pass back the same key that the kernel gave us, with the exception that we also fill in wildcard info. See
upcall_xlate() and ukey_create_from_upcall(). This is why my original patch works. It caused odp_flow_key_from_mask() to fill the ETHERTYPE for the mask.

So I think this can only be solved in one of two places:

  1) odp_flow_key_from_mask(), as my original patch did.
    - This sets the wildcard for all dpifs, including userspace/netdev.
  2) xlate_wc_init() and xlate_wc_finish().
    - Here we can limit by dpif type.

Neither which is particularly pleasing. I gave #2 a try. see patch below. It works for both check-kernel and check-system-userspace. It does not force eth_type() for userspace.

What do you think?

Eric.

--->8---

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 1f4fe1dd6725..f3074c78c4b2 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -6129,7 +6129,10 @@ xlate_wc_init(struct xlate_ctx *ctx)
     /* Some fields we consider to always be examined. */
     WC_MASK_FIELD(ctx->wc, packet_type);
     WC_MASK_FIELD(ctx->wc, in_port);
-    if (is_ethernet(&ctx->xin->flow, NULL)) {
+    if (is_ethernet(&ctx->xin->flow, NULL)
+        || (pt_ns(ctx->xin->flow.packet_type) == OFPHTN_ETHERTYPE
+            && !strcmp(dpif_normalize_type(dpif_type(ctx->xbridge->dpif)),
+                       "system"))) {
         WC_MASK_FIELD(ctx->wc, dl_type);
     }
     if (is_ip_any(&ctx->xin->flow)) {
@@ -6163,7 +6166,12 @@ xlate_wc_finish(struct xlate_ctx *ctx)
     if (ctx->xin->upcall_flow->packet_type != htonl(PT_ETH)) {
         ctx->wc->masks.dl_dst = eth_addr_zero;
         ctx->wc->masks.dl_src = eth_addr_zero;
-        ctx->wc->masks.dl_type = 0;
+        /* Kernel uses ETHERTYPE to implicitly mean packet_type(1, x) */
+        if (pt_ns(ctx->xin->upcall_flow->packet_type) != OFPHTN_ETHERTYPE
+            || strcmp(dpif_normalize_type(dpif_type(ctx->xbridge->dpif)),
+                      "system")) {
+            ctx->wc->masks.dl_type = 0;
+        }
     }

     /* ICMPv4 and ICMPv6 have 8-bit "type" and "code" fields.  struct flow


More information about the dev mailing list