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]>
---
1. Please note this is a draft-only proposal. We didn't take care on
all tunnel types, testsuite etc.
2. About backward compatibility: You mentioned that today OpenFlow
allows setting both tunnel IPv4 and IPv6 fields simultaneously.
While that is true technically, there is no valid use case or
semantic meaning to matching both address types at once. Matching
on IPv4 and IPv6 together simply does not make sense from a protocol
perspective. So, in practice, I would argue that this is not a real
compatibility issue, because the "dual set" case is already invalid.
3. About dependencies and pipeline design: I understand the point about
dependencies and the mismatch with OpenFlow's "all fields can be set"
model. But the key here is that the proposed tunnel_eth_type is an
internal field. It is not exposed directly in OpenFlow as a
user-visible match key. Instead, it provides internal consistency
for the datapath and avoids ambiguity in cases where the tunnel IP
version matters. Since it is internal, existing OpenFlow semantics
remain untouched, and no user-facing behavior is broken.
4. If we go forward with this approach, more commits will follow to
handle offloads and cleanups of "dual set", that as mentioned it is
already invalid.

include/openvswitch/match.h | 1 +
include/openvswitch/meta-flow.h | 14 ++++++++++++++
include/openvswitch/packets.h | 3 ++-
lib/match.c | 8 ++++++++
lib/meta-flow.c | 18 ++++++++++++++++++
lib/meta-flow.xml | 1 +
lib/netdev-native-tnl.c | 15 +++++++++++++++
lib/netdev-offload-dpdk.c | 3 +++
lib/netdev-offload-tc.c | 2 ++
ofproto/tunnel.c | 1 +
10 files changed, 65 insertions(+), 1 deletion(-)

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 fb7d17ebe736..cfdd286d8bf1 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.
+ * 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/match.c b/lib/match.c
index 9b7e06e0c7f8..e6681db82a05 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)
{
@@ -1398,6 +1405,7 @@ format_flow_tunnel(struct ds *s, const struct 
match *match)
FLOW_TNL_F_MASK);
ds_put_char(s, ',');
}
+ ds_put_format(s, "tun_eth_type=0x%04"PRIx16",", ntohs(tnl->eth_type));
tun_metadata_match_format(s, match);
}
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index b03fe7abf1d8..e2c870a126d4 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -211,6 +211,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:
@@ -522,6 +524,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:
@@ -678,6 +681,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;
@@ -1015,6 +1021,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;
@@ -1437,6 +1446,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;
@@ -1821,6 +1833,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:
@@ -1915,6 +1928,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:
@@ -2073,6 +2087,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;
@@ -2477,6 +2494,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 ac72a44bce40..45225f3ad351 100644
--- a/lib/meta-flow.xml
+++ b/lib/meta-flow.xml
@@ -1510,6 +1510,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 e172ed72e447..5bf56f480aa0 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -524,6 +524,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);
@@ -540,6 +541,10 @@ netdev_gre_pop_header(struct dp_packet *packet)
goto err;
}
+ eth = dp_packet_eth(packet);
+ if (eth) {
+ tnl->eth_type = eth->eth_type;
+ }
dp_packet_reset_packet(packet, hlen);
return packet;
@@ -1097,6 +1102,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);
@@ -1147,6 +1153,10 @@ 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;
+ }
dp_packet_reset_packet(packet, hlen + VXLAN_HLEN);
if (next_pt != PT_ETH) {
packet->l3_ofs = 0;
@@ -1214,6 +1224,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)) {
@@ -1255,6 +1266,10 @@ 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;
+ }
dp_packet_reset_packet(packet, hlen);
return packet;
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index f963bef2280b..7ce9da21b0f3 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -1367,6 +1367,9 @@ parse_flow_tnl_match(struct netdev *tnldev,
return ret;
}
+ /* Temporary ignore. */
+ match->wc.masks.tunnel.eth_type = 0;
+
if (!strcmp(netdev_get_type(tnldev), "vxlan")) {
ret = parse_vxlan_match(patterns, match);
}
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 66f2558ddb71..0b56b650b69f 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -2324,6 +2324,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct 
match *match,
memset(&tnl_mask->gbp_id, 0, sizeof tnl_mask->gbp_id);
memset(&tnl_mask->gbp_flags, 0, sizeof tnl_mask->gbp_flags);
tnl_mask->flags &= ~FLOW_TNL_F_KEY;
+ /* Tunnel's eth_type is for userspace only. Ignore it here. */
+ tnl_mask->eth_type = 0;
/* XXX: This is wrong! We're ignoring DF and CSUM flags configuration
* requested by the user. However, TC for now has no way to pass
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 80ddee78acf6..c63332051078 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;
if (is_ip_any(flow)
&& IP_ECN_is_ce(flow->tunnel.ip_tos)) {

-- 
2.34.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to