Skip to contentSkip to conversation

vxlan: Support user-defined reserved bits

PATCH Applied Petr Machata 3 participants 24 messages Dec 5, 2024 – Dec 9, 2024

Subsystems (4)
Networking Network drivers Kselftest IPv4/IPv6
lore
Share & tools
lore

Petr Machata

author
#0 lore raw

v2 cover vxlan: Support user-defined reserved bits

Currently the VXLAN header validation works by vxlan_rcv() going feature by feature, each feature clearing the bits that it consumes. If anything is left unparsed at the end, the packet is rejected.

Unfortunately there are machines out there that send VXLAN packets with reserved bits set, even if they are configured to not use the corresponding features. One such report is here[1], and we have heard similar complaints from our customers as well.

This patchset adds an attribute that makes it configurable which bits the user wishes to tolerate and which they consider reserved. This was recommended in [1] as well.

A knob like that inevitably allows users to set as reserved bits that are in fact required for the features enabled by the netdevice, such as GPE. This is detected, and such configurations are rejected.

In patches #1..#7, the reserved bits validation code is gradually moved away from the unparsed approach described above, to one where a given set of valid bits is precomputed and then the packet is validated against that.

In patch #8, this precomputed set is made configurable through a new attribute IFLA_VXLAN_RESERVED_BITS.

Patches #9 and #10 massage the testsuite a bit, so that patch #11 can introduce a selftest for the resreved bits feature.

The corresponding iproute2 support is available in [2].

[1] https://lore.kernel.org/netdev/[email protected]/
[2] https://github.com/pmachata/iproute2/commits/vxlan_reserved_bits/

v2:
- Patch #11:
- Add the new test to Makefile

v1 (vs. RFC):
- No changes.

Petr Machata (11):
vxlan: In vxlan_rcv(), access flags through the vxlan netdevice
vxlan: vxlan_rcv() callees: Move clearing of unparsed flags out
vxlan: vxlan_rcv() callees: Drop the unparsed argument
vxlan: vxlan_rcv(): Extract vxlan_hdr(skb) to a named variable
vxlan: Track reserved bits explicitly as part of the configuration
vxlan: Bump error counters for header mismatches
vxlan: vxlan_rcv(): Drop unparsed
vxlan: Add an attribute to make VXLAN header validation configurable
selftests: net: lib: Rename ip_link_master() to ip_link_set_master()
selftests: net: lib: Add several autodefer helpers
selftests: forwarding: Add a selftest for the new reserved_bits UAPI

 drivers/net/vxlan/vxlan_core.c                | 150 +++++---
 include/net/vxlan.h                           |   1 +
 include/uapi/linux/if_link.h                  |   1 +
 tools/testing/selftests/net/fdb_notify.sh     |   6 +-
 .../testing/selftests/net/forwarding/Makefile |   1 +
 tools/testing/selftests/net/forwarding/lib.sh |   7 -
 .../net/forwarding/vxlan_reserved.sh          | 352 ++++++++++++++++++
 tools/testing/selftests/net/lib.sh            |  41 +-
 8 files changed, 497 insertions(+), 62 deletions(-)
 create mode 100755 tools/testing/selftests/net/forwarding/vxlan_reserved.sh
Signature · 1 line
2.47.0
author
#1 lore raw

PATCH v2 1/11 vxlan: In vxlan_rcv(), access flags through the vxlan netdevice

vxlan_sock.flags is constructed from vxlan_dev.cfg.flags, as the subset of flags (named VXLAN_F_RCV_FLAGS) that is important from the point of view of socket sharing. Attempts to reconfigure these flags during the vxlan netdev lifetime are also bounced. It is therefore immaterial whether we access the flags through the vxlan_dev or through the socket.

Convert the socket accesses to netdevice accesses in this separate patch to make the conversions that take place in the following patches more obvious.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

 drivers/net/vxlan/vxlan_core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/net/vxlan/vxlan_core.c +5 -5
index 4053bd3f1023..d07d86ac1f03 100644
@@ -1717,7 +1717,7 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 	/* For backwards compatibility, only allow reserved fields to be
 	 * used by VXLAN extensions if explicitly requested.
 	 */
