Message ID | CAEEdnKGjBToBpp_4Rwm0=7d5caOEUH_u-5GRHN1iXJuC=YUnxQ@mail.gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev] ofproto-dpif: remove checking nd_ext for datapath | expand |
范开喜 <fankaixi.li@bytedance.com> writes: > We expect ICMPv6 Neighbor Discovery to be the same as ARP procedure. But > now OVS checks the datapath whether it supports setting "ND Option Type" > and "ND Reserved" fields. If not, ovs would reject openflow rules setting > these fields. So controller could not add this type of rules to support > ICMPv6 Neighbor Discovery Responder. > > Signed-off-by: fankaixi.li <fankaixi.li@bytedance.com> > --- Your patch is malformatted - please resbumit. Additionally, the kernel doesn't have support for these attributes and datastructures, so the current approach of checking per datapath seems correct. Did I miss something? What happens when you use the kernel tree ovs module with these actions and attempt to have flows installed. Do they actually get installed (check with 'ovs-appctl dpctl/show')? > datapath/linux/compat/include/linux/openvswitch.h | 6 +-- > lib/odp-util.c | 20 ++++---- > lib/odp-util.h | 4 -- > ofproto/ofproto-dpif.c | 57 > ----------------------- > tests/test-odp.c | 3 +- > vswitchd/vswitch.xml | 5 -- > 6 files changed, 11 insertions(+), 84 deletions(-) > > diff --git a/datapath/linux/compat/include/linux/openvswitch.h > b/datapath/linux/compat/include/linux/openvswitch.h > index cc41bbea4..2c42e7b97 100644 > --- a/datapath/linux/compat/include/linux/openvswitch.h > +++ b/datapath/linux/compat/include/linux/openvswitch.h > @@ -378,11 +378,11 @@ enum ovs_key_attr { > #endif > > #ifndef __KERNEL__ > - /* Only used within userspace data path. */ > 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..49fbc145f 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -6353,18 +6353,14 @@ odp_flow_key_from_flow__(const struct > odp_flow_key_parms *parms, > > /* Add ND Extensions Attr only if supported and 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) { > + nd_ext_key = nl_msg_put_unspec_uninit(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 > -- > 2.11.0 > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Yes, I would resubmit it soon. And it would split into datapath and userspace patchs. By now, openflow rules which contains actions setting "ipv6 neighbor discovery reserved and option field" could not added into ovs bridge. It's not the same as ipv4 arp. Because when adding this type of flows, ovs would check the kernel datapath capabilities and reject the flows setting "ipv6 neighbor discovery reserved and option field" . I do not expect kernel datapath to support setting these fields, and install this type of flows to kernel datapath. I just expect the same behavior as ipv4 arp. Thanks. 于2020年08月25日星期二 23:13 <aconole@redhat.com> 写道: 范开喜 writes: > We expect ICMPv6 Neighbor Discovery to be the same as ARP procedure. But > now OVS checks the datapath whether it supports setting "ND Option Type" > and "ND Reserved" fields. If not, ovs would reject openflow rules setting > these fields. So controller could not add this type of rules to support > ICMPv6 Neighbor Discovery Responder. > > Signed-off-by: fankaixi.li > --- Your patch is malformatted - please resbumit. Additionally, the kernel doesn't have support for these attributes and datastructures, so the current approach of checking per datapath seems correct. Did I miss something? What happens when you use the kernel tree ovs module with these actions and attempt to have flows installed. Do they actually get installed (check with 'ovs-appctl dpctl/show')? > datapath/linux/compat/include/linux/openvswitch.h | 6 +-- > lib/odp-util.c | 20 ++++---- > lib/odp-util.h | 4 -- > ofproto/ofproto-dpif.c | 57 > ----------------------- > tests/test-odp.c | 3 +- > vswitchd/vswitch.xml | 5 -- > 6 files changed, 11 insertions(+), 84 deletions(-) > > diff --git a/datapath/linux/compat/include/linux/openvswitch.h > b/datapath/linux/compat/include/linux/openvswitch.h > index cc41bbea4..2c42e7b97 100644 > --- a/datapath/linux/compat/include/linux/openvswitch.h > +++ b/datapath/linux/compat/include/linux/openvswitch.h > @@ -378,11 +378,11 @@ enum ovs_key_attr { > #endif > > #ifndef __KERNEL__ > - /* Only used within userspace data path. */ > 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..49fbc145f 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -6353,18 +6353,14 @@ odp_flow_key_from_flow__(const struct > odp_flow_key_parms *parms, > > /* Add ND Extensions Attr only if supported and 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) { > + nd_ext_key = nl_msg_put_unspec_uninit(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. > > - > - 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. > - > > > When Open vSwitch translates actions from OpenFlow into the > datapath > -- > 2.11.0 > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h index cc41bbea4..2c42e7b97 100644 --- a/datapath/linux/compat/include/linux/openvswitch.h +++ b/datapath/linux/compat/include/linux/openvswitch.h @@ -378,11 +378,11 @@ enum ovs_key_attr { #endif #ifndef __KERNEL__ - /* Only used within userspace data path. */ 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..49fbc145f 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -6353,18 +6353,14 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms *parms, /* Add ND Extensions Attr only if supported and 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) { + nd_ext_key = nl_msg_put_unspec_uninit(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">
We expect ICMPv6 Neighbor Discovery to be the same as ARP procedure. But now OVS checks the datapath whether it supports setting "ND Option Type" and "ND Reserved" fields. If not, ovs would reject openflow rules setting these fields. So controller could not add this type of rules to support ICMPv6 Neighbor Discovery Responder. Signed-off-by: fankaixi.li <fankaixi.li@bytedance.com> --- datapath/linux/compat/include/linux/openvswitch.h | 6 +-- lib/odp-util.c | 20 ++++---- lib/odp-util.h | 4 -- ofproto/ofproto-dpif.c | 57 ----------------------- tests/test-odp.c | 3 +- vswitchd/vswitch.xml | 5 -- 6 files changed, 11 insertions(+), 84 deletions(-) <p> When Open vSwitch translates actions from OpenFlow into the datapath -- 2.11.0