diff mbox series

[ovs-dev,v1] Remove checking ICMPv6 Neighbor Discovery feature for datapath

Message ID CAEEdnKHZpAAf0fWX2SvWiay=7iefC4iarhhNoRGVzsAnQUqsUw@mail.gmail.com
State New
Headers show
Series [ovs-dev,v1] Remove checking ICMPv6 Neighbor Discovery feature for datapath | expand

Commit Message

范开喜 Aug. 16, 2020, 3:03 a.m. UTC
Currently OVS supports setting and matching ICMPv6 Neighbor Discovery
related fields: “ND option type” and “ND reserved”. So we expect that OVS
could support ICMPv6 ND Responder as the same as ARP responder.
The OVS datapath could rewrite a ICMPv6 neighbor solicitation  packet to be
a neighbor advertisement packet, and then output the packet to its ingress
port.

But when using kernel datapath, controller and ovs-ofctl tool could not
insert openflow rule matching and setting “ND option type” and “ND
reserved” fields for ICMPv6 Neighbor Advertisement packet. So the user has
to implement ICMPv6 ND Responder procedure in the controller. It's not the
same as an ARP responder. This is because that the ofproto-dpif checks
whether the datapath supports nd_extention key at the startup time. If not,
it will reject the openflow rules which modify “ND option type” and “ND
reserved” fields.

This could be removed to keep the  same behavior for ARP and ICMPv6
Neighbor Discovery procedure in OVS . Then the controller could add
openflow rules to match and modify “ND option type” and “ND reserved”
fields.  Currently kernel fast datapath could not match and modify these
two fields as the same as ARP fields. So when a  ICMPv6 neighbor
solicitation packet is received by the kernel datapath, it will not match
any flows and would be delivered to the userspace datapath. The userspace
datapath would rewrite “ND option type” and “ND reserved” fields of the
packet and translate this packet to be a ICMPv6 neighbor advertisement
packet. The packet would be injected to the kernel datapath to output to a
port.

Signed-off-by: fankaixi <fankaixi.li@bytedance.com>
---
 datapath/linux/compat/include/linux/openvswitch.h |  8 +++-----
 lib/odp-util.c                                    | 21
++++++++-------------
 lib/odp-util.h                                    |  4 ----
 ofproto/ofproto-dpif.c                            | 57
---------------------------------------------------------
 tests/test-odp.c                                  |  3 +--
 vswitchd/vswitch.xml                              |  5 -----
 6 files changed, 12 insertions(+), 86 deletions(-)

         <p>
           When Open vSwitch translates actions from OpenFlow into the
datapath

Comments

Gregory Rose Aug. 17, 2020, 4:46 p.m. UTC | #1
On 8/15/2020 8:03 PM, 范开喜 wrote:
> Currently OVS supports setting and matching ICMPv6 Neighbor Discovery
> related fields: “ND option type” and “ND reserved”. So we expect that OVS
> could support ICMPv6 ND Responder as the same as ARP responder.
> The OVS datapath could rewrite a ICMPv6 neighbor solicitation  packet to be
> a neighbor advertisement packet, and then output the packet to its ingress
> port.
> 
> But when using kernel datapath, controller and ovs-ofctl tool could not
> insert openflow rule matching and setting “ND option type” and “ND
> reserved” fields for ICMPv6 Neighbor Advertisement packet. So the user has
> to implement ICMPv6 ND Responder procedure in the controller. It's not the
> same as an ARP responder. This is because that the ofproto-dpif checks
> whether the datapath supports nd_extention key at the startup time. If not,
> it will reject the openflow rules which modify “ND option type” and “ND
> reserved” fields.
> 
> This could be removed to keep the  same behavior for ARP and ICMPv6
> Neighbor Discovery procedure in OVS . Then the controller could add
> openflow rules to match and modify “ND option type” and “ND reserved”
> fields.  Currently kernel fast datapath could not match and modify these
> two fields as the same as ARP fields. So when a  ICMPv6 neighbor
> solicitation packet is received by the kernel datapath, it will not match
> any flows and would be delivered to the userspace datapath. The userspace
> datapath would rewrite “ND option type” and “ND reserved” fields of the
> packet and translate this packet to be a ICMPv6 neighbor advertisement
> packet. The packet would be injected to the kernel datapath to output to a
> port.
> 
> Signed-off-by: fankaixi <fankaixi.li@bytedance.com>
> ---
>   datapath/linux/compat/include/linux/openvswitch.h |  8 +++-----
>   lib/odp-util.c                                    | 21
> ++++++++-------------
>   lib/odp-util.h                                    |  4 ----
>   ofproto/ofproto-dpif.c                            | 57
> ---------------------------------------------------------
>   tests/test-odp.c                                  |  3 +--
>   vswitchd/vswitch.xml                              |  5 -----
>   6 files changed, 12 insertions(+), 86 deletions(-)
> 
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h
> b/datapath/linux/compat/include/linux/openvswitch.h
> index cc41bbea4..aafd506fc 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -377,12 +377,12 @@ enum ovs_key_attr {
>    OVS_KEY_ATTR_TUNNEL_INFO,  /* struct ovs_tunnel_info */
>   #endif
> 
> -#ifndef __KERNEL__
> - /* Only used within userspace data path. */
> +#ifndef __KERNEL__
>    OVS_KEY_ATTR_PACKET_TYPE,  /* be32 packet type */
> - OVS_KEY_ATTR_ND_EXTENSIONS, /* struct ovs_key_nd_extensions */
>   #endif
> 
> +    OVS_KEY_ATTR_ND_EXTENSIONS, /* struct ovs_key_nd_extensions */
> +
>    __OVS_KEY_ATTR_MAX
>   };
> 
> @@ -518,12 +518,10 @@ struct ovs_key_nd {
>    __u8 nd_tll[ETH_ALEN];
>   };
> 
> -#ifndef __KERNEL__
>   struct ovs_key_nd_extensions {
>       __be32  nd_reserved;
>       __u8    nd_options_type;
>   };
> -#endif
> 