-	if (vs->flags & VXLAN_F_GPE) {
+	if (vxlan->cfg.flags & VXLAN_F_GPE) {
 		if (!vxlan_parse_gpe_proto(&unparsed, &protocol))
 			goto drop;
 		unparsed.vx_flags &= ~VXLAN_GPE_USED_BITS;

@@ -1730,8 +1730,8 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		goto drop;
 	}
 
-	if (vs->flags & VXLAN_F_REMCSUM_RX) {
-		reason = vxlan_remcsum(&unparsed, skb, vs->flags);
+	if (vxlan->cfg.flags & VXLAN_F_REMCSUM_RX) {
+		reason = vxlan_remcsum(&unparsed, skb, vxlan->cfg.flags);
 		if (unlikely(reason))
 			goto drop;
 	}

@@ -1756,8 +1756,8 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		memset(md, 0, sizeof(*md));
 	}
 
-	if (vs->flags & VXLAN_F_GBP)
-		vxlan_parse_gbp_hdr(&unparsed, skb, vs->flags, md);
+	if (vxlan->cfg.flags & VXLAN_F_GBP)
+		vxlan_parse_gbp_hdr(&unparsed, skb, vxlan->cfg.flags, md);
 	/* Note that GBP and GPE can never be active together. This is
 	 * ensured in vxlan_dev_configure.
 	 */
Signature · 1 line
2.47.0
Reviewed-by on v2 1/11
#2 lore raw

Re: [PATCH net-next v2 01/11] vxlan: In vxlan_rcv(), access flags through the vxlan netdevice

Quoted 13 lines · Petr Machata

vxlan_sock.flags is constructed from vxlan_dev.cfg.flags, as the subset of flags (named VXLAN_F_RCV_FLAGS) that is important from the point of view of socket sharing. Attempts to reconfigure these flags during the vxlan netdev lifetime are also bounced. It is therefore immaterial whether we access the flags through the vxlan_dev or through the socket.

Convert the socket accesses to netdevice accesses in this separate patch to make the conversions that take place in the following patches more obvious.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Quoted 9 lines

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

 drivers/net/vxlan/vxlan_core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by Nikolay Aleksandrov <[email protected]>
author
#3 lore raw

PATCH v2 2/11 vxlan: vxlan_rcv() callees: Move clearing of unparsed flags out

In reply to Petr Machata

In order to migrate away from the use of unparsed to detect invalid flags, move all the code that actually clears the flags from callees directly to vxlan_rcv().

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

 drivers/net/vxlan/vxlan_core.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
drivers/net/vxlan/vxlan_core.c +7 -9
index d07d86ac1f03..ff653b95a6d5 100644
@@ -1562,7 +1562,7 @@ static enum skb_drop_reason vxlan_remcsum(struct vxlanhdr *unparsed,
 	size_t start, offset;
 
 	if (!(unparsed->vx_flags & VXLAN_HF_RCO) || skb->remcsum_offload)
-		goto out;
+		return SKB_NOT_DROPPED_YET;
 
 	start = vxlan_rco_start(unparsed->vx_vni);
 	offset = start + vxlan_rco_offset(unparsed->vx_vni);

@@ -1573,10 +1573,6 @@ static enum skb_drop_reason vxlan_remcsum(struct vxlanhdr *unparsed,
 
 	skb_remcsum_process(skb, (void *)(vxlan_hdr(skb) + 1), start, offset,
 			    !!(vxflags & VXLAN_F_REMCSUM_NOPARTIAL));
-out:
-	unparsed->vx_flags &= ~VXLAN_HF_RCO;
-	unparsed->vx_vni &= VXLAN_VNI_MASK;
-
 	return SKB_NOT_DROPPED_YET;
 }
 

@@ -1588,7 +1584,7 @@ static void vxlan_parse_gbp_hdr(struct vxlanhdr *unparsed,
 	struct metadata_dst *tun_dst;
 
 	if (!(unparsed->vx_flags & VXLAN_HF_GBP))
-		goto out;
+		return;
 
 	md->gbp = ntohs(gbp->policy_id);
 

@@ -1607,8 +1603,6 @@ static void vxlan_parse_gbp_hdr(struct vxlanhdr *unparsed,
 	/* In flow-based mode, GBP is carried in dst_metadata */
 	if (!(vxflags & VXLAN_F_COLLECT_METADATA))
 		skb->mark = md->gbp;
-out:
-	unparsed->vx_flags &= ~VXLAN_GBP_USED_BITS;
 }
 
 static enum skb_drop_reason vxlan_set_mac(struct vxlan_dev *vxlan,

@@ -1734,6 +1728,8 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		reason = vxlan_remcsum(&unparsed, skb, vxlan->cfg.flags);
 		if (unlikely(reason))
 			goto drop;
+		unparsed.vx_flags &= ~VXLAN_HF_RCO;
+		unparsed.vx_vni &= VXLAN_VNI_MASK;
 	}
 
 	if (vxlan_collect_metadata(vs)) {

@@ -1756,8 +1752,10 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		memset(md, 0, sizeof(*md));
 	}
 
-	if (vxlan->cfg.flags & VXLAN_F_GBP)
+	if (vxlan->cfg.flags & VXLAN_F_GBP) {
 		vxlan_parse_gbp_hdr(&unparsed, skb, vxlan->cfg.flags, md);
+		unparsed.vx_flags &= ~VXLAN_GBP_USED_BITS;
+	}
 	/* Note that GBP and GPE can never be active together. This is
 	 * ensured in vxlan_dev_configure.
 	 */
Signature · 1 line
2.47.0
Reviewed-by on v2 2/11
#4 lore raw

Re: [PATCH net-next v2 02/11] vxlan: vxlan_rcv() callees: Move clearing of unparsed flags out

Quoted 6 lines · Petr Machata

In order to migrate away from the use of unparsed to detect invalid flags, move all the code that actually clears the flags from callees directly to vxlan_rcv().

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Quoted 8 lines

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

Reviewed-by Nikolay Aleksandrov <[email protected]>
author
#5 lore raw

PATCH v2 3/11 vxlan: vxlan_rcv() callees: Drop the unparsed argument

In reply to Petr Machata

The functions vxlan_remcsum() and vxlan_parse_gbp_hdr() take both the SKB and the unparsed VXLAN header. Now that unparsed adjustment is handled directly by vxlan_rcv(), drop this argument, and have the function derive it from the SKB on its own.

vxlan_parse_gpe_proto() does not take SKB, so keep the header parameter. However const it so that it's clear that the intention is that it does not get changed.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

 drivers/net/vxlan/vxlan_core.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)
drivers/net/vxlan/vxlan_core.c +16 -15
index ff653b95a6d5..4905ed1c5e20 100644
@@ -622,9 +622,9 @@ static int vxlan_fdb_append(struct vxlan_fdb *f,
 	return 1;
 }
 
-static bool vxlan_parse_gpe_proto(struct vxlanhdr *hdr, __be16 *protocol)
+static bool vxlan_parse_gpe_proto(const struct vxlanhdr *hdr, __be16 *protocol)
 {
-	struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)hdr;
+	const struct vxlanhdr_gpe *gpe = (const struct vxlanhdr_gpe *)hdr;
 
 	/* Need to have Next Protocol set for interfaces in GPE mode. */
 	if (!gpe->np_applied)

@@ -1554,18 +1554,17 @@ static void vxlan_sock_release(struct vxlan_dev *vxlan)
 #endif
 }
 
-static enum skb_drop_reason vxlan_remcsum(struct vxlanhdr *unparsed,
-					  struct sk_buff *skb,
-					  u32 vxflags)
+static enum skb_drop_reason vxlan_remcsum(struct sk_buff *skb, u32 vxflags)
 {
+	const struct vxlanhdr *vh = vxlan_hdr(skb);
 	enum skb_drop_reason reason;
 	size_t start, offset;
 
-	if (!(unparsed->vx_flags & VXLAN_HF_RCO) || skb->remcsum_offload)
+	if (!(vh->vx_flags & VXLAN_HF_RCO) || skb->remcsum_offload)
 		return SKB_NOT_DROPPED_YET;
 
-	start = vxlan_rco_start(unparsed->vx_vni);
-	offset = start + vxlan_rco_offset(unparsed->vx_vni);
+	start = vxlan_rco_start(vh->vx_vni);
+	offset = start + vxlan_rco_offset(vh->vx_vni);
 
 	reason = pskb_may_pull_reason(skb, offset + sizeof(u16));
 	if (reason)

@@ -1576,14 +1575,16 @@ static enum skb_drop_reason vxlan_remcsum(struct vxlanhdr *unparsed,
 	return SKB_NOT_DROPPED_YET;
 }
 
-static void vxlan_parse_gbp_hdr(struct vxlanhdr *unparsed,
-				struct sk_buff *skb, u32 vxflags,
+static void vxlan_parse_gbp_hdr(struct sk_buff *skb, u32 vxflags,
 				struct vxlan_metadata *md)
 {
-	struct vxlanhdr_gbp *gbp = (struct vxlanhdr_gbp *)unparsed;
+	const struct vxlanhdr *vh = vxlan_hdr(skb);
+	const struct vxlanhdr_gbp *gbp;
 	struct metadata_dst *tun_dst;
 
-	if (!(unparsed->vx_flags & VXLAN_HF_GBP))
+	gbp = (const struct vxlanhdr_gbp *)vh;
+
+	if (!(vh->vx_flags & VXLAN_HF_GBP))
 		return;
 
 	md->gbp = ntohs(gbp->policy_id);

@@ -1712,7 +1713,7 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 	 * used by VXLAN extensions if explicitly requested.
 	 */
 	if (vxlan->cfg.flags & VXLAN_F_GPE) {
-		if (!vxlan_parse_gpe_proto(&unparsed, &protocol))
+		if (!vxlan_parse_gpe_proto(vxlan_hdr(skb), &protocol))
 			goto drop;
 		unparsed.vx_flags &= ~VXLAN_GPE_USED_BITS;
 		raw_proto = true;

@@ -1725,7 +1726,7 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 	}
 
 	if (vxlan->cfg.flags & VXLAN_F_REMCSUM_RX) {
-		reason = vxlan_remcsum(&unparsed, skb, vxlan->cfg.flags);
+		reason = vxlan_remcsum(skb, vxlan->cfg.flags);
 		if (unlikely(reason))
 			goto drop;
 		unparsed.vx_flags &= ~VXLAN_HF_RCO;

@@ -1753,7 +1754,7 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 	}
 
 	if (vxlan->cfg.flags & VXLAN_F_GBP) {
-		vxlan_parse_gbp_hdr(&unparsed, skb, vxlan->cfg.flags, md);
+		vxlan_parse_gbp_hdr(skb, vxlan->cfg.flags, md);
 		unparsed.vx_flags &= ~VXLAN_GBP_USED_BITS;
 	}
 	/* Note that GBP and GPE can never be active together. This is
Signature · 1 line
2.47.0
Reviewed-by on v2 3/11
#6 lore raw

Re: [PATCH net-next v2 03/11] vxlan: vxlan_rcv() callees: Drop the unparsed argument

Quoted 11 lines · Petr Machata

The functions vxlan_remcsum() and vxlan_parse_gbp_hdr() take both the SKB and the unparsed VXLAN header. Now that unparsed adjustment is handled directly by vxlan_rcv(), drop this argument, and have the function derive it from the SKB on its own.

vxlan_parse_gpe_proto() does not take SKB, so keep the header parameter. However const it so that it's clear that the intention is that it does not get changed.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Quoted 8 lines

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

Reviewed-by Nikolay Aleksandrov <[email protected]>
author
#7 lore raw

PATCH v2 4/11 vxlan: vxlan_rcv(): Extract vxlan_hdr(skb) to a named variable

In reply to Petr Machata

Having a named reference to the VXLAN header is more handy than having to conjure it anew through vxlan_hdr() on every use. Add a new variable and convert several open-coded sites.

Additionally, convert one "unparsed" use to the new variable as well. Thus the only "unparsed" uses that remain are the flag-clearing and the header validity check at the end.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Reviewed-by Mateusz Polchlopek <[email protected]>

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

 drivers/net/vxlan/vxlan_core.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/net/vxlan/vxlan_core.c +6 -5
index 4905ed1c5e20..257411d1ccca 100644
@@ -1667,6 +1667,7 @@ static bool vxlan_ecn_decapsulate(struct vxlan_sock *vs, void *oiph,
 static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 {
 	struct vxlan_vni_node *vninode = NULL;
+	const struct vxlanhdr *vh;
 	struct vxlan_dev *vxlan;
 	struct vxlan_sock *vs;
 	struct vxlanhdr unparsed;

@@ -1685,11 +1686,11 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		goto drop;
 
 	unparsed = *vxlan_hdr(skb);
+	vh = vxlan_hdr(skb);
 	/* VNI flag always required to be set */
-	if (!(unparsed.vx_flags & VXLAN_HF_VNI)) {
+	if (!(vh->vx_flags & VXLAN_HF_VNI)) {
 		netdev_dbg(skb->dev, "invalid vxlan flags=%#x vni=%#x\n",
-			   ntohl(vxlan_hdr(skb)->vx_flags),
-			   ntohl(vxlan_hdr(skb)->vx_vni));
+			   ntohl(vh->vx_flags), ntohl(vh->vx_vni));
 		reason = SKB_DROP_REASON_VXLAN_INVALID_HDR;
 		/* Return non vxlan pkt */
 		goto drop;

@@ -1701,7 +1702,7 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 	if (!vs)
 		goto drop;
 
-	vni = vxlan_vni(vxlan_hdr(skb)->vx_vni);
+	vni = vxlan_vni(vh->vx_vni);
 
 	vxlan = vxlan_vs_find_vni(vs, skb->dev->ifindex, vni, &vninode);
 	if (!vxlan) {

@@ -1713,7 +1714,7 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 	 * used by VXLAN extensions if explicitly requested.
 	 */
 	if (vxlan->cfg.flags & VXLAN_F_GPE) {
-		if (!vxlan_parse_gpe_proto(vxlan_hdr(skb), &protocol))
+		if (!vxlan_parse_gpe_proto(vh, &protocol))
 			goto drop;
 		unparsed.vx_flags &= ~VXLAN_GPE_USED_BITS;
 		raw_proto = true;
Signature · 1 line
2.47.0
Reviewed-by on v2 4/11
#8 lore raw

Re: [PATCH net-next v2 04/11] vxlan: vxlan_rcv(): Extract vxlan_hdr(skb) to a named variable

Quoted 11 lines · Petr Machata

Having a named reference to the VXLAN header is more handy than having to conjure it anew through vxlan_hdr() on every use. Add a new variable and convert several open-coded sites.

Additionally, convert one "unparsed" use to the new variable as well. Thus the only "unparsed" uses that remain are the flag-clearing and the header validity check at the end.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Reviewed-by Mateusz Polchlopek <[email protected]>
Quoted 8 lines

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

Reviewed-by Nikolay Aleksandrov <[email protected]>
author
#9 lore raw

PATCH v2 5/11 vxlan: Track reserved bits explicitly as part of the configuration

In reply to Petr Machata

In order to make it possible to configure which bits in VXLAN header should be considered reserved, introduce a new field vxlan_config::reserved_bits. Have it cover the whole header, except for the VNI-present bit and the bits for VNI itself, and have individual enabled features clear more bits off reserved_bits.

(This is expressed as first constructing a used_bits set, and then inverting it to get the reserved_bits. The set of used_bits will be useful on its own for validation of user-set reserved_bits in a following patch.)

The patch also moves a comment relevant to the validation from the unparsed validation site up to the new site. Logically this patch should add the new comment, and a later patch that removes the unparsed bits would remove the old comment. But keeping both legs in the same patch is better from the history spelunking point of view.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

 drivers/net/vxlan/vxlan_core.c | 41 +++++++++++++++++++++++++---------
 include/net/vxlan.h            |  1 +
 2 files changed, 31 insertions(+), 11 deletions(-)
drivers/net/vxlan/vxlan_core.c +30 -11
index 257411d1ccca..f6118de81b8a 100644
@@ -1710,9 +1710,20 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		goto drop;
 	}
 
-	/* For backwards compatibility, only allow reserved fields to be
-	 * used by VXLAN extensions if explicitly requested.
-	 */
+	if (vh->vx_flags & vxlan->cfg.reserved_bits.vx_flags ||
+	    vh->vx_vni & vxlan->cfg.reserved_bits.vx_vni) {
+		/* If the header uses bits besides those enabled by the
+		 * netdevice configuration, treat this as a malformed packet.
+		 * This behavior diverges from VXLAN RFC (RFC7348) which
+		 * stipulates that bits in reserved in reserved fields are to be
+		 * ignored. The approach here maintains compatibility with
+		 * previous stack code, and also is more robust and provides a
+		 * little more security in adding extensions to VXLAN.
+		 */
+		reason = SKB_DROP_REASON_VXLAN_INVALID_HDR;
+		goto drop;
+	}
+
 	if (vxlan->cfg.flags & VXLAN_F_GPE) {
 		if (!vxlan_parse_gpe_proto(vh, &protocol))
 			goto drop;

@@ -1763,14 +1774,6 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 	 */
 
 	if (unparsed.vx_flags || unparsed.vx_vni) {
-		/* If there are any unprocessed flags remaining treat
-		 * this as a malformed packet. This behavior diverges from
-		 * VXLAN RFC (RFC7348) which stipulates that bits in reserved
-		 * in reserved fields are to be ignored. The approach here
-		 * maintains compatibility with previous stack code, and also
-		 * is more robust and provides a little more security in
-		 * adding extensions to VXLAN.
-		 */
 		reason = SKB_DROP_REASON_VXLAN_INVALID_HDR;
 		goto drop;
 	}

@@ -4080,6 +4083,10 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 			 struct net_device *dev, struct vxlan_config *conf,
 			 bool changelink, struct netlink_ext_ack *extack)
 {
+	struct vxlanhdr used_bits = {
+		.vx_flags = VXLAN_HF_VNI,
+		.vx_vni = VXLAN_VNI_MASK,
+	};
 	struct vxlan_dev *vxlan = netdev_priv(dev);
 	int err = 0;
 

@@ -4306,6 +4313,8 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 				    extack);
 		if (err)
 			return err;
+		used_bits.vx_flags |= VXLAN_HF_RCO;
+		used_bits.vx_vni |= ~VXLAN_VNI_MASK;
 	}
 
 	if (data[IFLA_VXLAN_GBP]) {

@@ -4313,6 +4322,7 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 				    VXLAN_F_GBP, changelink, false, extack);
 		if (err)
 			return err;
+		used_bits.vx_flags |= VXLAN_GBP_USED_BITS;
 	}
 
 	if (data[IFLA_VXLAN_GPE]) {

@@ -4321,8 +4331,17 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 				    extack);
 		if (err)
 			return err;
+
+		used_bits.vx_flags |= VXLAN_GPE_USED_BITS;
 	}
 
+	/* For backwards compatibility, only allow reserved fields to be
+	 * used by VXLAN extensions if explicitly requested.
+	 */
+	conf->reserved_bits = (struct vxlanhdr) {
+		.vx_flags = ~used_bits.vx_flags,
+		.vx_vni = ~used_bits.vx_vni,
+	};
 	if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) {
 		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_NOPARTIAL,
 				    VXLAN_F_REMCSUM_NOPARTIAL, changelink,
include/net/vxlan.h +1 -0
index 33ba6fc151cf..2dd23ee2bacd 100644
@@ -227,6 +227,7 @@ struct vxlan_config {
 	unsigned int			addrmax;
 	bool				no_share;
 	enum ifla_vxlan_df		df;
+	struct vxlanhdr			reserved_bits;
 };
 
 enum {
Signature · 1 line
2.47.0
Reviewed-by on v2 5/11
#10 lore raw

Re: [PATCH net-next v2 05/11] vxlan: Track reserved bits explicitly as part of the configuration

Quoted 26 lines · Petr Machata

In order to make it possible to configure which bits in VXLAN header should be considered reserved, introduce a new field vxlan_config::reserved_bits. Have it cover the whole header, except for the VNI-present bit and the bits for VNI itself, and have individual enabled features clear more bits off reserved_bits.

(This is expressed as first constructing a used_bits set, and then inverting it to get the reserved_bits. The set of used_bits will be useful on its own for validation of user-set reserved_bits in a following patch.)

The patch also moves a comment relevant to the validation from the unparsed validation site up to the new site. Logically this patch should add the new comment, and a later patch that removes the unparsed bits would remove the old comment. But keeping both legs in the same patch is better from the history spelunking point of view.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

Quoted 4 lines
 drivers/net/vxlan/vxlan_core.c | 41 +++++++++++++++++++++++++---------
 include/net/vxlan.h            |  1 +
 2 files changed, 31 insertions(+), 11 deletions(-)

One very minor nit below, if there's another version. :)

Reviewed-by Nikolay Aleksandrov <[email protected]>
Quoted 4 lines
drivers/net/vxlan/vxlan_core.c +0 -0
index 257411d1ccca..f6118de81b8a 100644

[snip]

Quoted 29 lines
drivers/net/vxlan/vxlan_core.c +7 -0
@@ -4080,6 +4083,10 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 			 struct net_device *dev, struct vxlan_config *conf,
 			 bool changelink, struct netlink_ext_ack *extack)
 {
+	struct vxlanhdr used_bits = {
+		.vx_flags = VXLAN_HF_VNI,
+		.vx_vni = VXLAN_VNI_MASK,
+	};
 	struct vxlan_dev *vxlan = netdev_priv(dev);
 	int err = 0;
 

@@ -4306,6 +4313,8 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 				    extack);
 		if (err)
 			return err;
+		used_bits.vx_flags |= VXLAN_HF_RCO;
+		used_bits.vx_vni |= ~VXLAN_VNI_MASK;
 	}
 
 	if (data[IFLA_VXLAN_GBP]) {

@@ -4313,6 +4322,7 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 				    VXLAN_F_GBP, changelink, false, extack);
 		if (err)
 			return err;
+		used_bits.vx_flags |= VXLAN_GBP_USED_BITS;
 	}
 
 	if (data[IFLA_VXLAN_GPE]) {

drivers/net/vxlan/vxlan_core.c static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], · line 4334

Quoted 4 lines
drivers/net/vxlan/vxlan_core.c +1 -0
@@ -4321,8 +4331,17 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 				    extack);
 		if (err)
 			return err;
