[PATCH nf-next 0/2] nf_tables: vlan matching & mangling

[PATCH nf-next 0/2] nf_tables: vlan matching & mangling

[Pablo Neira Ayuso]

Hi,

This patchset revisits vlan matching & mangling support for nf_tables:

Patch #1 restores q-in-q matching by reverting
         f6ae9f120dad ("netfilter: nft_payload: add C-VLAN support").
         Support for matching on inner vlan headers when vlan offload
	 was already available before such commit.

Patch #2 adds a parser to deal with setting the skbuff vlan offload
         fields based on the payload offset and length. Userspace is
	 agnostic of the kernel vlan offload capabilities, hence,
	 kernel checks if offset and length refers to the skbuff
	 vlan_proto and vlan_tci fields. This also supports mangling
	 q-in-q too.

Note #2 only supports for vlan tag mangling: For pop/push tags a new
actions is required, I already made code for pushing tags which never
got integrated that I can polish and prepare for submission.

I am currently extending tests/shell/testcases/packetpath/vlan_8021ad_tag
to improve coverage for these two cases. I have already have a few
scripts to test this patches with containers but I need to integrate
them into the aforementioned tests/shell script, I will keep you posted.

Pablo Neira Ayuso (2):
  netfilter: nft_payload: restore vlan q-in-q match support
  netfilter: nft_payload: skbuff vlan metadata mangle support

 net/netfilter/nft_payload.c | 95 ++++++++++++++++++++++++++++---------
 1 file changed, 72 insertions(+), 23 deletions(-)

-- 
2.30.2

[PATCH nf-next 1/2] netfilter: nft_payload: restore vlan q-in-q match support

[Pablo Neira Ayuso]

Revert f6ae9f120dad ("netfilter: nft_payload: add C-VLAN support").

f41f72d09ee1 ("netfilter: nft_payload: simplify vlan header handling")
already allows to match on inner vlan tags by subtract the vlan header
size to the payload offset which has been popped and stored in skbuff
metadata fields.

Fixes: f6ae9f120dad ("netfilter: nft_payload: add C-VLAN support")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_payload.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
index 0a689c8e0295..a3cb5dbcb362 100644
--- a/net/netfilter/nft_payload.c
+++ b/net/netfilter/nft_payload.c
@@ -45,36 +45,27 @@ nft_payload_copy_vlan(u32 *d, const struct sk_buff *skb, u8 offset, u8 len)
 	int mac_off = skb_mac_header(skb) - skb->data;
 	u8 *vlanh, *dst_u8 = (u8 *) d;
 	struct vlan_ethhdr veth;
-	u8 vlan_hlen = 0;
-
-	if ((skb->protocol == htons(ETH_P_8021AD) ||
-	     skb->protocol == htons(ETH_P_8021Q)) &&
-	    offset >= VLAN_ETH_HLEN && offset < VLAN_ETH_HLEN + VLAN_HLEN)
-		vlan_hlen += VLAN_HLEN;
 
 	vlanh = (u8 *) &veth;
-	if (offset < VLAN_ETH_HLEN + vlan_hlen) {
+	if (offset < VLAN_ETH_HLEN) {
 		u8 ethlen = len;
 
-		if (vlan_hlen &&
-		    skb_copy_bits(skb, mac_off, &veth, VLAN_ETH_HLEN) < 0)
-			return false;
-		else if (!nft_payload_rebuild_vlan_hdr(skb, mac_off, &veth))
+		if (!nft_payload_rebuild_vlan_hdr(skb, mac_off, &veth))
 			return false;
 
-		if (offset + len > VLAN_ETH_HLEN + vlan_hlen)
-			ethlen -= offset + len - VLAN_ETH_HLEN - vlan_hlen;
+		if (offset + len > VLAN_ETH_HLEN)
+			ethlen -= offset + len - VLAN_ETH_HLEN;
 
-		memcpy(dst_u8, vlanh + offset - vlan_hlen, ethlen);
+		memcpy(dst_u8, vlanh + offset, ethlen);
 
 		len -= ethlen;
 		if (len == 0)
 			return true;
 
 		dst_u8 += ethlen;
-		offset = ETH_HLEN + vlan_hlen;
+		offset = ETH_HLEN;
 	} else {
-		offset -= VLAN_HLEN + vlan_hlen;
+		offset -= VLAN_HLEN;
 	}
 
 	return skb_copy_bits(skb, offset + mac_off, dst_u8, len) == 0;
-- 
2.30.2

[PATCH nf-next 2/2] netfilter: nft_payload: skbuff vlan metadata mangle support

[Pablo Neira Ayuso]

Userspace assumes vlan header is present at a given offset, but vlan
offload allows to store this in metadata fields of the skbuff. Handle
this transparently by adding a parser to the kernel.

If vlan metadata is present and payload offset is over 12 bytes (source
and destination mac address fields), then subtract vlan header present
in vlan metadata, otherwise mangle vlan metadata based on offset and
length, extracting data from the source register.

This is similar to:

  8cfd23e67401 ("netfilter: nft_payload: work around vlan header stripping")

to deal with vlan payload mangling.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_payload.c | 72 +++++++++++++++++++++++++++++++++----
 1 file changed, 65 insertions(+), 7 deletions(-)

diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
index a3cb5dbcb362..e1af7b5e70c6 100644
--- a/net/netfilter/nft_payload.c
+++ b/net/netfilter/nft_payload.c
@@ -145,12 +145,12 @@ int nft_payload_inner_offset(const struct nft_pktinfo *pkt)
 	return pkt->inneroff;
 }
 