If you're going to expose this structure and defines to the Linux
kernel datapath I believe you should first send a patch to the
netdev Linux kernel mailing list since openvswitch.h is part of
the Linux kernel.  When the changes are accepted and posted there
we can backport them to our out of tree driver.

Thanks,

- Greg


>   #define OVS_CT_LABELS_LEN_32 4
>   #define OVS_CT_LABELS_LEN (OVS_CT_LABELS_LEN_32 * sizeof(__u32))
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 5989381e9..1eaaebd99 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -6351,20 +6351,15 @@ odp_flow_key_from_flow__(const struct
> odp_flow_key_parms *parms,
>                   nd_key->nd_sll = data->arp_sha;
>                   nd_key->nd_tll = data->arp_tha;
> 
> -                /* Add ND Extensions Attr only if supported and reserved
> field
> +                /* Add ND Extensions Attr only if reserved field
>                    * or options type is set. */
> -                if (parms->support.nd_ext) {
> -                    struct ovs_key_nd_extensions *nd_ext_key;
> -
> -                    if (data->igmp_group_ip4 != 0 || data->tcp_flags != 0)
> {
> -                        /* 'struct ovs_key_nd_extensions' has padding,
> -                         * clear it. */
> -                        nd_ext_key = nl_msg_put_unspec_zero(buf,
> -                                            OVS_KEY_ATTR_ND_EXTENSIONS,
> -                                            sizeof *nd_ext_key);
> -                        nd_ext_key->nd_reserved = data->igmp_group_ip4;
> -                        nd_ext_key->nd_options_type =
> ntohs(data->tcp_flags);
> -                    }
> +                struct ovs_key_nd_extensions *nd_ext_key;
> +                if (data->igmp_group_ip4 != 0 || data->tcp_flags != 0) {
> +                    /* 'struct ovs_key_nd_extensions' has padding,
> +                     * clear it. */
> +                    nd_ext_key = nl_msg_put_unspec_zero(buf,
> OVS_KEY_ATTR_ND_EXTENSIONS, sizeof *nd_ext_key);
> +                    nd_ext_key->nd_reserved = data->igmp_group_ip4;
> +                    nd_ext_key->nd_options_type = ntohs(data->tcp_flags);
>                   }
>               }
>           }
> diff --git a/lib/odp-util.h b/lib/odp-util.h
> index 623a66aa2..b9700291c 100644
> --- a/lib/odp-util.h
> +++ b/lib/odp-util.h
> @@ -204,10 +204,6 @@ int odp_flow_from_string(const char *s, const struct
> simap *port_names,
>       /* Conntrack original direction tuple matching * supported. */
>    \
>       ODP_SUPPORT_FIELD(bool, ct_orig_tuple, "CT orig tuple")
>     \
>       ODP_SUPPORT_FIELD(bool, ct_orig_tuple6, "CT orig tuple for IPv6")
>     \
> -
>    \
> -    /* If true, it means that the datapath supports the IPv6 Neigh
>    \
> -     * Discovery Extension bits. */
>     \
> -    ODP_SUPPORT_FIELD(bool, nd_ext, "IPv6 ND Extension")
> 
>   /* Indicates support for various fields. This defines how flows will be
>    * serialised. */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 4f0638f23..b8a3a1003 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1474,53 +1474,6 @@ check_max_dp_hash_alg(struct dpif_backer *backer)
>       return max_alg;
>   }
> 
> -/* Tests whether 'backer''s datapath supports IPv6 ND extensions.
> - * Only userspace datapath support OVS_KEY_ATTR_ND_EXTENSIONS in keys.
> - *
> - * Returns false if 'backer' definitely does not support matching and
> - * setting reserved and options type, true if it seems to support. */
> -static bool
> -check_nd_extensions(struct dpif_backer *backer)
> -{
> -    struct eth_header *eth;
> -    struct ofpbuf actions;
> -    struct dp_packet packet;
> -    struct flow flow;
> -    int error;
> -    struct ovs_key_nd_extensions key, mask;
> -
> -    ofpbuf_init(&actions, 64);
> -    memset(&key, 0x53, sizeof key);
> -    memset(&mask, 0x7f, sizeof mask);
> -    commit_masked_set_action(&actions, OVS_KEY_ATTR_ND_EXTENSIONS, &key,
> &mask,
> -                             sizeof key);
> -
> -    /* Compose a dummy ethernet packet. */
> -    dp_packet_init(&packet, ETH_HEADER_LEN);
> -    eth = dp_packet_put_zeros(&packet, ETH_HEADER_LEN);
> -    eth->eth_type = htons(0x1234);
> -
> -    flow_extract(&packet, &flow);
> -
> -    /* Execute the actions.  On datapaths without support fails with
> EINVAL. */
> -    struct dpif_execute execute = {
> -        .actions = actions.data,
> -        .actions_len = actions.size,
> -        .packet = &packet,
> -        .flow = &flow,
> -        .probe = true,
> -    };
> -    error = dpif_execute(backer->dpif, &execute);
> -
> -    dp_packet_uninit(&packet);
> -    ofpbuf_uninit(&actions);
> -
> -    VLOG_INFO("%s: Datapath %s IPv6 ND Extensions",
> dpif_name(backer->dpif),
> -              error ? "does not support" : "supports");
> -
> -    return !error;
> -}
> -
>   #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE)
>    \
>   static bool
>    \
>   check_##NAME(struct dpif_backer *backer)
>   \
> @@ -1599,7 +1552,6 @@ check_support(struct dpif_backer *backer)
>       backer->rt_support.odp.ct_state_nat = check_ct_state_nat(backer);
>       backer->rt_support.odp.ct_orig_tuple = check_ct_orig_tuple(backer);
>       backer->rt_support.odp.ct_orig_tuple6 = check_ct_orig_tuple6(backer);
> -    backer->rt_support.odp.nd_ext = check_nd_extensions(backer);
>   }
> 
>   static int
> @@ -4637,14 +4589,6 @@ check_actions(const struct ofproto_dpif *ofproto,
>                                          "ct original direction tuple");
>                   return OFPERR_NXBAC_CT_DATAPATH_SUPPORT;
>               }
> -        } else if (!support->nd_ext && ofpact->type == OFPACT_SET_FIELD) {
> -            const struct mf_field *dst = ofpact_get_mf_dst(ofpact);
> -
> -            if (dst->id == MFF_ND_RESERVED || dst->id ==
> MFF_ND_OPTIONS_TYPE) {
> -                report_unsupported_act("set field",
> -                                       "setting IPv6 ND Extensions
> fields");
> -                return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
> -            }
>           }
>       }
> 
> @@ -5585,7 +5529,6 @@ get_datapath_cap(const char *datapath_type, struct
> smap *cap)
>       smap_add(cap, "ct_state_nat", odp.ct_state_nat ? "true" : "false");
>       smap_add(cap, "ct_orig_tuple", odp.ct_orig_tuple ? "true" : "false");
>       smap_add(cap, "ct_orig_tuple6", odp.ct_orig_tuple6 ? "true" : "false");
> -    smap_add(cap, "nd_ext", odp.nd_ext ? "true" : "false");
> 
>       /* DPIF_SUPPORT_FIELDS */
>       smap_add(cap, "masked_set_action", s.masked_set_action ? "true" :
> "false");
> diff --git a/tests/test-odp.c b/tests/test-odp.c
> index 0ddfd4070..644c51a89 100644
> --- a/tests/test-odp.c
> +++ b/tests/test-odp.c
> @@ -64,8 +64,7 @@ parse_keys(bool wc_keys)
>                       .ct_zone = true,
>                       .ct_mark = true,
>                       .ct_label = true,
> -                    .max_vlan_headers = SIZE_MAX,
> -                    .nd_ext = true,
> +                    .max_vlan_headers = SIZE_MAX
>                   },
>               };
> 
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 81c84927f..37cb7a685 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -5955,11 +5955,6 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
> type=patch options:peer=p1 \
>           packet to be sent to the Open vSwitch slow path, which is likely to
>           make it too slow for mirroring traffic in bulk.
>         </column>
> -      <column name="capabilities" key="nd_ext" type='{"type": "boolean"}'>
> -        True if the datapath supports OVS_KEY_ATTR_ND_EXTENSIONS to match
> on
> -        ICMPv6 "ND reserved" and "ND option type" header fields. If false,
> -        the datapath reports error if the feature is used.
> -      </column>
>         <group title="Clone Actions">
>           <p>
>             When Open vSwitch translates actions from OpenFlow into the
> datapath
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
范开喜 Aug. 18, 2020, 12:59 a.m. UTC | #2
OK. I will split this patch.

Thanks.

Gregory Rose <gvrose8192@gmail.com> 于2020年8月18日周二 上午12:46写道:

>
> On 8/15/2020 8:03 PM, 范开喜 wrote:
> > Currently OVS supports setting and matching ICMPv6 Neighbor Discovery
> > related fields: “ND option type” and “ND reserved”. So we expect that OVS
> > could support ICMPv6 ND Responder as the same as ARP responder.
> > The OVS datapath could rewrite a ICMPv6 neighbor solicitation  packet to
> be
> > a neighbor advertisement packet, and then output the packet to its
> ingress
> > port.
> >
> > But when using kernel datapath, controller and ovs-ofctl tool could not
> > insert openflow rule matching and setting “ND option type” and “ND
> > reserved” fields for ICMPv6 Neighbor Advertisement packet. So the user
> has
> > to implement ICMPv6 ND Responder procedure in the controller. It's not
> the
> > same as an ARP responder. This is because that the ofproto-dpif checks
> > whether the datapath supports nd_extention key at the startup time. If
> not,
> > it will reject the openflow rules which modify “ND option type” and “ND
> > reserved” fields.
> >
> > This could be removed to keep the  same behavior for ARP and ICMPv6
> > Neighbor Discovery procedure in OVS . Then the controller could add
> > openflow rules to match and modify “ND option type” and “ND reserved”
> > fields.  Currently kernel fast datapath could not match and modify these
> > two fields as the same as ARP fields. So when a  ICMPv6 neighbor
> > solicitation packet is received by the kernel datapath, it will not match
> > any flows and would be delivered to the userspace datapath. The userspace
> > datapath would rewrite “ND option type” and “ND reserved” fields of the
> > packet and translate this packet to be a ICMPv6 neighbor advertisement
> > packet. The packet would be injected to the kernel datapath to output to
> a
> > port.
> >
> > Signed-off-by: fankaixi <fankaixi.li@bytedance.com>
> > ---
> >   datapath/linux/compat/include/linux/openvswitch.h |  8 +++-----
> >   lib/odp-util.c                                    | 21
> > ++++++++-------------
> >   lib/odp-util.h                                    |  4 ----
> >   ofproto/ofproto-dpif.c                            | 57
> > ---------------------------------------------------------
> >   tests/test-odp.c                                  |  3 +--
> >   vswitchd/vswitch.xml                              |  5 -----
> >   6 files changed, 12 insertions(+), 86 deletions(-)
> >
> > diff --git a/datapath/linux/compat/include/linux/openvswitch.h
> > b/datapath/linux/compat/include/linux/openvswitch.h
> > index cc41bbea4..aafd506fc 100644
> > --- a/datapath/linux/compat/include/linux/openvswitch.h
> > +++ b/datapath/linux/compat/include/linux/openvswitch.h
> > @@ -377,12 +377,12 @@ enum ovs_key_attr {
> >    OVS_KEY_ATTR_TUNNEL_INFO,  /* struct ovs_tunnel_info */
> >   #endif
> >
> > -#ifndef __KERNEL__
> > - /* Only used within userspace data path. */
> > +#ifndef __KERNEL__
> >    OVS_KEY_ATTR_PACKET_TYPE,  /* be32 packet type */
> > - OVS_KEY_ATTR_ND_EXTENSIONS, /* struct ovs_key_nd_extensions */
> >   #endif
> >
> > +    OVS_KEY_ATTR_ND_EXTENSIONS, /* struct ovs_key_nd_extensions */
> > +
> >    __OVS_KEY_ATTR_MAX
> >   };
> >
> > @@ -518,12 +518,10 @@ struct ovs_key_nd {
> >    __u8 nd_tll[ETH_ALEN];
> >   };
> >
> > -#ifndef __KERNEL__
> >   struct ovs_key_nd_extensions {
> >       __be32  nd_reserved;
> >       __u8    nd_options_type;
> >   };
> > -#endif
> >
>
> If you're going to expose this structure and defines to the Linux
> kernel datapath I believe you should first send a patch to the
> netdev Linux kernel mailing list since openvswitch.h is part of
> the Linux kernel.  When the changes are accepted and posted there
> we can backport them to our out of tree driver.
>
> Thanks,
>
> - Greg
>
>
> >   #define OVS_CT_LABELS_LEN_32 4
> >   #define OVS_CT_LABELS_LEN (OVS_CT_LABELS_LEN_32 * sizeof(__u32))
> > diff --git a/lib/odp-util.c b/lib/odp-util.c
> > index 5989381e9..1eaaebd99 100644
> > --- a/lib/odp-util.c
> > +++ b/lib/odp-util.c
> > @@ -6351,20 +6351,15 @@ odp_flow_key_from_flow__(const struct
> > odp_flow_key_parms *parms,
> >                   nd_key->nd_sll = data->arp_sha;
> >                   nd_key->nd_tll = data->arp_tha;
> >
> > -                /* Add ND Extensions Attr only if supported and reserved
> > field
> > +                /* Add ND Extensions Attr only if reserved field
> >                    * or options type is set. */
> > -                if (parms->support.nd_ext) {
> > -                    struct ovs_key_nd_extensions *nd_ext_key;
> > -
> > -                    if (data->igmp_group_ip4 != 0 || data->tcp_flags !=
> 0)
> > {
> > -                        /* 'struct ovs_key_nd_extensions' has padding,
> > -                         * clear it. */
> > -                        nd_ext_key = nl_msg_put_unspec_zero(buf,
> > -                                            OVS_KEY_ATTR_ND_EXTENSIONS,
> > -                                            sizeof *nd_ext_key);
> > -                        nd_ext_key->nd_reserved = data->igmp_group_ip4;
> > -                        nd_ext_key->nd_options_type =
> > ntohs(data->tcp_flags);
> > -                    }
> > +                struct ovs_key_nd_extensions *nd_ext_key;
> > +                if (data->igmp_group_ip4 != 0 || data->tcp_flags != 0) {
> > +                    /* 'struct ovs_key_nd_extensions' has padding,
> > +                     * clear it. */
> > +                    nd_ext_key = nl_msg_put_unspec_zero(buf,
> > OVS_KEY_ATTR_ND_EXTENSIONS, sizeof *nd_ext_key);
> > +                    nd_ext_key->nd_reserved = data->igmp_group_ip4;
> > +                    nd_ext_key->nd_options_type =
> ntohs(data->tcp_flags);
> >                   }
> >               }
> >           }
> > diff --git a/lib/odp-util.h b/lib/odp-util.h
> > index 623a66aa2..b9700291c 100644
> > --- a/lib/odp-util.h
> > +++ b/lib/odp-util.h
> > @@ -204,10 +204,6 @@ int odp_flow_from_string(const char *s, const struct
> > simap *port_names,
> >       /* Conntrack original direction tuple matching * supported. */
> >    \
> >       ODP_SUPPORT_FIELD(bool, ct_orig_tuple, "CT orig tuple")
> >     \
> >       ODP_SUPPORT_FIELD(bool, ct_orig_tuple6, "CT orig tuple for IPv6")
> >     \
> > -
> >    \
> > -    /* If true, it means that the datapath supports the IPv6 Neigh
> >    \
> > -     * Discovery Extension bits. */
> >     \
> > -    ODP_SUPPORT_FIELD(bool, nd_ext, "IPv6 ND Extension")
> >
> >   /* Indicates support for various fields. This defines how flows will be
> >    * serialised. */
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 4f0638f23..b8a3a1003 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -1474,53 +1474,6 @@ check_max_dp_hash_alg(struct dpif_backer *backer)
> >       return max_alg;
> >   }
> >
> > -/* Tests whether 'backer''s datapath supports IPv6 ND extensions.
> > - * Only userspace datapath support OVS_KEY_ATTR_ND_EXTENSIONS in keys.
> > - *
> > - * Returns false if 'backer' definitely does not support matching and
> > - * setting reserved and options type, true if it seems to support. */
> > -static bool
> > -check_nd_extensions(struct dpif_backer *backer)
> > -{
> > -    struct eth_header *eth;
> > -    struct ofpbuf actions;
> > -    struct dp_packet packet;
> > -    struct flow flow;
> > -    int error;
> > -    struct ovs_key_nd_extensions key, mask;
> > -
> > -    ofpbuf_init(&actions, 64);
> > -    memset(&key, 0x53, sizeof key);
> > -    memset(&mask, 0x7f, sizeof mask);
> > -    commit_masked_set_action(&actions, OVS_KEY_ATTR_ND_EXTENSIONS, &key,
> > &mask,
> > -                             sizeof key);
> > -
> > -    /* Compose a dummy ethernet packet. */
> > -    dp_packet_init(&packet, ETH_HEADER_LEN);
> > -    eth = dp_packet_put_zeros(&packet, ETH_HEADER_LEN);
> > -    eth->eth_type = htons(0x1234);
> > -
> > -    flow_extract(&packet, &flow);
> > -
> > -    /* Execute the actions.  On datapaths without support fails with
> > EINVAL. */
> > -    struct dpif_execute execute = {
> > -        .actions = actions.data,
> > -        .actions_len = actions.size,
> > -        .packet = &packet,
> > -        .flow = &flow,
> > -        .probe = true,
> > -    };
> > -    error = dpif_execute(backer->dpif, &execute);
> > -
> > -    dp_packet_uninit(&packet);
> > -    ofpbuf_uninit(&actions);
> > -
> > -    VLOG_INFO("%s: Datapath %s IPv6 ND Extensions",
> > dpif_name(backer->dpif),
> > -              error ? "does not support" : "supports");
> > -
> > -    return !error;
> > -}
> > -
> >   #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE)
> >    \
> >   static bool
> >    \
> >   check_##NAME(struct dpif_backer *backer)
> >   \
> > @@ -1599,7 +1552,6 @@ check_support(struct dpif_backer *backer)
> >       backer->rt_support.odp.ct_state_nat = check_ct_state_nat(backer);
> >       backer->rt_support.odp.ct_orig_tuple = check_ct_orig_tuple(backer);
> >       backer->rt_support.odp.ct_orig_tuple6 =
> check_ct_orig_tuple6(backer);
> > -    backer->rt_support.odp.nd_ext = check_nd_extensions(backer);
> >   }
> >
> >   static int
> > @@ -4637,14 +4589,6 @@ check_actions(const struct ofproto_dpif *ofproto,
> >                                          "ct original direction tuple");
> >                   return OFPERR_NXBAC_CT_DATAPATH_SUPPORT;
> >               }
> > -        } else if (!support->nd_ext && ofpact->type ==
> OFPACT_SET_FIELD) {
> > -            const struct mf_field *dst = ofpact_get_mf_dst(ofpact);
> > -
> > -            if (dst->id == MFF_ND_RESERVED || dst->id ==
> > MFF_ND_OPTIONS_TYPE) {
> > -                report_unsupported_act("set field",
> > -                                       "setting IPv6 ND Extensions
> > fields");
> > -                return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
> > -            }
> >           }
> >       }
> >
> > @@ -5585,7 +5529,6 @@ get_datapath_cap(const char *datapath_type, struct
> > smap *cap)
> >       smap_add(cap, "ct_state_nat", odp.ct_state_nat ? "true" : "false");
> >       smap_add(cap, "ct_orig_tuple", odp.ct_orig_tuple ? "true" :
> "false");
> >       smap_add(cap, "ct_orig_tuple6", odp.ct_orig_tuple6 ? "true" :
> "false");
> > -    smap_add(cap, "nd_ext", odp.nd_ext ? "true" : "false");
> >
> >       /* DPIF_SUPPORT_FIELDS */
> >       smap_add(cap, "masked_set_action", s.masked_set_action ? "true" :
> > "false");
> > diff --git a/tests/test-odp.c b/tests/test-odp.c
> > index 0ddfd4070..644c51a89 100644
> > --- a/tests/test-odp.c
> > +++ b/tests/test-odp.c
> > @@ -64,8 +64,7 @@ parse_keys(bool wc_keys)
> >                       .ct_zone = true,
> >                       .ct_mark = true,
> >                       .ct_label = true,
> > -                    .max_vlan_headers = SIZE_MAX,
> > -                    .nd_ext = true,
> > +                    .max_vlan_headers = SIZE_MAX
> >                   },
> >               };
> >
> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> > index 81c84927f..37cb7a685 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -5955,11 +5955,6 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
> > type=patch options:peer=p1 \
> >           packet to be sent to the Open vSwitch slow path, which is
> likely to
> >           make it too slow for mirroring traffic in bulk.
> >         </column>
> > -      <column name="capabilities" key="nd_ext" type='{"type":
> "boolean"}'>
> > -        True if the datapath supports OVS_KEY_ATTR_ND_EXTENSIONS to
> match
> > on
> > -        ICMPv6 "ND reserved" and "ND option type" header fields. If
> false,
> > -        the datapath reports error if the feature is used.
> > -      </column>
> >         <group title="Clone Actions">
> >           <p>
> >             When Open vSwitch translates actions from OpenFlow into the
> > datapath
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
diff mbox series

Patch

diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index cc41bbea4..aafd506fc 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -377,12 +377,12 @@  enum ovs_key_attr {
  OVS_KEY_ATTR_TUNNEL_INFO,  /* struct ovs_tunnel_info */
 #endif

-#ifndef __KERNEL__
- /* Only used within userspace data path. */
+#ifndef __KERNEL__
  OVS_KEY_ATTR_PACKET_TYPE,  /* be32 packet type */
- OVS_KEY_ATTR_ND_EXTENSIONS, /* struct ovs_key_nd_extensions */
 #endif

+    OVS_KEY_ATTR_ND_EXTENSIONS, /* struct ovs_key_nd_extensions */
+
  __OVS_KEY_ATTR_MAX
 };

@@ -518,12 +518,10 @@  struct ovs_key_nd {
  __u8 nd_tll[ETH_ALEN];
 };

-#ifndef __KERNEL__
 struct ovs_key_nd_extensions {
     __be32  nd_reserved;
     __u8    nd_options_type;
 };
-#endif

 #define OVS_CT_LABELS_LEN_32 4
 #define OVS_CT_LABELS_LEN (OVS_CT_LABELS_LEN_32 * sizeof(__u32))
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 5989381e9..1eaaebd99 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -6351,20 +6351,15 @@  odp_flow_key_from_flow__(const struct
odp_flow_key_parms *parms,
                 nd_key->nd_sll = data->arp_sha;
                 nd_key->nd_tll = data->arp_tha;

-                /* Add ND Extensions Attr only if supported and reserved
field
+                /* Add ND Extensions Attr only if reserved field
                  * or options type is set. */
-                if (parms->support.nd_ext) {
-                    struct ovs_key_nd_extensions *nd_ext_key;
-
-                    if (data->igmp_group_ip4 != 0 || data->tcp_flags != 0)
{
-                        /* 'struct ovs_key_nd_extensions' has padding,
-                         * clear it. */
-                        nd_ext_key = nl_msg_put_unspec_zero(buf,
-                                            OVS_KEY_ATTR_ND_EXTENSIONS,
-                                            sizeof *nd_ext_key);
-                        nd_ext_key->nd_reserved = data->igmp_group_ip4;
-                        nd_ext_key->nd_options_type =
ntohs(data->tcp_flags);
-                    }
+                struct ovs_key_nd_extensions *nd_ext_key;
+                if (data->igmp_group_ip4 != 0 || data->tcp_flags != 0) {
+                    /* 'struct ovs_key_nd_extensions' has padding,
+                     * clear it. */
+                    nd_ext_key = nl_msg_put_unspec_zero(buf,
OVS_KEY_ATTR_ND_EXTENSIONS, sizeof *nd_ext_key);
+                    nd_ext_key->nd_reserved = data->igmp_group_ip4;
+                    nd_ext_key->nd_options_type = ntohs(data->tcp_flags);
                 }
             }
         }
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 623a66aa2..b9700291c 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -204,10 +204,6 @@  int odp_flow_from_string(const char *s, const struct
simap *port_names,
     /* Conntrack original direction tuple matching * supported. */
  \
     ODP_SUPPORT_FIELD(bool, ct_orig_tuple, "CT orig tuple")
   \
     ODP_SUPPORT_FIELD(bool, ct_orig_tuple6, "CT orig tuple for IPv6")
   \
-
  \
-    /* If true, it means that the datapath supports the IPv6 Neigh
  \
-     * Discovery Extension bits. */
   \
-    ODP_SUPPORT_FIELD(bool, nd_ext, "IPv6 ND Extension")

 /* Indicates support for various fields. This defines how flows will be
  * serialised. */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 4f0638f23..b8a3a1003 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1474,53 +1474,6 @@  check_max_dp_hash_alg(struct dpif_backer *backer)
     return max_alg;
 }

-/* Tests whether 'backer''s datapath supports IPv6 ND extensions.
- * Only userspace datapath support OVS_KEY_ATTR_ND_EXTENSIONS in keys.
- *
- * Returns false if 'backer' definitely does not support matching and
- * setting reserved and options type, true if it seems to support. */
-static bool
-check_nd_extensions(struct dpif_backer *backer)
-{
-    struct eth_header *eth;
-    struct ofpbuf actions;
-    struct dp_packet packet;
-    struct flow flow;
-    int error;
-    struct ovs_key_nd_extensions key, mask;
-
-    ofpbuf_init(&actions, 64);
-    memset(&key, 0x53, sizeof key);
-    memset(&mask, 0x7f, sizeof mask);
-    commit_masked_set_action(&actions, OVS_KEY_ATTR_ND_EXTENSIONS, &key,
&mask,
-                             sizeof key);
-
-    /* Compose a dummy ethernet packet. */
-    dp_packet_init(&packet, ETH_HEADER_LEN);
-    eth = dp_packet_put_zeros(&packet, ETH_HEADER_LEN);
-    eth->eth_type = htons(0x1234);
-
-    flow_extract(&packet, &flow);
-
-    /* Execute the actions.  On datapaths without support fails with
EINVAL. */
-    struct dpif_execute execute = {
-        .actions = actions.data,
-        .actions_len = actions.size,
-        .packet = &packet,
-        .flow = &flow,
-        .probe = true,
-    };
-    error = dpif_execute(backer->dpif, &execute);
-
-    dp_packet_uninit(&packet);
-    ofpbuf_uninit(&actions);
-
-    VLOG_INFO("%s: Datapath %s IPv6 ND Extensions",
dpif_name(backer->dpif),
-              error ? "does not support" : "supports");
-
-    return !error;
-}
-
 #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE)
  \
 static bool
  \
 check_##NAME(struct dpif_backer *backer)
 \