+

minor nit: extra newline here, there isn't one above for GBP

Quoted 2 lines
drivers/net/vxlan/vxlan_core.c +1 -0
+		used_bits.vx_flags |= VXLAN_GPE_USED_BITS;
 	}
author
#11 lore raw

PATCH v2 6/11 vxlan: Bump error counters for header mismatches

In reply to Petr Machata

The VXLAN driver so far has not increased the error counters for packets that set reserved bits. It does so for other packet errors, so do it for this case as well.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

 drivers/net/vxlan/vxlan_core.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/net/vxlan/vxlan_core.c +4 -0
index f6118de81b8a..b8afdcbdf235 100644
@@ -1721,6 +1721,10 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		 * little more security in adding extensions to VXLAN.
 		 */
 		reason = SKB_DROP_REASON_VXLAN_INVALID_HDR;
+		DEV_STATS_INC(vxlan->dev, rx_frame_errors);
+		DEV_STATS_INC(vxlan->dev, rx_errors);
+		vxlan_vnifilter_count(vxlan, vni, vninode,
+				      VXLAN_VNI_STATS_RX_ERRORS, 0);
 		goto drop;
 	}
Signature · 1 line
2.47.0
Reviewed-by on v2 6/11
#12 lore raw

Re: [PATCH net-next v2 06/11] vxlan: Bump error counters for header mismatches

Quoted 22 lines · Petr Machata

The VXLAN driver so far has not increased the error counters for packets that set reserved bits. It does so for other packet errors, so do it for this case as well.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

 drivers/net/vxlan/vxlan_core.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/net/vxlan/vxlan_core.c +0 -0
index f6118de81b8a..b8afdcbdf235 100644

drivers/net/vxlan/vxlan_core.c static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) · line 1730

Quoted 10 lines
drivers/net/vxlan/vxlan_core.c +4 -0
@@ -1721,6 +1721,10 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		 * little more security in adding extensions to VXLAN.
 		 */
 		reason = SKB_DROP_REASON_VXLAN_INVALID_HDR;
+		DEV_STATS_INC(vxlan->dev, rx_frame_errors);
+		DEV_STATS_INC(vxlan->dev, rx_errors);
+		vxlan_vnifilter_count(vxlan, vni, vninode,
+				      VXLAN_VNI_STATS_RX_ERRORS, 0);
 		goto drop;
 	}
Reviewed-by Nikolay Aleksandrov <[email protected]>
author
#13 lore raw

PATCH v2 7/11 vxlan: vxlan_rcv(): Drop unparsed

In reply to Petr Machata

The code currently validates the VXLAN header in two ways: first by comparing it with the set of reserved bits, constructed ahead of time during the netdevice construction; and second by gradually clearing the bits off a separate copy of VXLAN header, "unparsed". Drop the latter validation method.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

 drivers/net/vxlan/vxlan_core.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)
drivers/net/vxlan/vxlan_core.c +1 -15
index b8afdcbdf235..b79cc5da35c9 100644
@@ -1670,7 +1670,6 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 	const struct vxlanhdr *vh;
 	struct vxlan_dev *vxlan;
 	struct vxlan_sock *vs;
-	struct vxlanhdr unparsed;
 	struct vxlan_metadata _md;
 	struct vxlan_metadata *md = &_md;
 	__be16 protocol = htons(ETH_P_TEB);

@@ -1685,7 +1684,6 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 	if (reason)
 		goto drop;
 
-	unparsed = *vxlan_hdr(skb);
 	vh = vxlan_hdr(skb);
 	/* VNI flag always required to be set */
 	if (!(vh->vx_flags & VXLAN_HF_VNI)) {

@@ -1695,8 +1693,6 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		/* Return non vxlan pkt */
 		goto drop;
 	}
-	unparsed.vx_flags &= ~VXLAN_HF_VNI;
-	unparsed.vx_vni &= ~VXLAN_VNI_MASK;
 
 	vs = rcu_dereference_sk_user_data(sk);
 	if (!vs)

@@ -1731,7 +1727,6 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 	if (vxlan->cfg.flags & VXLAN_F_GPE) {
 		if (!vxlan_parse_gpe_proto(vh, &protocol))
 			goto drop;
-		unparsed.vx_flags &= ~VXLAN_GPE_USED_BITS;
 		raw_proto = true;
 	}
 

@@ -1745,8 +1740,6 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		reason = vxlan_remcsum(skb, vxlan->cfg.flags);
 		if (unlikely(reason))
 			goto drop;
-		unparsed.vx_flags &= ~VXLAN_HF_RCO;
-		unparsed.vx_vni &= VXLAN_VNI_MASK;
 	}
 
 	if (vxlan_collect_metadata(vs)) {

@@ -1769,19 +1762,12 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		memset(md, 0, sizeof(*md));
 	}
 
-	if (vxlan->cfg.flags & VXLAN_F_GBP) {
+	if (vxlan->cfg.flags & VXLAN_F_GBP)
 		vxlan_parse_gbp_hdr(skb, vxlan->cfg.flags, md);
-		unparsed.vx_flags &= ~VXLAN_GBP_USED_BITS;
-	}
 	/* Note that GBP and GPE can never be active together. This is
 	 * ensured in vxlan_dev_configure.
 	 */
 
-	if (unparsed.vx_flags || unparsed.vx_vni) {
-		reason = SKB_DROP_REASON_VXLAN_INVALID_HDR;
-		goto drop;
-	}
-
 	if (!raw_proto) {
 		reason = vxlan_set_mac(vxlan, vs, skb, vni);
 		if (reason)
Signature · 1 line
2.47.0
Reviewed-by on v2 7/11
#14 lore raw

Re: [PATCH net-next v2 07/11] vxlan: vxlan_rcv(): Drop unparsed

Quoted 8 lines · Petr Machata

The code currently validates the VXLAN header in two ways: first by comparing it with the set of reserved bits, constructed ahead of time during the netdevice construction; and second by gradually clearing the bits off a separate copy of VXLAN header, "unparsed". Drop the latter validation method.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Quoted 8 lines

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

Reviewed-by Nikolay Aleksandrov <[email protected]>
author
#15 lore raw

PATCH v2 8/11 vxlan: Add an attribute to make VXLAN header validation configurable

In reply to Petr Machata

The set of bits that the VXLAN netdevice currently considers reserved is defined by the features enabled at the netdevice construction. In order to make this configurable, add an attribute, IFLA_VXLAN_RESERVED_BITS. The payload is a pair of big-endian u32's covering the VXLAN header. This is validated against the set of flags used by the various enabled VXLAN features, and attempts to override bits used by an enabled feature are bounced.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

 drivers/net/vxlan/vxlan_core.c | 53 +++++++++++++++++++++++++++++-----
 include/uapi/linux/if_link.h   |  1 +
 2 files changed, 47 insertions(+), 7 deletions(-)
drivers/net/vxlan/vxlan_core.c +46 -7
index b79cc5da35c9..38e967e27683 100644
@@ -3438,6 +3438,7 @@ static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
 	[IFLA_VXLAN_VNIFILTER]	= { .type = NLA_U8 },
 	[IFLA_VXLAN_LOCALBYPASS]	= NLA_POLICY_MAX(NLA_U8, 1),
 	[IFLA_VXLAN_LABEL_POLICY]       = NLA_POLICY_MAX(NLA_U32, VXLAN_LABEL_MAX),
+	[IFLA_VXLAN_RESERVED_BITS] = NLA_POLICY_EXACT_LEN(sizeof(struct vxlanhdr)),
 };
 
 static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[],

@@ -4325,13 +4326,44 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 		used_bits.vx_flags |= VXLAN_GPE_USED_BITS;
 	}
 
-	/* For backwards compatibility, only allow reserved fields to be
-	 * used by VXLAN extensions if explicitly requested.
-	 */
-	conf->reserved_bits = (struct vxlanhdr) {
-		.vx_flags = ~used_bits.vx_flags,
-		.vx_vni = ~used_bits.vx_vni,
-	};
+	if (data[IFLA_VXLAN_RESERVED_BITS]) {
+		struct vxlanhdr reserved_bits;
+
+		if (changelink) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    data[IFLA_VXLAN_RESERVED_BITS],
+					    "Cannot change reserved_bits");
+			return -EOPNOTSUPP;
+		}
+
+		nla_memcpy(&reserved_bits, data[IFLA_VXLAN_RESERVED_BITS],
+			   sizeof(reserved_bits));
+		if (used_bits.vx_flags & reserved_bits.vx_flags ||
+		    used_bits.vx_vni & reserved_bits.vx_vni) {
+			__be64 ub_be64, rb_be64;
+
+			memcpy(&ub_be64, &used_bits, sizeof(ub_be64));
+			memcpy(&rb_be64, &reserved_bits, sizeof(rb_be64));
+
+			NL_SET_ERR_MSG_ATTR_FMT(extack,
+						data[IFLA_VXLAN_RESERVED_BITS],
+						"Used bits %#018llx cannot overlap reserved bits %#018llx",
+						be64_to_cpu(ub_be64),
+						be64_to_cpu(rb_be64));
+			return -EINVAL;
+		}
+
+		conf->reserved_bits = reserved_bits;
+	} else {
+		/* For backwards compatibility, only allow reserved fields to be
+		 * used by VXLAN extensions if explicitly requested.
+		 */
+		conf->reserved_bits = (struct vxlanhdr) {
+			.vx_flags = ~used_bits.vx_flags,
+			.vx_vni = ~used_bits.vx_vni,
+		};
+	}
+
 	if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) {
 		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_NOPARTIAL,
 				    VXLAN_F_REMCSUM_NOPARTIAL, changelink,

@@ -4516,6 +4548,8 @@ static size_t vxlan_get_size(const struct net_device *dev)
 		nla_total_size(0) + /* IFLA_VXLAN_GPE */
 		nla_total_size(0) + /* IFLA_VXLAN_REMCSUM_NOPARTIAL */
 		nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_VNIFILTER */
+		/* IFLA_VXLAN_RESERVED_BITS */
+		nla_total_size(sizeof(struct vxlanhdr)) +
 		0;
 }
 

@@ -4618,6 +4652,11 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
 		       !!(vxlan->cfg.flags & VXLAN_F_VNIFILTER)))
 		goto nla_put_failure;
 
+	if (nla_put(skb, IFLA_VXLAN_RESERVED_BITS,
+		    sizeof(vxlan->cfg.reserved_bits),
+		    &vxlan->cfg.reserved_bits))
+		goto nla_put_failure;
+
 	return 0;
 
 nla_put_failure:
