Hi Ilya, I submitted now V4 of the change. Please note that in kernel, IP version conversion is blocked by the kernel module: In dmesg: openvswitch: netlink: Mixed IPv4 and IPv6 tunnel attributes
Also note that in DPDK it's also irrelevant - there's no offload of tunnel anyway. Tried with and without SLOW_MATCH, no difference. Regards, Reuven On 05/12/2025 20:25, Ilya Maximets wrote: > External email: Use caution opening links or attachments > > > On 12/2/25 9:34 AM, Reuven Plevinsky wrote: >> Hi Ilya, >> Thanks for the review. We're addressing the comments. >> >> On 11/11/2025 21:59, Ilya Maximets wrote: >>> External email: Use caution opening links or attachments >>> >>> >>> On 11/10/25 7:57 PM, Reuven Plevinsky via dev wrote: >>>> From: Eli Britstein <[email protected]> >>>> >>>> Introduce eth-type field to tunnel metadata that is internally matched >>>> for tunnels, for robust detection of the IP type. >>>> >>>> Signed-off-by: Eli Britstein <[email protected]> >>>> --- >>>> include/openvswitch/match.h | 1 + >>>> include/openvswitch/meta-flow.h | 14 +++++++++++ >>>> include/openvswitch/packets.h | 3 ++- >>>> lib/dpif.c | 6 +++++ >>>> lib/dpif.h | 1 + >>>> lib/match.c | 10 ++++++++ >>>> lib/meta-flow.c | 18 +++++++++++++++ >>>> lib/meta-flow.xml | 1 + >>>> lib/netdev-native-tnl.c | 20 ++++++++++++++++ >>>> lib/netdev-offload-dpdk.c | 41 +++++++++++++++++++++++++++++---- >>>> lib/odp-util.h | 5 +++- >>>> ofproto/ofproto-dpif-xlate.c | 16 +++++++++++++ >>>> ofproto/ofproto-dpif.c | 3 +++ >>>> ofproto/tunnel.c | 1 + >>>> tests/bfd.at | 2 +- >>>> tests/tunnel.at | 40 ++++++++++++++++---------------- >>>> 16 files changed, 154 insertions(+), 28 deletions(-) >>> Hi, Eli and Reuven. >>> >>> Thanks for the update! This version is still mising a big part of the >>> change, which is the actual datapth flow attribute to match on, i.e. >>> the OVS_TUNNEL_KEY_ATTR_ETHERTYPE. This is necessary even for userpspace >>> datapath implementation, because netlink-formatted attributes is the only >>> way revalidators work. Wihtout it, each datapath flow will likely be >>> constantly removed from the datapath on a very next revalidation cycle. >>> >>>> diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h >>>> index 2e8812048e86..b962badfcfd3 100644 >>>> --- a/include/openvswitch/match.h >>>> +++ b/include/openvswitch/match.h >>>> @@ -127,6 +127,7 @@ void match_set_tun_gtpu_flags_masked(struct match >>>> *match, uint8_t flags, >>>> void match_set_tun_gtpu_msgtype(struct match *match, uint8_t msgtype); >>>> void match_set_tun_gtpu_msgtype_masked(struct match *match, uint8_t >>>> msgtype, >>>> uint8_t mask); >>>> +void match_set_tun_eth_type(struct match *, ovs_be16); >>>> void match_set_in_port(struct match *, ofp_port_t ofp_port); >>>> void match_set_pkt_mark(struct match *, uint32_t pkt_mark); >>>> void match_set_pkt_mark_masked(struct match *, uint32_t pkt_mark, >>>> uint32_t mask); >>>> diff --git a/include/openvswitch/meta-flow.h >>>> b/include/openvswitch/meta-flow.h >>>> index 875f122c5f55..e644c684bbf0 100644 >>>> --- a/include/openvswitch/meta-flow.h >>>> +++ b/include/openvswitch/meta-flow.h >>>> @@ -304,6 +304,20 @@ enum OVS_PACKED_ENUM mf_field_id { >>>> */ >>>> MFF_TUN_ID, >>>> >>>> + /* "tun_eth_type". >>>> + * >>>> + * Packet's Tunnel Ethernet type. >>>> + * >>>> + * Type: be16. >>>> + * Maskable: no. >>>> + * Formatting: decimal. >>> Maybe hexadecimal? >>> >>>> + * Prerequisites: none. >>>> + * Access: read-only. >>>> + * NXM: none. >>>> + * OXM: none. >>>> + */ >>>> + MFF_TUN_ETH_TYPE, >>>> + >>>> /* "tun_src". >>>> * >>>> * The IPv4 source address in the outer IP header of a tunneled >>>> packet. >>>> diff --git a/include/openvswitch/packets.h b/include/openvswitch/packets.h >>>> index a65cb0d04e77..c9af1b6ceca6 100644 >>>> --- a/include/openvswitch/packets.h >>>> +++ b/include/openvswitch/packets.h >>>> @@ -45,7 +45,8 @@ struct flow_tnl { >>>> uint8_t erspan_hwid; >>>> uint8_t gtpu_flags; >>>> uint8_t gtpu_msgtype; >>>> - uint8_t pad1[4]; /* Pad to 64 bits. */ >>>> + ovs_be16 eth_type; >>>> + uint8_t pad1[2]; /* Pad to 64 bits. */ >>>> struct tun_metadata metadata; >>>> }; >>>> >>>> diff --git a/lib/dpif.c b/lib/dpif.c >>>> index 070fc0131ffb..dbec240cc9c8 100644 >>>> --- a/lib/dpif.c >>>> +++ b/lib/dpif.c >>>> @@ -1945,6 +1945,12 @@ dpif_may_support_psample(const struct dpif *dpif) >>>> return !dpif_is_netdev(dpif); >>>> } >>>> >>>> +bool >>>> +dpif_supports_tun_eth_type(const struct dpif *dpif) >>>> +{ >>>> + return dpif_is_netdev(dpif); >>> This should be a proper probing function that provides a key with >>> OVS_TUNNEL_KEY_ATTR_ETHERTYPE to the datapath and checks the result. >>> >>>> +} >>>> + >>>> /* Meters */ >>>> void >>>> dpif_meter_get_features(const struct dpif *dpif, >>>> diff --git a/lib/dpif.h b/lib/dpif.h >>>> index 0b685d8df133..0a410fca5bd2 100644 >>>> --- a/lib/dpif.h >>>> +++ b/lib/dpif.h >>>> @@ -921,6 +921,7 @@ int dpif_bond_add(struct dpif *, uint32_t bond_id, >>>> odp_port_t *member_map); >>>> int dpif_bond_del(struct dpif *, uint32_t bond_id); >>>> int dpif_bond_stats_get(struct dpif *, uint32_t bond_id, uint64_t >>>> *n_bytes); >>>> bool dpif_supports_lb_output_action(const struct dpif *); >>>> +bool dpif_supports_tun_eth_type(const struct dpif *); >>>> >>>> >>>> /* Cache */ >>>> diff --git a/lib/match.c b/lib/match.c >>>> index 9b7e06e0c7f8..bd8b21d592b1 100644 >>>> --- a/lib/match.c >>>> +++ b/lib/match.c >>>> @@ -402,6 +402,13 @@ match_set_tun_gtpu_msgtype(struct match *match, >>>> uint8_t msgtype) >>>> match_set_tun_gtpu_msgtype_masked(match, msgtype, UINT8_MAX); >>>> } >>>> >>>> +void >>>> +match_set_tun_eth_type(struct match *match, ovs_be16 eth_type) >>>> +{ >>>> + match->wc.masks.tunnel.eth_type = OVS_BE16_MAX; >>>> + match->flow.tunnel.eth_type = eth_type; >>>> +} >>>> + >>>> void >>>> match_set_in_port(struct match *match, ofp_port_t ofp_port) >>>> { >>>> @@ -1391,6 +1398,9 @@ format_flow_tunnel(struct ds *s, const struct match >>>> *match) >>>> if (wc->masks.tunnel.gtpu_msgtype) { >>>> ds_put_format(s, "gtpu_msgtype=%"PRIu8",", tnl->gtpu_msgtype); >>>> } >>>> + if (wc->masks.tunnel.eth_type) { >>>> + ds_put_format(s, "tun_eth_type=0x%04"PRIx16",", >>>> ntohs(tnl->eth_type)); >>>> + } >>>> if (wc->masks.tunnel.flags & FLOW_TNL_F_MASK) { >>>> format_flags_masked(s, "tun_flags", flow_tun_flag_to_string, >>>> tnl->flags & FLOW_TNL_F_MASK, >>>> diff --git a/lib/meta-flow.c b/lib/meta-flow.c >>>> index 2595fd634b4d..2a6ae129a91a 100644 >>>> --- a/lib/meta-flow.c >>>> +++ b/lib/meta-flow.c >>>> @@ -213,6 +213,8 @@ mf_is_all_wild(const struct mf_field *mf, const struct >>>> flow_wildcards *wc) >>>> return !wc->masks.packet_type; >>>> case MFF_CONJ_ID: >>>> return !wc->masks.conj_id; >>>> + case MFF_TUN_ETH_TYPE: >>>> + return !wc->masks.tunnel.eth_type; >>>> case MFF_TUN_SRC: >>>> return !wc->masks.tunnel.ip_src; >>>> case MFF_TUN_DST: >>>> @@ -524,6 +526,7 @@ mf_is_value_valid(const struct mf_field *mf, const >>>> union mf_value *value) >>>> case MFF_PACKET_TYPE: >>>> case MFF_CONJ_ID: >>>> case MFF_TUN_ID: >>>> + case MFF_TUN_ETH_TYPE: >>>> case MFF_TUN_SRC: >>>> case MFF_TUN_DST: >>>> case MFF_TUN_IPV6_SRC: >>>> @@ -680,6 +683,9 @@ mf_get_value(const struct mf_field *mf, const struct >>>> flow *flow, >>>> case MFF_TUN_ID: >>>> value->be64 = flow->tunnel.tun_id; >>>> break; >>>> + case MFF_TUN_ETH_TYPE: >>>> + value->be16 = flow->tunnel.eth_type; >>>> + break; >>>> case MFF_TUN_SRC: >>>> value->be32 = flow->tunnel.ip_src; >>>> break; >>>> @@ -1017,6 +1023,9 @@ mf_set_value(const struct mf_field *mf, >>>> case MFF_TUN_ID: >>>> match_set_tun_id(match, value->be64); >>>> break; >>>> + case MFF_TUN_ETH_TYPE: >>>> + match_set_tun_eth_type(match, value->be16); >>>> + break; >>>> case MFF_TUN_SRC: >>>> match_set_tun_src(match, value->be32); >>>> break; >>>> @@ -1439,6 +1448,9 @@ mf_set_flow_value(const struct mf_field *mf, >>>> case MFF_TUN_ID: >>>> flow->tunnel.tun_id = value->be64; >>>> break; >>>> + case MFF_TUN_ETH_TYPE: >>>> + flow->tunnel.eth_type = value->be16; >>>> + break; >>>> case MFF_TUN_SRC: >>>> flow->tunnel.ip_src = value->be32; >>>> break; >>>> @@ -1823,6 +1835,7 @@ mf_is_any_metadata(const struct mf_field *mf) >>>> return true; >>>> >>>> case MFF_TUN_ID: >>>> + case MFF_TUN_ETH_TYPE: >>>> case MFF_TUN_SRC: >>>> case MFF_TUN_DST: >>>> case MFF_TUN_IPV6_SRC: >>>> @@ -1917,6 +1930,7 @@ mf_is_pipeline_field(const struct mf_field *mf) >>>> { >>>> switch (mf->id) { >>>> case MFF_TUN_ID: >>>> + case MFF_TUN_ETH_TYPE: >>>> case MFF_TUN_SRC: >>>> case MFF_TUN_DST: >>>> case MFF_TUN_IPV6_SRC: >>>> @@ -2075,6 +2089,9 @@ mf_set_wild(const struct mf_field *mf, struct match >>>> *match, char **err_str) >>>> case MFF_TUN_ID: >>>> match_set_tun_id_masked(match, htonll(0), htonll(0)); >>>> break; >>>> + case MFF_TUN_ETH_TYPE: >>>> + match->flow.tunnel.eth_type = 0; >>>> + break; >>>> case MFF_TUN_SRC: >>>> match_set_tun_src_masked(match, htonl(0), htonl(0)); >>>> break; >>>> @@ -2479,6 +2496,7 @@ mf_set(const struct mf_field *mf, >>>> case MFF_ICMPV6_CODE: >>>> case MFF_ND_RESERVED: >>>> case MFF_ND_OPTIONS_TYPE: >>>> + case MFF_TUN_ETH_TYPE: >>>> return OFPUTIL_P_NONE; >>>> >>>> case MFF_DP_HASH: >>>> diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml >>>> index 5c57ab08ff18..558298279b9e 100644 >>>> --- a/lib/meta-flow.xml >>>> +++ b/lib/meta-flow.xml >>>> @@ -1508,6 +1508,7 @@ ovs-ofctl add-flow br-int >>>> 'in_port=3,tun_src=192.168.1.1,tun_id=5001 actions=1' >>>> </diagram> >>>> </field> >>>> >>>> + <field id="MFF_TUN_ETH_TYPE" title="Tunnel Ethernet type" >>>> internal="yes"/> >>>> <field id="MFF_TUN_SRC" title="Tunnel IPv4 Source"> >>>> <p> >>>> When a packet is received from a tunnel, this field is the >>>> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c >>>> index 008b452b8a6e..92bc1d08a813 100644 >>>> --- a/lib/netdev-native-tnl.c >>>> +++ b/lib/netdev-native-tnl.c >>>> @@ -506,6 +506,7 @@ netdev_gre_pop_header(struct dp_packet *packet) >>>> struct pkt_metadata *md = &packet->md; >>>> struct flow_tnl *tnl = &md->tunnel; >>>> int hlen = sizeof(struct eth_header) + 4; >>>> + const struct eth_header *eth; >>>> >>>> ovs_assert(data_dp); >>>> >>>> @@ -522,6 +523,11 @@ netdev_gre_pop_header(struct dp_packet *packet) >>>> goto err; >>>> } >>>> >>>> + eth = dp_packet_eth(packet); >>>> + if (eth) { >>>> + tnl->eth_type = eth->eth_type; >>>> + } >>>> + >>>> tnl_ol_pop(packet, hlen); >>>> >>>> return packet; >>>> @@ -1102,6 +1108,7 @@ netdev_vxlan_pop_header(struct dp_packet *packet) >>>> unsigned int hlen; >>>> ovs_be32 vx_flags; >>>> enum packet_type next_pt = PT_ETH; >>>> + const struct eth_header *eth; >>>> >>>> ovs_assert(packet->l3_ofs > 0); >>>> ovs_assert(packet->l4_ofs > 0); >>>> @@ -1152,6 +1159,12 @@ netdev_vxlan_pop_header(struct dp_packet *packet) >>>> tnl->flags |= FLOW_TNL_F_KEY; >>>> >>>> packet->packet_type = htonl(next_pt); >>>> + >>>> + eth = dp_packet_eth(packet); >>>> + if (eth) { >>>> + tnl->eth_type = eth->eth_type; >>>> + } >>>> + >>>> tnl_ol_pop(packet, hlen + VXLAN_HLEN); >>>> if (next_pt != PT_ETH) { >>>> packet->l3_ofs = 0; >>>> @@ -1219,6 +1232,7 @@ netdev_geneve_pop_header(struct dp_packet *packet) >>>> struct flow_tnl *tnl = &md->tunnel; >>>> struct genevehdr *gnh; >>>> unsigned int hlen, opts_len, ulen; >>>> + const struct eth_header *eth; >>>> >>>> pkt_metadata_init_tnl(md); >>>> if (GENEVE_BASE_HLEN > dp_packet_l4_size(packet)) { >>>> @@ -1260,6 +1274,12 @@ netdev_geneve_pop_header(struct dp_packet *packet) >>>> tnl->flags |= FLOW_TNL_F_UDPIF; >>>> >>>> packet->packet_type = htonl(PT_ETH); >>>> + >>>> + eth = dp_packet_eth(packet); >>>> + if (eth) { >>>> + tnl->eth_type = eth->eth_type; >>>> + } >>> This should be done in ip_extract_tnl_md(). >>> >>>> + >>>> tnl_ol_pop(packet, hlen); >>>> >>>> return packet; >>>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >>>> index ca1982ccd8fb..b8942294691f 100644 >>>> --- a/lib/netdev-offload-dpdk.c >>>> +++ b/lib/netdev-offload-dpdk.c >>>> @@ -1184,11 +1184,32 @@ parse_tnl_ip_match(struct flow_patterns *patterns, >>>> struct match *match, >>>> uint8_t proto) >>>> { >>>> + ovs_be16 eth_type = match->flow.tunnel.eth_type; >>>> struct flow *consumed_masks; >>>> + bool is_ipv4, is_ipv6; >>>> + >>>> + /* Determine IP version - either explicitly set via eth_type >>>> + * or inferred from masks. >>>> + */ >>>> + if (eth_type == htons(ETH_TYPE_IP)) { >>>> + is_ipv4 = true; >>>> + is_ipv6 = false; >>>> + } else if (eth_type == htons(ETH_TYPE_IPV6)) { >>>> + is_ipv4 = false; >>>> + is_ipv6 = true; >>>> + } else { >>>> + /* eth_type is 0 - infer from which address masks are set */ >>>> + is_ipv4 = (match->wc.masks.tunnel.ip_src || >>>> + match->wc.masks.tunnel.ip_dst); >>>> + is_ipv6 = (!is_all_zeros(&match->wc.masks.tunnel.ipv6_src, >>>> + sizeof(struct in6_addr)) || >>>> + !is_all_zeros(&match->wc.masks.tunnel.ipv6_dst, >>>> + sizeof(struct in6_addr))); >>>> + } >>>> >>>> consumed_masks = &match->wc.masks; >>>> /* IP v4 */ >>>> - if (match->wc.masks.tunnel.ip_src || match->wc.masks.tunnel.ip_dst) { >>>> + if (is_ipv4) { >>>> struct rte_flow_item_ipv4 *spec, *mask; >>>> >>>> spec = xzalloc(sizeof *spec); >>>> @@ -1212,10 +1233,7 @@ parse_tnl_ip_match(struct flow_patterns *patterns, >>>> consumed_masks->tunnel.ip_dst = 0; >>>> >>>> add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_IPV4, spec, mask, >>>> NULL); >>>> - } else if (!is_all_zeros(&match->wc.masks.tunnel.ipv6_src, >>>> - sizeof(struct in6_addr)) || >>>> - !is_all_zeros(&match->wc.masks.tunnel.ipv6_dst, >>>> - sizeof(struct in6_addr))) { >>>> + } else if (is_ipv6) { >>>> /* IP v6 */ >>>> struct rte_flow_item_ipv6 *spec, *mask; >>>> >>>> @@ -1378,6 +1396,19 @@ parse_flow_tnl_match(struct netdev *tnldev, >>>> return ret; >>>> } >>>> >>>> + if (match->wc.masks.tunnel.eth_type) { >>>> + struct rte_flow_item_eth *spec, *mask; >>>> + >>>> + spec = xzalloc(sizeof *spec); >>>> + mask = xzalloc(sizeof *mask); >>>> + >>>> + spec->type = match->flow.tunnel.eth_type; >>>> + mask->type = match->wc.masks.tunnel.eth_type; >>>> + match->wc.masks.tunnel.eth_type = 0; >>>> + >>>> + add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, mask, >>>> NULL); >>>> + } >>> This should probbaly be in parse_tnl_ip_match(). >>> >>>> + >>>> if (!strcmp(netdev_get_type(tnldev), "vxlan")) { >>>> ret = parse_vxlan_match(patterns, match); >>>> } >>>> diff --git a/lib/odp-util.h b/lib/odp-util.h >>>> index 339ffc14f248..66b5339ed2ba 100644 >>>> --- a/lib/odp-util.h >>>> +++ b/lib/odp-util.h >>>> @@ -207,7 +207,10 @@ int odp_flow_from_string(const char *s, const struct >>>> simap *port_names, >>>> >>>> \ >>>> /* If true, it means that the datapath supports the IPv6 Neigh >>>> \ >>>> * Discovery Extension bits. */ >>>> \ >>>> - ODP_SUPPORT_FIELD(bool, nd_ext, "IPv6 ND Extension") >>>> + ODP_SUPPORT_FIELD(bool, nd_ext, "IPv6 ND Extension") >>>> \ >>>> + >>>> \ >>>> + /* Tunnel ethernet type matching supported. */ >>>> \ >>>> + ODP_SUPPORT_FIELD(bool, tun_eth_type, "Tunnel Ethernet Type") >>>> >>>> /* Indicates support for various fields. This defines how flows will be >>>> * serialised. */ >>>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c >>>> index 2c8197fb7307..7350d141c906 100644 >>>> --- a/ofproto/ofproto-dpif-xlate.c >>>> +++ b/ofproto/ofproto-dpif-xlate.c >>>> @@ -8081,6 +8081,8 @@ xlate_wc_init(struct xlate_ctx *ctx) >>>> static void >>>> xlate_wc_finish(struct xlate_ctx *ctx) >>>> { >>>> + struct ofproto_dpif *ofproto = ctx->xbridge->ofproto; >>>> + struct dpif_backer *backer = ofproto->backer; >>>> int i; >>>> >>>> /* Clear the metadata and register wildcard masks, because we won't >>>> @@ -8152,6 +8154,20 @@ xlate_wc_finish(struct xlate_ctx *ctx) >>>> if (!flow_tnl_dst_is_set(&ctx->xin->upcall_flow->tunnel)) { >>>> memset(&ctx->wc->masks.tunnel, 0, sizeof ctx->wc->masks.tunnel); >>>> } >>>> + /* Clear tunnel eth type if not supported. */ >>>> + if (!backer->rt_support.odp.tun_eth_type) { >>>> + ctx->wc->masks.tunnel.eth_type = 0; >>>> + } >>>> + /* Clear IPv6 artifacts when eth_type explicitly indicates IPv4 */ >>>> + if (ctx->wc->masks.tunnel.eth_type == htons(ETH_TYPE_IP)) { >>> We shouldn't compare the mask here. It should be the value in the >>> upcall_flow instead. And we should not clear anything if eth_type >>> is not exact matched. >>> >>>> + ctx->wc->masks.tunnel.ipv6_src = in6addr_any; >>>> + ctx->wc->masks.tunnel.ipv6_dst = in6addr_any; >>>> + } >>>> + /* Clear IPv4 artifacts when eth_type explicitly indicates IPv6 */ >>>> + if (ctx->wc->masks.tunnel.eth_type == htons(ETH_TYPE_IPV6)) { >>> Same here. >>> >>>> + ctx->wc->masks.tunnel.ip_src = 0; >>>> + ctx->wc->masks.tunnel.ip_dst = 0; >>>> + } >>>> } >>>> >>>> /* This will tweak the odp actions generated. For now, it will: >>>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c >>>> index 4a200dd08828..c1b3474a1b56 100644 >>>> --- a/ofproto/ofproto-dpif.c >>>> +++ b/ofproto/ofproto-dpif.c >>>> @@ -1759,6 +1759,8 @@ check_support(struct dpif_backer *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); >>>> + backer->rt_support.odp.tun_eth_type = >>>> + dpif_supports_tun_eth_type(backer->dpif); >>>> } >>>> >>>> /* TC does not support offloading the explicit drop action. As such we >>>> need to >>>> @@ -5924,6 +5926,7 @@ get_datapath_cap(const char *datapath_type, struct >>>> smap *cap) >>>> smap_add(cap, "ct_orig_tuple", s->odp.ct_orig_tuple ? "true" : >>>> "false"); >>>> smap_add(cap, "ct_orig_tuple6", s->odp.ct_orig_tuple6 ? "true" : >>>> "false"); >>>> smap_add(cap, "nd_ext", s->odp.nd_ext ? "true" : "false"); >>>> + smap_add(cap, "tun_eth_type", s->odp.tun_eth_type ? "true" : "false"); >>>> >>>> /* DPIF_SUPPORT_FIELDS */ >>>> smap_add(cap, "masked_set_action", >>>> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c >>>> index f067a6c26c19..4adc87b91baa 100644 >>>> --- a/ofproto/tunnel.c >>>> +++ b/ofproto/tunnel.c >>>> @@ -381,6 +381,7 @@ tnl_wc_init(struct flow *flow, struct flow_wildcards >>>> *wc) >>>> * wildcarded, not to unwildcard them here. */ >>>> wc->masks.tunnel.tp_src = 0; >>>> wc->masks.tunnel.tp_dst = 0; >>>> + wc->masks.tunnel.eth_type = OVS_BE16_MAX; >>> We should not match on this if it is not needed in the final flow. It's >>> better if we unwildcard it later when we know that we actually need to >>> match on this field, as it may not be supported. And if we have to match, >>> but it is not supported, then we'd need to do a SLOW_MATCH. >> Where is "later"? xlate_wc_finish()? Anywhere else? We couldn't find >> better place. > xlate_wc_finish is probbaly the place to go. > >> Also regarding the SLOW_MATCH: Why needed? Wouldn't this break kernel >> datapath tunnel offloads since kernel doesn't support tun_eth_type? > It will. That's why we need to add it only when necessary. If it's > not necessary, and it's not supported, then don't match on it. But > if it is the only way to distinguish the traffic properly, then we > have to send these packets to userspace for matching. > > But also, we'll need to add support to the kernel once we figure out > the end design here. > > Best regards, Ilya Maximets. > >>>> if (is_ip_any(flow) >>>> && IP_ECN_is_ce(flow->tunnel.ip_tos)) { >>>> diff --git a/tests/bfd.at b/tests/bfd.at >>>> index f5c6409f6c65..aea0a3392325 100644 >>>> --- a/tests/bfd.at >>>> +++ b/tests/bfd.at >>>> @@ -1127,7 +1127,7 @@ bridge("br0") >>>> -> no learned MAC for destination, flooding >>>> >>>> Final flow: unchanged >>>> -Megaflow: >>>> recirc_id=0,eth,udp,tun_id=0,tun_src=2.2.2.2,tun_dst=2.2.2.1,tun_tos=0,tun_flags=-df-csum+key,in_port=1,dl_src=00:11:22:33:44:66,dl_dst=00:23:20:00:00:77,nw_frag=no,tp_dst=3784 >>>> +Megaflow: >>>> recirc_id=0,eth,udp,tun_id=0,tun_src=2.2.2.2,tun_dst=2.2.2.1,tun_tos=0,tun_eth_type=0x0000,tun_flags=-df-csum+key,in_port=1,dl_src=00:11:22:33:44:66,dl_dst=00:23:20:00:00:77,nw_frag=no,tp_dst=3784 >>> Not sure why we're matching on tun_eth_type being zero here. It's clearly >>> incorrect, as the real type is IPv4. Hence this flow will never properly >>> match on the actual traffic. We need to either provide the value to the >>> call or infer it from the other data we have. >>> >>> Same for all the tests below. >>> >>> Also, would be good to see a few tests specific to the new functionality >>> introduced in this patch. >>> >>> Best regards, Ilya Maximets. >> Best regards, >> >> Reuven Plevinsky >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