@@ -1599,7 +1552,6 @@  check_support(struct dpif_backer *backer)
     backer->rt_support.odp.ct_state_nat = check_ct_state_nat(backer);
     backer->rt_support.odp.ct_orig_tuple = check_ct_orig_tuple(backer);
     backer->rt_support.odp.ct_orig_tuple6 = check_ct_orig_tuple6(backer);
-    backer->rt_support.odp.nd_ext = check_nd_extensions(backer);
 }

 static int
@@ -4637,14 +4589,6 @@  check_actions(const struct ofproto_dpif *ofproto,
                                        "ct original direction tuple");
                 return OFPERR_NXBAC_CT_DATAPATH_SUPPORT;
             }
-        } else if (!support->nd_ext && ofpact->type == OFPACT_SET_FIELD) {
-            const struct mf_field *dst = ofpact_get_mf_dst(ofpact);
-
-            if (dst->id == MFF_ND_RESERVED || dst->id ==
MFF_ND_OPTIONS_TYPE) {
-                report_unsupported_act("set field",
-                                       "setting IPv6 ND Extensions
fields");
-                return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
-            }
         }
     }

@@ -5585,7 +5529,6 @@  get_datapath_cap(const char *datapath_type, struct
smap *cap)
     smap_add(cap, "ct_state_nat", odp.ct_state_nat ? "true" : "false");
     smap_add(cap, "ct_orig_tuple", odp.ct_orig_tuple ? "true" : "false");
     smap_add(cap, "ct_orig_tuple6", odp.ct_orig_tuple6 ? "true" : "false");
-    smap_add(cap, "nd_ext", odp.nd_ext ? "true" : "false");

     /* DPIF_SUPPORT_FIELDS */
     smap_add(cap, "masked_set_action", s.masked_set_action ? "true" :
"false");
diff --git a/tests/test-odp.c b/tests/test-odp.c
index 0ddfd4070..644c51a89 100644
--- a/tests/test-odp.c
+++ b/tests/test-odp.c
@@ -64,8 +64,7 @@  parse_keys(bool wc_keys)
                     .ct_zone = true,
                     .ct_mark = true,
                     .ct_label = true,
-                    .max_vlan_headers = SIZE_MAX,
-                    .nd_ext = true,
+                    .max_vlan_headers = SIZE_MAX
                 },
             };

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 81c84927f..37cb7a685 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -5955,11 +5955,6 @@  ovs-vsctl add-port br0 p0 -- set Interface p0
type=patch options:peer=p1 \
         packet to be sent to the Open vSwitch slow path, which is likely to
         make it too slow for mirroring traffic in bulk.
       </column>
-      <column name="capabilities" key="nd_ext" type='{"type": "boolean"}'>
-        True if the datapath supports OVS_KEY_ATTR_ND_EXTENSIONS to match
on
-        ICMPv6 "ND reserved" and "ND option type" header fields. If false,
-        the datapath reports error if the feature is used.
-      </column>
       <group title="Clone Actions">