include/uapi/linux/if_link.h +1 -0
index 2575e0cd9b48..77730c340c8f 100644
@@ -1394,6 +1394,7 @@ enum {
 	IFLA_VXLAN_VNIFILTER, /* only applicable with COLLECT_METADATA mode */
 	IFLA_VXLAN_LOCALBYPASS,
 	IFLA_VXLAN_LABEL_POLICY, /* IPv6 flow label policy; ifla_vxlan_label_policy */
+	IFLA_VXLAN_RESERVED_BITS,
 	__IFLA_VXLAN_MAX
 };
 #define IFLA_VXLAN_MAX	(__IFLA_VXLAN_MAX - 1)
Signature · 1 line
2.47.0
Reviewed-by on v2 8/11
#16 lore raw

Re: [PATCH net-next v2 08/11] vxlan: Add an attribute to make VXLAN header validation configurable

Quoted 10 lines · Petr Machata

The set of bits that the VXLAN netdevice currently considers reserved is defined by the features enabled at the netdevice construction. In order to make this configurable, add an attribute, IFLA_VXLAN_RESERVED_BITS. The payload is a pair of big-endian u32's covering the VXLAN header. This is validated against the set of flags used by the various enabled VXLAN features, and attempts to override bits used by an enabled feature are bounced.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Quoted 8 lines

---

Notes:
CC: Menglong Dong <[email protected]>
CC: Guillaume Nault <[email protected]>
CC: Alexander Lobakin <[email protected]>
CC: Breno Leitao <[email protected]>

Reviewed-by Nikolay Aleksandrov <[email protected]>
author
#17 lore raw

PATCH v2 9/11 selftests: net: lib: Rename ip_link_master() to ip_link_set_master()

In reply to Petr Machata

Let's have a verb in that function name to make it clearer what's going on.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
CC: Shuah Khan <[email protected]>
CC: Benjamin Poirier <[email protected]>
CC: Hangbin Liu <[email protected]>
CC: Vladimir Oltean <[email protected]>
CC: [email protected]

 tools/testing/selftests/net/fdb_notify.sh | 6 +++---
 tools/testing/selftests/net/lib.sh        | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
tools/testing/selftests/net/fdb_notify.sh +3 -3
index c03151e7791c..c159230c9b62 100755
@@ -49,7 +49,7 @@ test_dup_vxlan_self()
 {
 	ip_link_add br up type bridge vlan_filtering 1
 	ip_link_add vx up type vxlan id 2000 dstport 4789
-	ip_link_master vx br
+	ip_link_set_master vx br
 
 	do_test_dup add "vxlan" dev vx self dst 192.0.2.1
 	do_test_dup del "vxlan" dev vx self dst 192.0.2.1

@@ -59,7 +59,7 @@ test_dup_vxlan_master()
 {
 	ip_link_add br up type bridge vlan_filtering 1
 	ip_link_add vx up type vxlan id 2000 dstport 4789
-	ip_link_master vx br
+	ip_link_set_master vx br
 
 	do_test_dup add "vxlan master" dev vx master
 	do_test_dup del "vxlan master" dev vx master

@@ -79,7 +79,7 @@ test_dup_macvlan_master()
 	ip_link_add br up type bridge vlan_filtering 1
 	ip_link_add dd up type dummy
 	ip_link_add mv up link dd type macvlan mode passthru
-	ip_link_master mv br
+	ip_link_set_master mv br
 
 	do_test_dup add "macvlan master" dev mv self
 	do_test_dup del "macvlan master" dev mv self
tools/testing/selftests/net/lib.sh +1 -1
index 8994fec1c38f..5ea6537acd2b 100644
@@ -451,7 +451,7 @@ ip_link_add()
 	defer ip link del dev "$name"
 }
 
-ip_link_master()
+ip_link_set_master()
 {
 	local member=$1; shift
 	local master=$1; shift
Signature · 1 line
2.47.0
Reviewed-by on v2 9/11
#18 lore raw

Re: [PATCH net-next v2 09/11] selftests: net: lib: Rename ip_link_master() to ip_link_set_master()

Quoted 4 lines · Petr Machata

Let's have a verb in that function name to make it clearer what's going on.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Quoted 9 lines

---

Notes:
CC: Shuah Khan <[email protected]>
CC: Benjamin Poirier <[email protected]>
CC: Hangbin Liu <[email protected]>
CC: Vladimir Oltean <[email protected]>
CC: [email protected]

Reviewed-by Nikolay Aleksandrov <[email protected]>
author
#19 lore raw

PATCH v2 10/11 selftests: net: lib: Add several autodefer helpers

In reply to Petr Machata

Add ip_link_set_addr(), ip_link_set_up(), ip_addr_add() and ip_route_add() to the suite of helpers that automatically schedule a corresponding cleanup.

When setting a new MAC, one needs to remember the old address first. Move mac_get() from forwarding/ to that end.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
CC: Shuah Khan <[email protected]>
CC: Benjamin Poirier <[email protected]>
CC: Hangbin Liu <[email protected]>
CC: Vladimir Oltean <[email protected]>
CC: [email protected]

 tools/testing/selftests/net/forwarding/lib.sh |  7 ----
 tools/testing/selftests/net/lib.sh            | 39 +++++++++++++++++++
 2 files changed, 39 insertions(+), 7 deletions(-)
tools/testing/selftests/net/forwarding/lib.sh +0 -7
index 7337f398f9cc..1fd40bada694 100644
@@ -932,13 +932,6 @@ packets_rate()
 	echo $(((t1 - t0) / interval))
 }
 
-mac_get()
-{
-	local if_name=$1
-
-	ip -j link show dev $if_name | jq -r '.[]["address"]'
-}
-
 ether_addr_to_u64()
 {
 	local addr="$1"
tools/testing/selftests/net/lib.sh +39 -0
index 5ea6537acd2b..2cd5c743b2d9 100644
@@ -435,6 +435,13 @@ xfail_on_veth()
 	fi
 }
 
+mac_get()
+{
+	local if_name=$1
+
+	ip -j link show dev $if_name | jq -r '.[]["address"]'
+}
+
 kill_process()
 {
 	local pid=$1; shift

@@ -459,3 +466,35 @@ ip_link_set_master()
 	ip link set dev "$member" master "$master"
 	defer ip link set dev "$member" nomaster
 }
+
+ip_link_set_addr()
+{
+	local name=$1; shift
+	local addr=$1; shift
+
+	local old_addr=$(mac_get "$name")
+	ip link set dev "$name" address "$addr"
+	defer ip link set dev "$name" address "$old_addr"
+}
+
+ip_link_set_up()
+{
+	local name=$1; shift
+
+	ip link set dev "$name" up
+	defer ip link set dev "$name" down
+}
+
+ip_addr_add()
+{
+	local name=$1; shift
+
+	ip addr add dev "$name" "$@"
+	defer ip addr del dev "$name" "$@"
+}
+
+ip_route_add()
+{
+	ip route add "$@"
+	defer ip route del "$@"
+}
Signature · 1 line
2.47.0
Reviewed-by on v2 10/11
#20 lore raw

Re: [PATCH net-next v2 10/11] selftests: net: lib: Add several autodefer helpers

Quoted 9 lines · Petr Machata

Add ip_link_set_addr(), ip_link_set_up(), ip_addr_add() and ip_route_add() to the suite of helpers that automatically schedule a corresponding cleanup.

When setting a new MAC, one needs to remember the old address first. Move mac_get() from forwarding/ to that end.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Quoted 9 lines

---

Notes:
CC: Shuah Khan <[email protected]>
CC: Benjamin Poirier <[email protected]>
CC: Hangbin Liu <[email protected]>
CC: Vladimir Oltean <[email protected]>
CC: [email protected]

Reviewed-by Nikolay Aleksandrov <[email protected]>
author
#21 lore raw

PATCH v2 11/11 selftests: forwarding: Add a selftest for the new reserved_bits UAPI

In reply to Petr Machata

Run VXLAN packets through a gateway. Flip individual bits of the packet and/or reserved bits of the gateway, and check that the gateway treats the packets as expected.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
v2:
- Add the new test to Makefile

CC Shuah Khan <[email protected]>
CC Benjamin Poirier <[email protected]>
CC Hangbin Liu <[email protected]>
CC Vladimir Oltean <[email protected]>
CC [email protected]
 .../testing/selftests/net/forwarding/Makefile |   1 +
 .../net/forwarding/vxlan_reserved.sh          | 352 ++++++++++++++++++
 2 files changed, 353 insertions(+)
 create mode 100755 tools/testing/selftests/net/forwarding/vxlan_reserved.sh
tools/testing/selftests/net/forwarding/Makefile +1 -0
index 7d885cff8d79..00bde7b6f39e 100644
@@ -105,6 +105,7 @@ TEST_PROGS = bridge_fdb_learning_limit.sh \
 	vxlan_bridge_1q_port_8472_ipv6.sh \
 	vxlan_bridge_1q_port_8472.sh \
 	vxlan_bridge_1q.sh \
+	vxlan_reserved.sh \
 	vxlan_symmetric_ipv6.sh \
 	vxlan_symmetric.sh
tools/testing/selftests/net/forwarding/vxlan_reserved.sh new file +352 -0

352 lines not on this page · raw message

Signature · 1 line
2.47.0
Reviewed-by on v2 11/11
#22 lore raw

Re: [PATCH net-next v2 11/11] selftests: forwarding: Add a selftest for the new reserved_bits UAPI

Quoted 12 lines · Petr Machata

Run VXLAN packets through a gateway. Flip individual bits of the packet and/or reserved bits of the gateway, and check that the gateway treats the packets as expected.

Signed-off-by Petr Machata <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>

---

Notes:
v2:
- Add the new test to Makefile

Quoted 6 lines
CC Shuah Khan <[email protected]>
CC Benjamin Poirier <[email protected]>
CC Hangbin Liu <[email protected]>
CC Vladimir Oltean <[email protected]>
CC [email protected]
Reviewed-by Nikolay Aleksandrov <[email protected]>
CI Applied
#23 lore raw
In reply to Petr Machata
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 10 lines · On Thu, 5 Dec 2024 16:40:49 +0100 you wrote:

Currently the VXLAN header validation works by vxlan_rcv() going feature by feature, each feature clearing the bits that it consumes. If anything is left unparsed at the end, the packet is rejected.

Unfortunately there are machines out there that send VXLAN packets with reserved bits set, even if they are configured to not use the corresponding features. One such report is here[1], and we have heard similar complaints from our customers as well.

[...]

Here is the summary with links:
- [net-next,v2,01/11] vxlan: In vxlan_rcv(), access flags through the vxlan netdevice

    https://git.kernel.org/netdev/net-next/c/9234a37a495d
  - [net-next,v2,02/11] vxlan: vxlan_rcv() callees: Move clearing of unparsed flags out
    https://git.kernel.org/netdev/net-next/c/0f09ae907818
  - [net-next,v2,03/11] vxlan: vxlan_rcv() callees: Drop the unparsed argument
    https://git.kernel.org/netdev/net-next/c/fe3dcbcfae52
  - [net-next,v2,04/11] vxlan: vxlan_rcv(): Extract vxlan_hdr(skb) to a named variable
    https://git.kernel.org/netdev/net-next/c/e713130dfb4d
  - [net-next,v2,05/11] vxlan: Track reserved bits explicitly as part of the configuration
    https://git.kernel.org/netdev/net-next/c/e4f8647767cf
  - [net-next,v2,06/11] vxlan: Bump error counters for header mismatches
    https://git.kernel.org/netdev/net-next/c/752b1c8d8b40
  - [net-next,v2,07/11] vxlan: vxlan_rcv(): Drop unparsed
    https://git.kernel.org/netdev/net-next/c/bb16786ed6fd
  - [net-next,v2,08/11] vxlan: Add an attribute to make VXLAN header validation configurable
    https://git.kernel.org/netdev/net-next/c/6c11379b104e
  - [net-next,v2,09/11] selftests: net: lib: Rename ip_link_master() to ip_link_set_master()
    https://git.kernel.org/netdev/net-next/c/8653eb21d68c
  - [net-next,v2,10/11] selftests: net: lib: Add several autodefer helpers
    https://git.kernel.org/netdev/net-next/c/d76ccb2ec368
  - [net-next,v2,11/11] selftests: forwarding: Add a selftest for the new reserved_bits UAPI
    https://git.kernel.org/netdev/net-next/c/d84b5dccf3eb

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html