-static bool nft_payload_need_vlan_copy(const struct nft_payload *priv)
+static bool nft_payload_need_vlan_adjust(u32 offset, u32 len)
 {
-	unsigned int len = priv->offset + priv->len;
+	unsigned int boundary = offset + len;
 
 	/* data past ether src/dst requested, copy needed */
-	if (len > offsetof(struct ethhdr, h_proto))
+	if (boundary > offsetof(struct ethhdr, h_proto))
 		return true;
 
 	return false;
@@ -174,7 +174,7 @@ void nft_payload_eval(const struct nft_expr *expr,
 			goto err;
 
 		if (skb_vlan_tag_present(skb) &&
-		    nft_payload_need_vlan_copy(priv)) {
+		    nft_payload_need_vlan_adjust(priv->offset, priv->len)) {
 			if (!nft_payload_copy_vlan(dest, skb,
 						   priv->offset, priv->len))
 				goto err;
@@ -801,21 +801,79 @@ struct nft_payload_set {
 	u8			csum_flags;
 };
 
+/* This is not struct vlan_hdr. */
+struct nft_payload_vlan_hdr {
+        __be16          h_vlan_proto;
+        __be16          h_vlan_TCI;
+};
+
+static bool
+nft_payload_set_vlan(const u32 *src, struct sk_buff *skb, u8 offset, u8 len,
+		     int *vlan_hlen)
+{
+	struct nft_payload_vlan_hdr *vlanh;
+	__be16 vlan_proto;
+	__be16 vlan_tci;
+
+	if (offset >= offsetof(struct vlan_ethhdr, h_vlan_encapsulated_proto)) {
+		*vlan_hlen = VLAN_HLEN;
+		return true;
+	}
+
+	switch (offset) {
+	case offsetof(struct vlan_ethhdr, h_vlan_proto):
+		if (len == 2) {
+			vlan_proto = nft_reg_load16(src);
+			skb->vlan_proto = vlan_proto;
+		} else if (len == 4) {
+			vlanh = (struct nft_payload_vlan_hdr *)src;
+			__vlan_hwaccel_put_tag(skb, vlanh->h_vlan_proto,
+					       ntohs(vlanh->h_vlan_TCI));
+		} else {
+			return false;
+		}
+		break;
+	case offsetof(struct vlan_ethhdr, h_vlan_TCI):
+		if (len != 2)
+			return false;
+
+		vlan_tci = ntohs(nft_reg_load16(src));
+		skb->vlan_tci = vlan_tci;
+		break;
+	default:
+		return false;
+	}
+
+	return true;
+}
+
 static void nft_payload_set_eval(const struct nft_expr *expr,
 				 struct nft_regs *regs,
 				 const struct nft_pktinfo *pkt)
 {
 	const struct nft_payload_set *priv = nft_expr_priv(expr);
-	struct sk_buff *skb = pkt->skb;
 	const u32 *src = &regs->data[priv->sreg];
-	int offset, csum_offset;
+	int offset, csum_offset, vlan_hlen = 0;
+	struct sk_buff *skb = pkt->skb;
 	__wsum fsum, tsum;
 
 	switch (priv->base) {
 	case NFT_PAYLOAD_LL_HEADER:
 		if (!skb_mac_header_was_set(skb))
 			goto err;
-		offset = skb_mac_header(skb) - skb->data;
+
+		if (skb_vlan_tag_present(skb) &&
+		    nft_payload_need_vlan_adjust(priv->offset, priv->len)) {
+			if (!nft_payload_set_vlan(src, skb,
+						  priv->offset, priv->len,
+						  &vlan_hlen))
+				goto err;
+
+			if (!vlan_hlen)
+				return;
+		}
+
+		offset = skb_mac_header(skb) - skb->data - vlan_hlen;
 		break;
 	case NFT_PAYLOAD_NETWORK_HEADER:
 		offset = skb_network_offset(skb);
-- 
2.30.2

Re: [PATCH nf-next 2/2] netfilter: nft_payload: skbuff vlan metadata mangle support

[kernel test robot]

Hi Pablo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on netfilter-nf/main]
[also build test WARNING on linus/master v6.9-rc7 next-20240510]
[cannot apply to nf-next/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Pablo-Neira-Ayuso/netfilter-nft_payload-restore-vlan-q-in-q-match-support/20240510-080839
base:   git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git main
patch link:    https://lore.kernel.org/r/20240510000719.3205-3-pablo%40netfilter.org
patch subject: [PATCH nf-next 2/2] netfilter: nft_payload: skbuff vlan metadata mangle support
config: powerpc64-randconfig-r133-20240510 (https://download.01.org/0day-ci/archive/20240510/202405102106.cxYkCzFw-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240510/202405102106.cxYkCzFw-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405102106.cxYkCzFw-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> net/netfilter/nft_payload.c:826:36: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be16 [usertype] vlan_proto @@     got unsigned short @@
   net/netfilter/nft_payload.c:826:36: sparse:     expected restricted __be16 [usertype] vlan_proto
   net/netfilter/nft_payload.c:826:36: sparse:     got unsigned short
>> net/netfilter/nft_payload.c:840:28: sparse: sparse: cast to restricted __be16
>> net/netfilter/nft_payload.c:840:26: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be16 [usertype] vlan_tci @@     got unsigned short [usertype] @@
   net/netfilter/nft_payload.c:840:26: sparse:     expected restricted __be16 [usertype] vlan_tci
   net/netfilter/nft_payload.c:840:26: sparse:     got unsigned short [usertype]
>> net/netfilter/nft_payload.c:841:31: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short [usertype] vlan_tci @@     got restricted __be16 [usertype] vlan_tci @@
   net/netfilter/nft_payload.c:841:31: sparse:     expected unsigned short [usertype] vlan_tci
   net/netfilter/nft_payload.c:841:31: sparse:     got restricted __be16 [usertype] vlan_tci

vim +826 net/netfilter/nft_payload.c

   809	
   810	static bool
   811	nft_payload_set_vlan(const u32 *src, struct sk_buff *skb, u8 offset, u8 len,
   812			     int *vlan_hlen)
   813	{
   814		struct nft_payload_vlan_hdr *vlanh;
   815		__be16 vlan_proto;
   816		__be16 vlan_tci;
   817	
   818		if (offset >= offsetof(struct vlan_ethhdr, h_vlan_encapsulated_proto)) {
   819			*vlan_hlen = VLAN_HLEN;
   820			return true;
   821		}
   822	
   823		switch (offset) {
   824		case offsetof(struct vlan_ethhdr, h_vlan_proto):
   825			if (len == 2) {
 > 826				vlan_proto = nft_reg_load16(src);
   827				skb->vlan_proto = vlan_proto;
   828			} else if (len == 4) {
   829				vlanh = (struct nft_payload_vlan_hdr *)src;
   830				__vlan_hwaccel_put_tag(skb, vlanh->h_vlan_proto,
   831						       ntohs(vlanh->h_vlan_TCI));
   832			} else {
   833				return false;
   834			}
   835			break;
   836		case offsetof(struct vlan_ethhdr, h_vlan_TCI):
   837			if (len != 2)
   838				return false;
   839	
 > 840			vlan_tci = ntohs(nft_reg_load16(src));
 > 841			skb->vlan_tci = vlan_tci;
   842			break;
   843		default:
   844			return false;
   845		}
   846	
   847		return true;
   848	}
   849	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki