[PATCH 00/12] flow_dissector: Dissect UDP encapsulation protocols

[PATCH 00/12] flow_dissector: Dissect UDP encapsulation protocols

[Tom Herbert]

Add support in flow_dissector for dissecting into UDP
encapsulations like VXLAN. __skb_flow_dissect_udp is called for
IPPROTO_UDP. The flag FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS enables parsing
of UDP encapsulations. If the flag is set when parsing a UDP packet then
a socket lookup is performed. The offset of the base network header,
either an IPv4 or IPv6 header, is tracked and passed to
__skb_flow_dissect_udp so that it can perform the socket lookup.
If a socket is found and it's for a UDP encapsulation (encap_type is
set in the UDP socket) then a switch is performed on the encap_type
value (cases are UDP_ENCAP_* values)

Changes in the patch set:

- Unconstantify struct net argument in flowdis functions so we can call
  UDP socket lookup functions
- Dissect ETH_P_TEB in main flow dissector loop, move ETH_P_TEB check
  out of __skb_flow_dissect_gre and process it in main loop
- Add UDP_ENCAP constants for tipc, fou, gue, sctp, rxe, pfcp,
  wireguard, bareudp, vxlan, vxlan_gpe, geneve, and amt
- For the various UDP encapsulation protocols, Instead of just setting
  UDP tunnel encap type to 1, set it to the corresponding UDP_ENCAP
  constant. This allows identify the encapsulation protocol for a
  UDP socket by the encap_type
- Add function __skb_flow_dissect_udp in flow_dissector and call it for
  UDP packets. If a UDP encapsulation is present then the function
  returns either FLOW_DISSECT_RET_PROTO_AGAIN or
  FLOW_DISSECT_RET_IPPROTO_AGAIN
- Add flag FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS that indicates UDP
  encapsulations should be dissected
- Add __skb_flow_dissect_vxlan which is called when encap_type is
  UDP_ENCAP_VXLAN or UDP_ENCAP_VXLAN_GPE. Dissect VXLAN and return
  a next protocol and offset
- Add __skb_flow_dissect_fou which is called when encap_type is
  UDP_ENCAP_FOU. Dissect FOU and return a next protocol and offset
- Add support for ESP, L2TP, and SCTP in UDP in __skb_flow_dissect_udp.
  All we need to do is return FLOW_DISSECT_RET_IPPROTO_AGAIN and the
  corresponding IP protocol number
- Add __skb_flow_dissect_geneve which is called when encap_type is
  UDP_ENCAP_GENEVE. Dissect geneve and return a next protocol and offset
- Add __skb_flow_dissect_gue which is called when encap_type is
  UDP_ENCAP_GUE. Dissect gue and return a next protocol and offset
- Add __skb_flow_dissect_gtp which is called when encap_type is
  UDP_ENCAP_GTP. Dissect gtp and return a next protocol and offset

Tested: Verified fou, gue, vxlan, and geneve are properly dissected for
IPv4 and IPv6 cases. This includes testing ETH_P_TEB case

Tom Herbert (12):
  skbuff: Unconstantify struct net argument in flowdis functions
  flow_dissector: Parse ETH_P_TEB
  flow_dissector: Move ETH_P_TEB out of GRE
  udp_encaps: Add new UDP_ENCAP constants
  udp_encaps: Set proper UDP_ENCAP types in tunnel setup
  flow_dissector: UDP encap infrastructure
  flow_dissector: Parse vxlan in UDP
  flow_dissector: Parse foo-over-udp (FOU)
  flow_dissector: Parse ESP, L2TP, and SCTP in UDP
  flow_dissector: Parse Geneve in UDP
  flow_dissector: Parse GUE in UDP
  flow_dissector: Parse gtp in UDP

 drivers/infiniband/sw/rxe/rxe_net.c |   2 +-
 drivers/net/amt.c                   |   2 +-
 drivers/net/bareudp.c               |   2 +-
 drivers/net/geneve.c                |   2 +-
 drivers/net/pfcp.c                  |   2 +-
 drivers/net/vxlan/vxlan_core.c      |   3 +-
 drivers/net/wireguard/socket.c      |   2 +-
 include/linux/skbuff.h              |  10 +-
 include/net/flow_dissector.h        |   1 +
 include/net/fou.h                   |  16 +
 include/uapi/linux/udp.h            |  13 +
 net/core/flow_dissector.c           | 434 ++++++++++++++++++++++++++--
 net/ipv4/fou_core.c                 |  19 +-
 net/sctp/protocol.c                 |   2 +-
 net/tipc/udp_media.c                |   2 +-
 15 files changed, 452 insertions(+), 60 deletions(-)

-- 
2.34.1

[PATCH 01/12] skbuff: Unconstantify struct net argument in flowdis functions

[Tom Herbert]

We want __skb_flow_dissect to be able to call functions that
take a non-constant struct net argument (UDP socket lookup
functions for instance). Change the net argument of flow dissector
functions to not be const

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/linux/skbuff.h    | 10 +++++-----
 net/core/flow_dissector.c |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 29c3ea5b6e93..a0b5687fa49c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1504,14 +1504,14 @@ __skb_set_sw_hash(struct sk_buff *skb, __u32 hash, bool is_l4)
 	__skb_set_hash(skb, hash, true, is_l4);
 }
 
-u32 __skb_get_hash_symmetric_net(const struct net *net, const struct sk_buff *skb);
+u32 __skb_get_hash_symmetric_net(struct net *net, const struct sk_buff *skb);
 
 static inline u32 __skb_get_hash_symmetric(const struct sk_buff *skb)
 {
 	return __skb_get_hash_symmetric_net(NULL, skb);
 }
 
-void __skb_get_hash_net(const struct net *net, struct sk_buff *skb);
+void __skb_get_hash_net(struct net *net, struct sk_buff *skb);
 u32 skb_get_poff(const struct sk_buff *skb);
 u32 __skb_get_poff(const struct sk_buff *skb, const void *data,
 		   const struct flow_keys_basic *keys, int hlen);
@@ -1532,7 +1532,7 @@ struct bpf_flow_dissector;
 u32 bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
 		     __be16 proto, int nhoff, int hlen, unsigned int flags);
 
-bool __skb_flow_dissect(const struct net *net,
+bool __skb_flow_dissect(struct net *net,
 			const struct sk_buff *skb,
 			struct flow_dissector *flow_dissector,
 			void *target_container, const void *data,
@@ -1556,7 +1556,7 @@ static inline bool skb_flow_dissect_flow_keys(const struct sk_buff *skb,
 }
 
 static inline bool
-skb_flow_dissect_flow_keys_basic(const struct net *net,
+skb_flow_dissect_flow_keys_basic(struct net *net,
 				 const struct sk_buff *skb,
 				 struct flow_keys_basic *flow,
 				 const void *data, __be16 proto,
@@ -1590,7 +1590,7 @@ void skb_flow_dissect_hash(const struct sk_buff *skb,
 			   struct flow_dissector *flow_dissector,
 			   void *target_container);
 
-static inline __u32 skb_get_hash_net(const struct net *net, struct sk_buff *skb)
+static inline __u32 skb_get_hash_net(struct net *net, struct sk_buff *skb)
 {
 	if (!skb->l4_hash && !skb->sw_hash)
 		__skb_get_hash_net(net, skb);
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 0e638a37aa09..e034e502ab49 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -1045,7 +1045,7 @@ static bool is_pppoe_ses_hdr_valid(const struct pppoe_hdr *hdr)
  *
  * Caller must take care of zeroing target container memory.
  */
-bool __skb_flow_dissect(const struct net *net,
+bool __skb_flow_dissect(struct net *net,
 			const struct sk_buff *skb,
 			struct flow_dissector *flow_dissector,
 			void *target_container, const void *data,
@@ -1854,7 +1854,7 @@ EXPORT_SYMBOL(make_flow_keys_digest);
 
 static struct flow_dissector flow_keys_dissector_symmetric __read_mostly;
 
-u32 __skb_get_hash_symmetric_net(const struct net *net, const struct sk_buff *skb)
+u32 __skb_get_hash_symmetric_net(struct net *net, const struct sk_buff *skb)
 {
 	struct flow_keys keys;
 
@@ -1878,7 +1878,7 @@ EXPORT_SYMBOL_GPL(__skb_get_hash_symmetric_net);
  * on success, zero indicates no valid hash.  Also, sets l4_hash in skb
  * if hash is a canonical 4-tuple hash over transport ports.
  */
-void __skb_get_hash_net(const struct net *net, struct sk_buff *skb)
+void __skb_get_hash_net(struct net *net, struct sk_buff *skb)
 {
 	struct flow_keys keys;
 	u32 hash;
-- 
2.34.1

Re: [PATCH 01/12] skbuff: Unconstantify struct net argument in flowdis functions

[Willem de Bruijn]

Tom Herbert wrote:
> We want __skb_flow_dissect to be able to call functions that
> take a non-constant struct net argument (UDP socket lookup
> functions for instance). Change the net argument of flow dissector
> functions to not be const
> 
> Signed-off-by: Tom Herbert <tom@herbertland.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [PATCH 01/12] skbuff: Unconstantify struct net argument in flowdis functions

[Eric Dumazet]

On Wed, Jul 31, 2024 at 7:23 PM Tom Herbert <tom@herbertland.com> wrote:
>
> We want __skb_flow_dissect to be able to call functions that
> take a non-constant struct net argument (UDP socket lookup
> functions for instance). Change the net argument of flow dissector
> functions to not be const
>
> Signed-off-by: Tom Herbert <tom@herbertland.com>


Hmm... let me send a patch series doing the opposite, ie add const
qualifiers to lookup functions.

Re: [PATCH 01/12] skbuff: Unconstantify struct net argument in flowdis functions

[Tom Herbert]

On Fri, Aug 2, 2024 at 6:34 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Jul 31, 2024 at 7:23 PM Tom Herbert <tom@herbertland.com> wrote:
> >
> > We want __skb_flow_dissect to be able to call functions that
> > take a non-constant struct net argument (UDP socket lookup
> > functions for instance). Change the net argument of flow dissector
> > functions to not be const
> >
> > Signed-off-by: Tom Herbert <tom@herbertland.com>
>
>
> Hmm... let me send a patch series doing the opposite, ie add const
> qualifiers to lookup functions.

I had done that originally, but there were a lot of callers so it was
pretty messy.

Tom

Re: [PATCH 01/12] skbuff: Unconstantify struct net argument in flowdis functions

[Tom Herbert]

On Thu, Aug 15, 2024 at 11:50 AM Tom Herbert <tom@herbertland.com> wrote:
>
> On Fri, Aug 2, 2024 at 6:34 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Wed, Jul 31, 2024 at 7:23 PM Tom Herbert <tom@herbertland.com> wrote:
> > >
> > > We want __skb_flow_dissect to be able to call functions that
> > > take a non-constant struct net argument (UDP socket lookup
> > > functions for instance). Change the net argument of flow dissector
> > > functions to not be const
> > >
> > > Signed-off-by: Tom Herbert <tom@herbertland.com>
> >
> >
> > Hmm... let me send a patch series doing the opposite, ie add const
> > qualifiers to lookup functions.
>
> I had done that originally, but there were a lot of callers so it was
> pretty messy.

Oops, I see this was already applied. Thanks!

Tom

>
> Tom

[PATCH 02/12] flow_dissector: Parse ETH_P_TEB

[Tom Herbert]

ETH_P_TEB (Trans Ether Bridging) is the EtherType to carry
a plain Etherent frame. Add case in skb_flow_dissect to parse
packets of this type

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/core/flow_dissector.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index e034e502ab49..d9abb4ae021b 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -1284,6 +1284,27 @@ bool __skb_flow_dissect(struct net *net,
 
 		break;
 	}
+	case htons(ETH_P_TEB): {
+		const struct ethhdr *eth;
+		struct ethhdr _eth;
+
+		eth = __skb_header_pointer(skb, nhoff, sizeof(_eth),
+					   data, hlen, &_eth);
+		if (!eth)
+			goto out_bad;
+
+		proto = eth->h_proto;
+		nhoff += sizeof(*eth);
+
+		/* Cap headers that we access via pointers at the
+		 * end of the Ethernet header as our maximum alignment
+		 * at that point is only 2 bytes.
+		 */
+		if (NET_IP_ALIGN)
+			hlen = nhoff;
+
+		goto proto_again;
+	}
 	case htons(ETH_P_8021AD):
 	case htons(ETH_P_8021Q): {
 		const struct vlan_hdr *vlan = NULL;
-- 
2.34.1

Re: [PATCH 02/12] flow_dissector: Parse ETH_P_TEB

[Willem de Bruijn]

Tom Herbert wrote:
> ETH_P_TEB (Trans Ether Bridging) is the EtherType to carry
> a plain Etherent frame. Add case in skb_flow_dissect to parse
> packets of this type
> 
> Signed-off-by: Tom Herbert <tom@herbertland.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

[PATCH 03/12] flow_dissector: Move ETH_P_TEB out of GRE

[Tom Herbert]

If the GRE protocol is ETH_P_TEB then just process that as any
another EtherType since it's now supported in the main loop

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/core/flow_dissector.c | 36 ++++++++++--------------------------
 1 file changed, 10 insertions(+), 26 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index d9abb4ae021b..416f889c623c 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -654,7 +654,7 @@ __skb_flow_dissect_gre(const struct sk_buff *skb,
 		       struct flow_dissector_key_control *key_control,
 		       struct flow_dissector *flow_dissector,
 		       void *target_container, const void *data,
-		       __be16 *p_proto, int *p_nhoff, int *p_hlen,
+		       __be16 *p_proto, int *p_nhoff, int hlen,
 		       unsigned int flags)
 {
 	struct flow_dissector_key_keyid *key_keyid;
@@ -663,7 +663,7 @@ __skb_flow_dissect_gre(const struct sk_buff *skb,
 	u16 gre_ver;
 
 	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr),
-				   data, *p_hlen, &_hdr);
+				   data, hlen, &_hdr);
 	if (!hdr)
 		return FLOW_DISSECT_RET_OUT_BAD;
 
@@ -695,7 +695,7 @@ __skb_flow_dissect_gre(const struct sk_buff *skb,
 
 		keyid = __skb_header_pointer(skb, *p_nhoff + offset,
 					     sizeof(_keyid),
-					     data, *p_hlen, &_keyid);
+					     data, hlen, &_keyid);
 		if (!keyid)
 			return FLOW_DISSECT_RET_OUT_BAD;
 
@@ -715,27 +715,11 @@ __skb_flow_dissect_gre(const struct sk_buff *skb,
 	if (hdr->flags & GRE_SEQ)
 		offset += sizeof_field(struct pptp_gre_header, seq);
 
-	if (gre_ver == 0) {
-		if (*p_proto == htons(ETH_P_TEB)) {
-			const struct ethhdr *eth;
-			struct ethhdr _eth;
-
-			eth = __skb_header_pointer(skb, *p_nhoff + offset,
-						   sizeof(_eth),
-						   data, *p_hlen, &_eth);
-			if (!eth)
-				return FLOW_DISSECT_RET_OUT_BAD;
-			*p_proto = eth->h_proto;
-			offset += sizeof(*eth);
-
-			/* Cap headers that we access via pointers at the
-			 * end of the Ethernet header as our maximum alignment
-			 * at that point is only 2 bytes.
-			 */
-			if (NET_IP_ALIGN)
-				*p_hlen = *p_nhoff + offset;
-		}
-	} else { /* version 1, must be PPTP */
+	/* For GRE version 0 p_proto is already correctly set (including if
+	 * it is ETH_P_TEB)
+	 */
+
+	if (gre_ver == 1) { /* Version 1 is PPP */
 		u8 _ppp_hdr[PPP_HDRLEN];
 		u8 *ppp_hdr;
 
@@ -744,7 +728,7 @@ __skb_flow_dissect_gre(const struct sk_buff *skb,
 
 		ppp_hdr = __skb_header_pointer(skb, *p_nhoff + offset,
 					       sizeof(_ppp_hdr),
-					       data, *p_hlen, _ppp_hdr);
+					       data, hlen, _ppp_hdr);
 		if (!ppp_hdr)
 			return FLOW_DISSECT_RET_OUT_BAD;
 
@@ -1552,7 +1536,7 @@ bool __skb_flow_dissect(struct net *net,
 
 		fdret = __skb_flow_dissect_gre(skb, key_control, flow_dissector,
 					       target_container, data,
-					       &proto, &nhoff, &hlen, flags);
+					       &proto, &nhoff, hlen, flags);
 		break;
 
 	case NEXTHDR_HOP:
-- 
2.34.1

Re: [PATCH 03/12] flow_dissector: Move ETH_P_TEB out of GRE

[Willem de Bruijn]

Tom Herbert wrote:
> If the GRE protocol is ETH_P_TEB then just process that as any
> another EtherType since it's now supported in the main loop
> 
> Signed-off-by: Tom Herbert <tom@herbertland.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Might make sense to squash with previous. To make it clear that the
previous is not new code, but a (cleanup and) move.

[PATCH 04/12] udp_encaps: Add new UDP_ENCAP constants

[Tom Herbert]

Add constants for various UDP encapsulations that are supported

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/uapi/linux/udp.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/uapi/linux/udp.h b/include/uapi/linux/udp.h
index 1a0fe8b151fb..0432a9a6536d 100644
--- a/include/uapi/linux/udp.h
+++ b/include/uapi/linux/udp.h
@@ -36,6 +36,7 @@ struct udphdr {
 #define UDP_GRO		104	/* This socket can receive UDP GRO packets */
 
 /* UDP encapsulation types */
+#define UDP_ENCAP_NONE		0
 #define UDP_ENCAP_ESPINUDP_NON_IKE	1 /* unused  draft-ietf-ipsec-nat-t-ike-00/01 */
 #define UDP_ENCAP_ESPINUDP	2 /* draft-ietf-ipsec-udp-encaps-06 */
 #define UDP_ENCAP_L2TPINUDP	3 /* rfc2661 */
@@ -43,5 +44,17 @@ struct udphdr {
 #define UDP_ENCAP_GTP1U		5 /* 3GPP TS 29.060 */
 #define UDP_ENCAP_RXRPC		6
 #define TCP_ENCAP_ESPINTCP	7 /* Yikes, this is really xfrm encap types. */
+#define UDP_ENCAP_TIPC		8
+#define UDP_ENCAP_FOU		9
+#define UDP_ENCAP_GUE		10
+#define UDP_ENCAP_SCTP		11
+#define UDP_ENCAP_RXE		12
+#define UDP_ENCAP_PFCP		13
+#define UDP_ENCAP_WIREGUARD	14
+#define UDP_ENCAP_BAREUDP	15
+#define UDP_ENCAP_VXLAN		16
+#define UDP_ENCAP_VXLAN_GPE	17
+#define UDP_ENCAP_GENEVE	18
+#define UDP_ENCAP_AMT		19
 
 #endif /* _UAPI_LINUX_UDP_H */
-- 
2.34.1

Re: [PATCH 04/12] udp_encaps: Add new UDP_ENCAP constants

[Willem de Bruijn]

Tom Herbert wrote:
> Add constants for various UDP encapsulations that are supported
> 
> Signed-off-by: Tom Herbert <tom@herbertland.com>
> ---
>  include/uapi/linux/udp.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/include/uapi/linux/udp.h b/include/uapi/linux/udp.h
> index 1a0fe8b151fb..0432a9a6536d 100644
> --- a/include/uapi/linux/udp.h
> +++ b/include/uapi/linux/udp.h
> @@ -36,6 +36,7 @@ struct udphdr {
>  #define UDP_GRO		104	/* This socket can receive UDP GRO packets */
>  
>  /* UDP encapsulation types */
> +#define UDP_ENCAP_NONE		0
>  #define UDP_ENCAP_ESPINUDP_NON_IKE	1 /* unused  draft-ietf-ipsec-nat-t-ike-00/01 */
>  #define UDP_ENCAP_ESPINUDP	2 /* draft-ietf-ipsec-udp-encaps-06 */
>  #define UDP_ENCAP_L2TPINUDP	3 /* rfc2661 */
> @@ -43,5 +44,17 @@ struct udphdr {
>  #define UDP_ENCAP_GTP1U		5 /* 3GPP TS 29.060 */
>  #define UDP_ENCAP_RXRPC		6
>  #define TCP_ENCAP_ESPINTCP	7 /* Yikes, this is really xfrm encap types. */
> +#define UDP_ENCAP_TIPC		8
> +#define UDP_ENCAP_FOU		9
> +#define UDP_ENCAP_GUE		10
> +#define UDP_ENCAP_SCTP		11
> +#define UDP_ENCAP_RXE		12
> +#define UDP_ENCAP_PFCP		13
> +#define UDP_ENCAP_WIREGUARD	14
> +#define UDP_ENCAP_BAREUDP	15
> +#define UDP_ENCAP_VXLAN		16
> +#define UDP_ENCAP_VXLAN_GPE	17
> +#define UDP_ENCAP_GENEVE	18
> +#define UDP_ENCAP_AMT		19

Should these existing constants never have been UAPI to begin with?

Re: [PATCH 04/12] udp_encaps: Add new UDP_ENCAP constants

[Tom Herbert]

On Thu, Aug 1, 2024 at 6:22 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Tom Herbert wrote:
> > Add constants for various UDP encapsulations that are supported
> >
> > Signed-off-by: Tom Herbert <tom@herbertland.com>
> > ---
> >  include/uapi/linux/udp.h | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/include/uapi/linux/udp.h b/include/uapi/linux/udp.h
> > index 1a0fe8b151fb..0432a9a6536d 100644
> > --- a/include/uapi/linux/udp.h
> > +++ b/include/uapi/linux/udp.h
> > @@ -36,6 +36,7 @@ struct udphdr {
> >  #define UDP_GRO              104     /* This socket can receive UDP GRO packets */
> >
> >  /* UDP encapsulation types */
> > +#define UDP_ENCAP_NONE               0
> >  #define UDP_ENCAP_ESPINUDP_NON_IKE   1 /* unused  draft-ietf-ipsec-nat-t-ike-00/01 */
> >  #define UDP_ENCAP_ESPINUDP   2 /* draft-ietf-ipsec-udp-encaps-06 */
> >  #define UDP_ENCAP_L2TPINUDP  3 /* rfc2661 */
> > @@ -43,5 +44,17 @@ struct udphdr {
> >  #define UDP_ENCAP_GTP1U              5 /* 3GPP TS 29.060 */
> >  #define UDP_ENCAP_RXRPC              6
> >  #define TCP_ENCAP_ESPINTCP   7 /* Yikes, this is really xfrm encap types. */
> > +#define UDP_ENCAP_TIPC               8
> > +#define UDP_ENCAP_FOU                9
> > +#define UDP_ENCAP_GUE                10
> > +#define UDP_ENCAP_SCTP               11
> > +#define UDP_ENCAP_RXE                12
> > +#define UDP_ENCAP_PFCP               13
> > +#define UDP_ENCAP_WIREGUARD  14
> > +#define UDP_ENCAP_BAREUDP    15
> > +#define UDP_ENCAP_VXLAN              16
> > +#define UDP_ENCAP_VXLAN_GPE  17
> > +#define UDP_ENCAP_GENEVE     18
> > +#define UDP_ENCAP_AMT                19
>
> Should these existing constants never have been UAPI to begin with?

Hi Willem,

I'm inclined to think they probably should be, especially if we need
these in eBPF.

Tom

>

[PATCH 05/12] udp_encaps: Set proper UDP_ENCAP types in tunnel setup

[Tom Herbert]

Instead of just setting UDP tunnel config encap_type to 1,
use the appropriate constat for the tunnel type. This value
can be used to determine the encapsulated protocol in UDP
by looking at the socket

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 drivers/infiniband/sw/rxe/rxe_net.c | 2 +-
 drivers/net/amt.c                   | 2 +-
 drivers/net/bareudp.c               | 2 +-
 drivers/net/geneve.c                | 2 +-
 drivers/net/pfcp.c                  | 2 +-
 drivers/net/vxlan/vxlan_core.c      | 3 ++-
 drivers/net/wireguard/socket.c      | 2 +-
 net/ipv4/fou_core.c                 | 3 ++-
 net/sctp/protocol.c                 | 2 +-
 net/tipc/udp_media.c                | 2 +-
 10 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 75d1407db52d..1c2bb88132c5 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -193,7 +193,7 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
 	if (err < 0)
 		return ERR_PTR(err);
 
-	tnl_cfg.encap_type = 1;
+	tnl_cfg.encap_type = UDP_ENCAP_RXE;
 	tnl_cfg.encap_rcv = rxe_udp_encap_recv;
 
 	/* Setup UDP tunnel */
diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index 6d15ab3bfbbc..fc421cf2c032 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -2970,7 +2970,7 @@ static int amt_socket_create(struct amt_dev *amt)
 	/* Mark socket as an encapsulation socket */
 	memset(&tunnel_cfg, 0, sizeof(tunnel_cfg));
 	tunnel_cfg.sk_user_data = amt;
-	tunnel_cfg.encap_type = 1;
+	tunnel_cfg.encap_type = UDP_ENCAP_AMT;
 	tunnel_cfg.encap_rcv = amt_rcv;
 	tunnel_cfg.encap_err_lookup = amt_err_lookup;
 	tunnel_cfg.encap_destroy = NULL;
diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
index d5c56ca91b77..007fb8c5168b 100644
--- a/drivers/net/bareudp.c
+++ b/drivers/net/bareudp.c
@@ -248,7 +248,7 @@ static int bareudp_socket_create(struct bareudp_dev *bareudp, __be16 port)
 	/* Mark socket as an encapsulation socket */
 	memset(&tunnel_cfg, 0, sizeof(tunnel_cfg));
 	tunnel_cfg.sk_user_data = bareudp;
-	tunnel_cfg.encap_type = 1;
+	tunnel_cfg.encap_type = UDP_ENCAP_BAREUDP;
 	tunnel_cfg.encap_rcv = bareudp_udp_encap_recv;
 	tunnel_cfg.encap_err_lookup = bareudp_err_lookup;
 	tunnel_cfg.encap_destroy = NULL;
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 838e85ddec67..923c573b6e5c 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -611,7 +611,7 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
 	/* Mark socket as an encapsulation socket */
 	memset(&tunnel_cfg, 0, sizeof(tunnel_cfg));
 	tunnel_cfg.sk_user_data = gs;
-	tunnel_cfg.encap_type = 1;
+	tunnel_cfg.encap_type = UDP_ENCAP_GENEVE;
 	tunnel_cfg.gro_receive = geneve_gro_receive;
 	tunnel_cfg.gro_complete = geneve_gro_complete;
 	tunnel_cfg.encap_rcv = geneve_udp_encap_recv;
diff --git a/drivers/net/pfcp.c b/drivers/net/pfcp.c
index 69434fd13f96..c7e4fa606b16 100644
--- a/drivers/net/pfcp.c
+++ b/drivers/net/pfcp.c
@@ -170,7 +170,7 @@ static struct socket *pfcp_create_sock(struct pfcp_dev *pfcp)
 
 	tuncfg.sk_user_data = pfcp;
 	tuncfg.encap_rcv = pfcp_encap_recv;
-	tuncfg.encap_type = 1;
+	tuncfg.encap_type = UDP_ENCAP_PFCP;
 
 	setup_udp_tunnel_sock(net, sock, &tuncfg);
 
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index ba59e92ab941..38715186aac4 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -3572,7 +3572,8 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, bool ipv6,
 	/* Mark socket as an encapsulation socket. */
 	memset(&tunnel_cfg, 0, sizeof(tunnel_cfg));
 	tunnel_cfg.sk_user_data = vs;
-	tunnel_cfg.encap_type = 1;
+	tunnel_cfg.encap_type = vs->flags & VXLAN_F_GPE ?
+			UDP_ENCAP_VXLAN_GPE : UDP_ENCAP_VXLAN;
 	tunnel_cfg.encap_rcv = vxlan_rcv;
 	tunnel_cfg.encap_err_lookup = vxlan_err_lookup;
 	tunnel_cfg.encap_destroy = NULL;
diff --git a/drivers/net/wireguard/socket.c b/drivers/net/wireguard/socket.c
index 0414d7a6ce74..f4b5bd14fd56 100644
--- a/drivers/net/wireguard/socket.c
+++ b/drivers/net/wireguard/socket.c
@@ -352,7 +352,7 @@ int wg_socket_init(struct wg_device *wg, u16 port)
 	int ret;
 	struct udp_tunnel_sock_cfg cfg = {
 		.sk_user_data = wg,
-		.encap_type = 1,
+		.encap_type = UDP_ENCAP_WIREGUARD,
 		.encap_rcv = wg_receive
 	};
 	struct socket *new4 = NULL, *new6 = NULL;
diff --git a/net/ipv4/fou_core.c b/net/ipv4/fou_core.c
index 0abbc413e0fe..8241f762e45b 100644
--- a/net/ipv4/fou_core.c
+++ b/net/ipv4/fou_core.c
@@ -578,19 +578,20 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
 	fou->sock = sock;
 
 	memset(&tunnel_cfg, 0, sizeof(tunnel_cfg));
-	tunnel_cfg.encap_type = 1;
 	tunnel_cfg.sk_user_data = fou;
 	tunnel_cfg.encap_destroy = NULL;
 
 	/* Initial for fou type */
 	switch (cfg->type) {
 	case FOU_ENCAP_DIRECT:
+		tunnel_cfg.encap_type = UDP_ENCAP_FOU;
 		tunnel_cfg.encap_rcv = fou_udp_recv;
 		tunnel_cfg.gro_receive = fou_gro_receive;
 		tunnel_cfg.gro_complete = fou_gro_complete;
 		fou->protocol = cfg->protocol;
 		break;
 	case FOU_ENCAP_GUE:
+		tunnel_cfg.encap_type = UDP_ENCAP_GUE;
 		tunnel_cfg.encap_rcv = gue_udp_recv;
 		tunnel_cfg.gro_receive = gue_gro_receive;
 		tunnel_cfg.gro_complete = gue_gro_complete;
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 5a7436a13b74..290ebcf17a48 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -876,7 +876,7 @@ int sctp_udp_sock_start(struct net *net)
 		return err;
 	}
 
-	tuncfg.encap_type = 1;
+	tuncfg.encap_type = UDP_ENCAP_SCTP;
 	tuncfg.encap_rcv = sctp_udp_rcv;
 	tuncfg.encap_err_lookup = sctp_udp_v4_err;
 	setup_udp_tunnel_sock(net, sock, &tuncfg);
diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index 439f75539977..3c081b7b9d67 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -771,7 +771,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
 	if (err)
 		goto err;
 	tuncfg.sk_user_data = ub;
-	tuncfg.encap_type = 1;
+	tuncfg.encap_type = UDP_ENCAP_TIPC;
 	tuncfg.encap_rcv = tipc_udp_recv;
 	tuncfg.encap_destroy = NULL;
 	setup_udp_tunnel_sock(net, ub->ubsock, &tuncfg);
-- 
2.34.1

Re: [PATCH 05/12] udp_encaps: Set proper UDP_ENCAP types in tunnel setup

[Willem de Bruijn]

Tom Herbert wrote:
> Instead of just setting UDP tunnel config encap_type to 1,
> use the appropriate constat for the tunnel type. This value
> can be used to determine the encapsulated protocol in UDP
> by looking at the socket
> 
> Signed-off-by: Tom Herbert <tom@herbertland.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

[PATCH 06/12] flow_dissector: UDP encap infrastructure

[Tom Herbert]

Add infrastructure for parsing into UDP encapsulations

Add function __skb_flow_dissect_udp that is called for IPPROTO_UDP.
The flag FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS enables parsing of UDP
encapsulations. If the flag is set when parsing a UDP packet then
a socket lookup is performed. The offset of the base network header,
either an IPv4 or IPv6 header, is tracked and passed to
__skb_flow_dissect_udp so that it can perform the socket lookup

If a socket is found and it's for a UDP encapsulation (encap_type is
set in the UDP socket) then a switch is performed on the encap_type
value (cases are UDP_ENCAP_* values)

An encapsulated packet in UDP can either be indicated by an
EtherType or IP protocol. The processing for dissecting a UDP encap
protocol returns a flow dissector return code. If
FLOW_DISSECT_RET_PROTO_AGAIN or FLOW_DISSECT_RET_IPPROTO_AGAIN is
returned then the corresponding  encapsulated protocol is dissected.
The nhoff is set to point to the header to process.  In the case
FLOW_DISSECT_RET_PROTO_AGAIN the EtherType protocol is returned and
the IP protocol is set to zero. In the case of
FLOW_DISSECT_RET_IPPROTO_AGAIN, the IP protocol is returned and
the EtherType protocol is returned unchanged

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/flow_dissector.h |   1 +
 net/core/flow_dissector.c    | 114 +++++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+)

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index ced79dc8e856..8a868a88a6f1 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -384,6 +384,7 @@ enum flow_dissector_key_id {
 #define FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL	BIT(1)
 #define FLOW_DISSECTOR_F_STOP_AT_ENCAP		BIT(2)
 #define FLOW_DISSECTOR_F_STOP_BEFORE_ENCAP	BIT(3)
+#define FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS	BIT(4)
 
 struct flow_dissector_key {
 	enum flow_dissector_key_id key_id;
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 416f889c623c..006db3b893d0 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -13,6 +13,7 @@
 #include <net/gre.h>
 #include <net/pptp.h>
 #include <net/tipc.h>
+#include <net/udp.h>
 #include <linux/igmp.h>
 #include <linux/icmp.h>
 #include <linux/sctp.h>
@@ -806,6 +807,110 @@ __skb_flow_dissect_batadv(const struct sk_buff *skb,
 	return FLOW_DISSECT_RET_PROTO_AGAIN;
 }
 
+static enum flow_dissect_ret
+__skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
+		       struct flow_dissector *flow_dissector,
+		       void *target_container, const void *data,
+		       int *p_nhoff, int hlen, __be16 *p_proto,
+		       u8 *p_ip_proto, int bpoff, unsigned int flags)
+{
+	enum flow_dissect_ret ret;
+	const struct udphdr *udph;
+	struct udphdr _udph;
+	struct sock *sk;
+	__u8 encap_type;
+	int nhoff;
+
+	if (!(flags & FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS))
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	switch (*p_proto) {
+	case htons(ETH_P_IP): {
+		const struct iphdr *iph;
+		struct iphdr _iph;
+
+		iph = __skb_header_pointer(skb, bpoff, sizeof(_iph), data,
+					   hlen, &_iph);
+		if (!iph)
+			return FLOW_DISSECT_RET_OUT_BAD;
+
+		udph = __skb_header_pointer(skb, *p_nhoff, sizeof(_udph), data,
+					    hlen, &_udph);
+		if (!udph)
+			return FLOW_DISSECT_RET_OUT_BAD;
+
+		rcu_read_lock();
+		/* Look up the UDPv4 socket and get the encap_type */
+		sk = __udp4_lib_lookup(net, iph->saddr, udph->source,
+				       iph->daddr, udph->dest,
+				       inet_iif(skb), inet_sdif(skb),
+				       net->ipv4.udp_table, NULL);
+		if (!sk || !udp_sk(sk)->encap_type) {
+			rcu_read_unlock();
+			return FLOW_DISSECT_RET_OUT_GOOD;
+		}
+
+		encap_type = udp_sk(sk)->encap_type;
+		rcu_read_unlock();
+
+		break;
+	}
+	case htons(ETH_P_IPV6): {
+		const struct ipv6hdr *iph;
+		struct ipv6hdr _iph;
+
+		iph = __skb_header_pointer(skb, bpoff, sizeof(_iph), data,
+					   hlen, &_iph);
+		if (!iph)
+			return FLOW_DISSECT_RET_OUT_BAD;
+
+		udph = __skb_header_pointer(skb, *p_nhoff, sizeof(_udph), data,
+					    hlen, &_udph);
+		if (!udph)
+			return FLOW_DISSECT_RET_OUT_BAD;
+
+		rcu_read_lock();
+		/* Look up the UDPv6 socket and get the encap_type */
+		sk = __udp6_lib_lookup(net, &iph->saddr, udph->source,
+				       &iph->daddr, udph->dest,
+				       inet_iif(skb), inet_sdif(skb),
+				       net->ipv4.udp_table, NULL);
+		if (!sk || !udp_sk(sk)->encap_type) {
+			rcu_read_unlock();
+			return FLOW_DISSECT_RET_OUT_GOOD;
+		}
+
+		encap_type = udp_sk(sk)->encap_type;
+		rcu_read_unlock();
+
+		break;
+	}
+	default:
+		return FLOW_DISSECT_RET_OUT_GOOD;
+	}
+
+	nhoff = *p_nhoff + sizeof(struct udphdr);
+	ret = FLOW_DISSECT_RET_OUT_GOOD;
+
+	switch (encap_type) {
+	default:
+		break;
+	}
+
+	switch (ret) {
+	case FLOW_DISSECT_RET_PROTO_AGAIN:
+		*p_ip_proto = 0;
+		fallthrough;
+	case FLOW_DISSECT_RET_IPPROTO_AGAIN:
+		*p_nhoff = nhoff;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
 static void
 __skb_flow_dissect_tcp(const struct sk_buff *skb,
 		       struct flow_dissector *flow_dissector,
@@ -1046,6 +1151,7 @@ bool __skb_flow_dissect(struct net *net,
 	int mpls_lse = 0;
 	int num_hdrs = 0;
 	u8 ip_proto = 0;
+	int bpoff;
 	bool ret;
 
 	if (!data) {
@@ -1168,6 +1274,7 @@ bool __skb_flow_dissect(struct net *net,
 
 proto_again:
 	fdret = FLOW_DISSECT_RET_CONTINUE;
+	bpoff = nhoff;
 
 	switch (proto) {
 	case htons(ETH_P_IP): {
@@ -1635,6 +1742,13 @@ bool __skb_flow_dissect(struct net *net,
 				       data, nhoff, hlen);
 		break;
 
+	case IPPROTO_UDP:
+		fdret = __skb_flow_dissect_udp(skb, net, flow_dissector,
+					       target_container, data, &nhoff,
+					       hlen, &proto, &ip_proto,
+					       bpoff, flags);
+		break;
+
 	case IPPROTO_ICMP:
 	case IPPROTO_ICMPV6:
 		__skb_flow_dissect_icmp(skb, flow_dissector, target_container,
-- 
2.34.1

Re: [PATCH 06/12] flow_dissector: UDP encap infrastructure

[Willem de Bruijn]

Tom Herbert wrote:
> Add infrastructure for parsing into UDP encapsulations
> 
> Add function __skb_flow_dissect_udp that is called for IPPROTO_UDP.
> The flag FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS enables parsing of UDP
> encapsulations. If the flag is set when parsing a UDP packet then
> a socket lookup is performed. The offset of the base network header,
> either an IPv4 or IPv6 header, is tracked and passed to
> __skb_flow_dissect_udp so that it can perform the socket lookup
> 
> If a socket is found and it's for a UDP encapsulation (encap_type is
> set in the UDP socket) then a switch is performed on the encap_type
> value (cases are UDP_ENCAP_* values)
> 
> An encapsulated packet in UDP can either be indicated by an
> EtherType or IP protocol. The processing for dissecting a UDP encap
> protocol returns a flow dissector return code. If
> FLOW_DISSECT_RET_PROTO_AGAIN or FLOW_DISSECT_RET_IPPROTO_AGAIN is
> returned then the corresponding  encapsulated protocol is dissected.
> The nhoff is set to point to the header to process.  In the case
> FLOW_DISSECT_RET_PROTO_AGAIN the EtherType protocol is returned and
> the IP protocol is set to zero. In the case of
> FLOW_DISSECT_RET_IPPROTO_AGAIN, the IP protocol is returned and
> the EtherType protocol is returned unchanged
> 
> Signed-off-by: Tom Herbert <tom@herbertland.com>
> ---
>  include/net/flow_dissector.h |   1 +
>  net/core/flow_dissector.c    | 114 +++++++++++++++++++++++++++++++++++
>  2 files changed, 115 insertions(+)
> 

> +static enum flow_dissect_ret
> +__skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
> +		       struct flow_dissector *flow_dissector,
> +		       void *target_container, const void *data,
> +		       int *p_nhoff, int hlen, __be16 *p_proto,
> +		       u8 *p_ip_proto, int bpoff, unsigned int flags)
> +{
> +	enum flow_dissect_ret ret;
> +	const struct udphdr *udph;
> +	struct udphdr _udph;
> +	struct sock *sk;
> +	__u8 encap_type;
> +	int nhoff;
> +
> +	if (!(flags & FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS))
> +		return FLOW_DISSECT_RET_OUT_GOOD;
> +
> +	switch (*p_proto) {
> +	case htons(ETH_P_IP): {
> +		const struct iphdr *iph;
> +		struct iphdr _iph;
> +
> +		iph = __skb_header_pointer(skb, bpoff, sizeof(_iph), data,
> +					   hlen, &_iph);
> +		if (!iph)
> +			return FLOW_DISSECT_RET_OUT_BAD;
> +
> +		udph = __skb_header_pointer(skb, *p_nhoff, sizeof(_udph), data,
> +					    hlen, &_udph);
> +		if (!udph)
> +			return FLOW_DISSECT_RET_OUT_BAD;
> +
> +		rcu_read_lock();
> +		/* Look up the UDPv4 socket and get the encap_type */
> +		sk = __udp4_lib_lookup(net, iph->saddr, udph->source,
> +				       iph->daddr, udph->dest,
> +				       inet_iif(skb), inet_sdif(skb),
> +				       net->ipv4.udp_table, NULL);
> +		if (!sk || !udp_sk(sk)->encap_type) {
> +			rcu_read_unlock();
> +			return FLOW_DISSECT_RET_OUT_GOOD;
> +		}
> +
> +		encap_type = udp_sk(sk)->encap_type;
> +		rcu_read_unlock();
> +
> +		break;
> +	}
> +	case htons(ETH_P_IPV6): {
> +		const struct ipv6hdr *iph;
> +		struct ipv6hdr _iph;
> +
> +		iph = __skb_header_pointer(skb, bpoff, sizeof(_iph), data,
> +					   hlen, &_iph);
> +		if (!iph)
> +			return FLOW_DISSECT_RET_OUT_BAD;
> +
> +		udph = __skb_header_pointer(skb, *p_nhoff, sizeof(_udph), data,
> +					    hlen, &_udph);
> +		if (!udph)
> +			return FLOW_DISSECT_RET_OUT_BAD;
> +
> +		rcu_read_lock();
> +		/* Look up the UDPv6 socket and get the encap_type */
> +		sk = __udp6_lib_lookup(net, &iph->saddr, udph->source,
> +				       &iph->daddr, udph->dest,
> +				       inet_iif(skb), inet_sdif(skb),
> +				       net->ipv4.udp_table, NULL);

#if IS_ENABLED(CONFIG_IPV6)

similar to net/ipv4/udp_diag.c

> +		if (!sk || !udp_sk(sk)->encap_type) {
> +			rcu_read_unlock();
> +			return FLOW_DISSECT_RET_OUT_GOOD;
> +		}
> +
> +		encap_type = udp_sk(sk)->encap_type;
> +		rcu_read_unlock();
> +
> +		break;
> +	}
> +	default:
> +		return FLOW_DISSECT_RET_OUT_GOOD;
> +	}
> +

>  static void
>  __skb_flow_dissect_tcp(const struct sk_buff *skb,
>  		       struct flow_dissector *flow_dissector,
> @@ -1046,6 +1151,7 @@ bool __skb_flow_dissect(struct net *net,
>  	int mpls_lse = 0;
>  	int num_hdrs = 0;
>  	u8 ip_proto = 0;
> +	int bpoff;

What does bp mean here?

It points to the network header off, but nhoff does not mean network
header, but next header. And now points to the udp header. Just not
sure what bp is meant to convey.

Re: [PATCH 06/12] flow_dissector: UDP encap infrastructure

[kernel test robot]

Hi Tom,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]
[also build test ERROR on net/main linus/master v6.11-rc1 next-20240802]
[cannot apply to horms-ipvs/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/Tom-Herbert/skbuff-Unconstantify-struct-net-argument-in-flowdis-functions/20240802-084418
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240731172332.683815-7-tom%40herbertland.com
patch subject: [PATCH 06/12] flow_dissector: UDP encap infrastructure
config: arc-vdk_hs38_defconfig (https://download.01.org/0day-ci/archive/20240802/202408022051.e0Dqh7P1-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240802/202408022051.e0Dqh7P1-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/202408022051.e0Dqh7P1-lkp@intel.com/

All errors (new ones prefixed by >>):

   arc-elf-ld: net/core/flow_dissector.o: in function `__skb_flow_dissect_udp.isra.0':
   flow_dissector.c:(.text+0x81c): undefined reference to `__udp6_lib_lookup'
>> arc-elf-ld: flow_dissector.c:(.text+0x81c): undefined reference to `__udp6_lib_lookup'

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

Re: [PATCH 06/12] flow_dissector: UDP encap infrastructure

[kernel test robot]

Hi Tom,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]
[also build test ERROR on net/main linus/master v6.11-rc1 next-20240802]
[cannot apply to horms-ipvs/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/Tom-Herbert/skbuff-Unconstantify-struct-net-argument-in-flowdis-functions/20240802-084418
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240731172332.683815-7-tom%40herbertland.com
patch subject: [PATCH 06/12] flow_dissector: UDP encap infrastructure
config: um-defconfig (https://download.01.org/0day-ci/archive/20240802/202408022046.h8kMQ01e-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 430b90f04533b099d788db2668176038be38c53b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240802/202408022046.h8kMQ01e-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/202408022046.h8kMQ01e-lkp@intel.com/

All errors (new ones prefixed by >>):

   /usr/bin/ld: warning: .tmp_vmlinux1 has a LOAD segment with RWX permissions
   /usr/bin/ld: net/core/flow_dissector.o: in function `__skb_flow_dissect':
>> net/core/flow_dissector.c:874:(.ltext+0x1ff9): undefined reference to `__udp6_lib_lookup'
   clang: error: linker command failed with exit code 1 (use -v to see invocation)


vim +874 net/core/flow_dissector.c

   809	
   810	static enum flow_dissect_ret
   811	__skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
   812			       struct flow_dissector *flow_dissector,
   813			       void *target_container, const void *data,
   814			       int *p_nhoff, int hlen, __be16 *p_proto,
   815			       u8 *p_ip_proto, int bpoff, unsigned int flags)
   816	{
   817		enum flow_dissect_ret ret;
   818		const struct udphdr *udph;
   819		struct udphdr _udph;
   820		struct sock *sk;
   821		__u8 encap_type;
   822		int nhoff;
   823	
   824		if (!(flags & FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS))
   825			return FLOW_DISSECT_RET_OUT_GOOD;
   826	
   827		switch (*p_proto) {
   828		case htons(ETH_P_IP): {
   829			const struct iphdr *iph;
   830			struct iphdr _iph;
   831	
   832			iph = __skb_header_pointer(skb, bpoff, sizeof(_iph), data,
   833						   hlen, &_iph);
   834			if (!iph)
   835				return FLOW_DISSECT_RET_OUT_BAD;
   836	
   837			udph = __skb_header_pointer(skb, *p_nhoff, sizeof(_udph), data,
   838						    hlen, &_udph);
   839			if (!udph)
   840				return FLOW_DISSECT_RET_OUT_BAD;
   841	
   842			rcu_read_lock();
   843			/* Look up the UDPv4 socket and get the encap_type */
   844			sk = __udp4_lib_lookup(net, iph->saddr, udph->source,
   845					       iph->daddr, udph->dest,
   846					       inet_iif(skb), inet_sdif(skb),
   847					       net->ipv4.udp_table, NULL);
   848			if (!sk || !udp_sk(sk)->encap_type) {
   849				rcu_read_unlock();
   850				return FLOW_DISSECT_RET_OUT_GOOD;
   851			}
   852	
   853			encap_type = udp_sk(sk)->encap_type;
   854			rcu_read_unlock();
   855	
   856			break;
   857		}
   858		case htons(ETH_P_IPV6): {
   859			const struct ipv6hdr *iph;
   860			struct ipv6hdr _iph;
   861	
   862			iph = __skb_header_pointer(skb, bpoff, sizeof(_iph), data,
   863						   hlen, &_iph);
   864			if (!iph)
   865				return FLOW_DISSECT_RET_OUT_BAD;
   866	
   867			udph = __skb_header_pointer(skb, *p_nhoff, sizeof(_udph), data,
   868						    hlen, &_udph);
   869			if (!udph)
   870				return FLOW_DISSECT_RET_OUT_BAD;
   871	
   872			rcu_read_lock();
   873			/* Look up the UDPv6 socket and get the encap_type */
 > 874			sk = __udp6_lib_lookup(net, &iph->saddr, udph->source,
   875					       &iph->daddr, udph->dest,
   876					       inet_iif(skb), inet_sdif(skb),
   877					       net->ipv4.udp_table, NULL);
   878			if (!sk || !udp_sk(sk)->encap_type) {
   879				rcu_read_unlock();
   880				return FLOW_DISSECT_RET_OUT_GOOD;
   881			}
   882	
   883			encap_type = udp_sk(sk)->encap_type;
   884			rcu_read_unlock();
   885	
   886			break;
   887		}
   888		default:
   889			return FLOW_DISSECT_RET_OUT_GOOD;
   890		}
   891	
   892		nhoff = *p_nhoff + sizeof(struct udphdr);
   893		ret = FLOW_DISSECT_RET_OUT_GOOD;
   894	
   895		switch (encap_type) {
   896		default:
   897			break;
   898		}
   899	
   900		switch (ret) {
   901		case FLOW_DISSECT_RET_PROTO_AGAIN:
   902			*p_ip_proto = 0;
   903			fallthrough;
   904		case FLOW_DISSECT_RET_IPPROTO_AGAIN:
   905			*p_nhoff = nhoff;
   906			break;
   907		default:
   908			break;
   909		}
   910	
   911		return ret;
   912	}
   913	

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

[PATCH 07/12] flow_dissector: Parse vxlan in UDP

[Tom Herbert]

Parse vxlan in a UDP encapsulation

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/core/flow_dissector.c | 57 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 006db3b893d0..6ad45b09dda4 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -13,7 +13,9 @@
 #include <net/gre.h>
 #include <net/pptp.h>
 #include <net/tipc.h>
+#include <net/tun_proto.h>
 #include <net/udp.h>
+#include <net/vxlan.h>
 #include <linux/igmp.h>
 #include <linux/icmp.h>
 #include <linux/sctp.h>
@@ -756,6 +758,55 @@ __skb_flow_dissect_gre(const struct sk_buff *skb,
 	return FLOW_DISSECT_RET_PROTO_AGAIN;
 }
 
+static enum flow_dissect_ret
+__skb_flow_dissect_vxlan(const struct sk_buff *skb,
+			 struct flow_dissector *flow_dissector,
+			 void *target_container, const void *data,
+			 __be16 *p_proto, int *p_nhoff, int hlen,
+			 unsigned int flags)
+{
+	struct vxlanhdr *hdr, _hdr;
+	__be16 protocol;
+
+	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
+				   &_hdr);
+	if (!hdr)
+		return FLOW_DISSECT_RET_OUT_BAD;
+
+	/* VNI flag always required to be set */
+	if (!(hdr->vx_flags & VXLAN_HF_VNI))
+		return FLOW_DISSECT_RET_OUT_BAD;
+
+	if (hdr->vx_flags & VXLAN_F_GPE) {
+		struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)hdr;
+
+		/* Need to have Next Protocol set for interfaces in GPE mode. */
+		if (!gpe->np_applied)
+			return FLOW_DISSECT_RET_OUT_BAD;
+
+		/* The initial version is 0 */
+		if (gpe->version != 0)
+			return FLOW_DISSECT_RET_OUT_GOOD;
+
+		/* "When the O bit is set to 1, the packet is an OAM packet and
+		 * OAM so ignore
+		 */
+		if (gpe->oam_flag)
+			return FLOW_DISSECT_RET_OUT_GOOD;
+
+		protocol = tun_p_to_eth_p(gpe->next_protocol);
+		if (!protocol)
+			return FLOW_DISSECT_RET_OUT_GOOD;
+	} else {
+		protocol = htons(ETH_P_TEB);
+	}
+
+	*p_nhoff += sizeof(struct vxlanhdr);
+	*p_proto = protocol;
+
+	return FLOW_DISSECT_RET_PROTO_AGAIN;
+}
+
 /**
  * __skb_flow_dissect_batadv() - dissect batman-adv header
  * @skb: sk_buff to with the batman-adv header
@@ -893,6 +944,12 @@ __skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
 	ret = FLOW_DISSECT_RET_OUT_GOOD;
 
 	switch (encap_type) {
+	case UDP_ENCAP_VXLAN:
+	case UDP_ENCAP_VXLAN_GPE:
+		ret = __skb_flow_dissect_vxlan(skb, flow_dissector,
+					       target_container, data,
+					       p_proto, &nhoff, hlen, flags);
+		break;
 	default:
 		break;
 	}
-- 
2.34.1

Re: [PATCH 07/12] flow_dissector: Parse vxlan in UDP

[Simon Horman]

On Wed, Jul 31, 2024 at 10:23:27AM -0700, Tom Herbert wrote:
> Parse vxlan in a UDP encapsulation
> 
> Signed-off-by: Tom Herbert <tom@herbertland.com>
> ---
>  net/core/flow_dissector.c | 57 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c

...

> @@ -756,6 +758,55 @@ __skb_flow_dissect_gre(const struct sk_buff *skb,
>  	return FLOW_DISSECT_RET_PROTO_AGAIN;
>  }
>  
> +static enum flow_dissect_ret
> +__skb_flow_dissect_vxlan(const struct sk_buff *skb,
> +			 struct flow_dissector *flow_dissector,
> +			 void *target_container, const void *data,
> +			 __be16 *p_proto, int *p_nhoff, int hlen,
> +			 unsigned int flags)
> +{
> +	struct vxlanhdr *hdr, _hdr;
> +	__be16 protocol;
> +
> +	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
> +				   &_hdr);
> +	if (!hdr)
> +		return FLOW_DISSECT_RET_OUT_BAD;
> +
> +	/* VNI flag always required to be set */
> +	if (!(hdr->vx_flags & VXLAN_HF_VNI))
> +		return FLOW_DISSECT_RET_OUT_BAD;
> +
> +	if (hdr->vx_flags & VXLAN_F_GPE) {

Hi Tom,

Sparse flags an byte-order miss match on the line above.
I expect this would resolve it (completely untested!):

	if (hdr->vx_flags & cpu_to_be32(VXLAN_F_GPE))

...

Re: [PATCH 07/12] flow_dissector: Parse vxlan in UDP

[kernel test robot]

Hi Tom,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]
[also build test WARNING on net/main linus/master v6.11-rc1 next-20240802]
[cannot apply to horms-ipvs/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/Tom-Herbert/skbuff-Unconstantify-struct-net-argument-in-flowdis-functions/20240802-084418
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240731172332.683815-8-tom%40herbertland.com
patch subject: [PATCH 07/12] flow_dissector: Parse vxlan in UDP
config: i386-randconfig-062-20240802 (https://download.01.org/0day-ci/archive/20240803/202408031144.ln4wxJc4-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240803/202408031144.ln4wxJc4-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/202408031144.ln4wxJc4-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> net/core/flow_dissector.c:780:16: sparse: sparse: restricted __be32 degrades to integer
   net/core/flow_dissector.c: note: in included file (through include/linux/if_pppox.h):
   include/uapi/linux/if_pppox.h:153:29: sparse: sparse: array of flexible structures

vim +780 net/core/flow_dissector.c

   760	
   761	static enum flow_dissect_ret
   762	__skb_flow_dissect_vxlan(const struct sk_buff *skb,
   763				 struct flow_dissector *flow_dissector,
   764				 void *target_container, const void *data,
   765				 __be16 *p_proto, int *p_nhoff, int hlen,
   766				 unsigned int flags)
   767	{
   768		struct vxlanhdr *hdr, _hdr;
   769		__be16 protocol;
   770	
   771		hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
   772					   &_hdr);
   773		if (!hdr)
   774			return FLOW_DISSECT_RET_OUT_BAD;
   775	
   776		/* VNI flag always required to be set */
   777		if (!(hdr->vx_flags & VXLAN_HF_VNI))
   778			return FLOW_DISSECT_RET_OUT_BAD;
   779	
 > 780		if (hdr->vx_flags & VXLAN_F_GPE) {
   781			struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)hdr;
   782	
   783			/* Need to have Next Protocol set for interfaces in GPE mode. */
   784			if (!gpe->np_applied)
   785				return FLOW_DISSECT_RET_OUT_BAD;
   786	
   787			/* The initial version is 0 */
   788			if (gpe->version != 0)
   789				return FLOW_DISSECT_RET_OUT_GOOD;
   790	
   791			/* "When the O bit is set to 1, the packet is an OAM packet and
   792			 * OAM so ignore
   793			 */
   794			if (gpe->oam_flag)
   795				return FLOW_DISSECT_RET_OUT_GOOD;
   796	
   797			protocol = tun_p_to_eth_p(gpe->next_protocol);
   798			if (!protocol)
   799				return FLOW_DISSECT_RET_OUT_GOOD;
   800		} else {
   801			protocol = htons(ETH_P_TEB);
   802		}
   803	
   804		*p_nhoff += sizeof(struct vxlanhdr);
   805		*p_proto = protocol;
   806	
   807		return FLOW_DISSECT_RET_PROTO_AGAIN;
   808	}
   809	

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

[PATCH 08/12] flow_dissector: Parse foo-over-udp (FOU)

[Tom Herbert]

Parse FOU by getting the FOU protocol from the matching socket.
This includes moving "struct fou" and "fou_from_sock" to fou.h

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/fou.h         | 16 ++++++++++++++++
 net/core/flow_dissector.c | 13 ++++++++++++-
 net/ipv4/fou_core.c       | 16 ----------------
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/include/net/fou.h b/include/net/fou.h
index 824eb4b231fd..8574767b91b6 100644
--- a/include/net/fou.h
+++ b/include/net/fou.h
@@ -17,6 +17,22 @@ int __fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
 int __gue_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
 		       u8 *protocol, __be16 *sport, int type);
 
+struct fou {
+	struct socket *sock;
+	u8 protocol;
+	u8 flags;
+	__be16 port;
+	u8 family;
+	u16 type;
+	struct list_head list;
+	struct rcu_head rcu;
+};
+
+static inline struct fou *fou_from_sock(struct sock *sk)
+{
+	return sk->sk_user_data;
+}
+
 int register_fou_bpf(void);
 
 #endif
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 6ad45b09dda4..68906c4bb474 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -8,6 +8,7 @@
 #include <linux/filter.h>
 #include <net/dsa.h>
 #include <net/dst_metadata.h>
+#include <net/fou.h>
 #include <net/ip.h>
 #include <net/ipv6.h>
 #include <net/gre.h>
@@ -865,11 +866,11 @@ __skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
 		       int *p_nhoff, int hlen, __be16 *p_proto,
 		       u8 *p_ip_proto, int bpoff, unsigned int flags)
 {
+	__u8 encap_type, fou_protocol;
 	enum flow_dissect_ret ret;
 	const struct udphdr *udph;
 	struct udphdr _udph;
 	struct sock *sk;
-	__u8 encap_type;
 	int nhoff;
 
 	if (!(flags & FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS))
@@ -902,6 +903,9 @@ __skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
 		}
 
 		encap_type = udp_sk(sk)->encap_type;
+		if (encap_type == UDP_ENCAP_FOU)
+			fou_protocol = fou_from_sock(sk)->protocol;
+
 		rcu_read_unlock();
 
 		break;
@@ -932,6 +936,9 @@ __skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
 		}
 
 		encap_type = udp_sk(sk)->encap_type;
+		if (encap_type == UDP_ENCAP_FOU)
+			fou_protocol = fou_from_sock(sk)->protocol;
+
 		rcu_read_unlock();
 
 		break;
@@ -944,6 +951,10 @@ __skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
 	ret = FLOW_DISSECT_RET_OUT_GOOD;
 
 	switch (encap_type) {
+	case UDP_ENCAP_FOU:
+		*p_ip_proto = fou_protocol;
+		ret = FLOW_DISSECT_RET_IPPROTO_AGAIN;
+		break;
 	case UDP_ENCAP_VXLAN:
 	case UDP_ENCAP_VXLAN_GPE:
 		ret = __skb_flow_dissect_vxlan(skb, flow_dissector,
diff --git a/net/ipv4/fou_core.c b/net/ipv4/fou_core.c
index 8241f762e45b..137eb80c56a2 100644
--- a/net/ipv4/fou_core.c
+++ b/net/ipv4/fou_core.c
@@ -21,17 +21,6 @@
 
 #include "fou_nl.h"
 
-struct fou {
-	struct socket *sock;
-	u8 protocol;
-	u8 flags;
-	__be16 port;
-	u8 family;
-	u16 type;
-	struct list_head list;
-	struct rcu_head rcu;
-};
-
 #define FOU_F_REMCSUM_NOPARTIAL BIT(0)
 
 struct fou_cfg {
@@ -48,11 +37,6 @@ struct fou_net {
 	struct mutex fou_lock;
 };
 
-static inline struct fou *fou_from_sock(struct sock *sk)
-{
-	return sk->sk_user_data;
-}
-
 static int fou_recv_pull(struct sk_buff *skb, struct fou *fou, size_t len)
 {
 	/* Remove 'len' bytes from the packet (UDP header and
-- 
2.34.1

Re: [PATCH 08/12] flow_dissector: Parse foo-over-udp (FOU)

[Willem de Bruijn]

Tom Herbert wrote:
> Parse FOU by getting the FOU protocol from the matching socket.
> This includes moving "struct fou" and "fou_from_sock" to fou.h
> 
> Signed-off-by: Tom Herbert <tom@herbertland.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Will look at the other protocol handlers later.

[PATCH 09/12] flow_dissector: Parse ESP, L2TP, and SCTP in UDP

[Tom Herbert]

These don't have an encapsulation header so it's fairly easy to
support them

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/core/flow_dissector.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 68906c4bb474..3766dc4d5b23 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -951,10 +951,23 @@ __skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
 	ret = FLOW_DISSECT_RET_OUT_GOOD;
 
 	switch (encap_type) {
+	case UDP_ENCAP_ESPINUDP_NON_IKE:
+	case UDP_ENCAP_ESPINUDP:
+		*p_ip_proto = IPPROTO_ESP;
+		ret = FLOW_DISSECT_RET_IPPROTO_AGAIN;
+		break;
+	case UDP_ENCAP_L2TPINUDP:
+		*p_ip_proto = IPPROTO_L2TP;
+		ret = FLOW_DISSECT_RET_IPPROTO_AGAIN;
+		break;
 	case UDP_ENCAP_FOU:
 		*p_ip_proto = fou_protocol;
 		ret = FLOW_DISSECT_RET_IPPROTO_AGAIN;
 		break;
+	case UDP_ENCAP_SCTP:
+		*p_ip_proto = IPPROTO_SCTP;
+		ret = FLOW_DISSECT_RET_IPPROTO_AGAIN;
+		break;
 	case UDP_ENCAP_VXLAN:
 	case UDP_ENCAP_VXLAN_GPE:
 		ret = __skb_flow_dissect_vxlan(skb, flow_dissector,
-- 
2.34.1

[PATCH 10/12] flow_dissector: Parse Geneve in UDP

[Tom Herbert]

Parse Geneve in a UDP encapsulation

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/core/flow_dissector.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 3766dc4d5b23..4fff60233992 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -11,6 +11,7 @@
 #include <net/fou.h>
 #include <net/ip.h>
 #include <net/ipv6.h>
+#include <net/geneve.h>
 #include <net/gre.h>
 #include <net/pptp.h>
 #include <net/tipc.h>
@@ -808,6 +809,29 @@ __skb_flow_dissect_vxlan(const struct sk_buff *skb,
 	return FLOW_DISSECT_RET_PROTO_AGAIN;
 }
 
+static enum flow_dissect_ret
+__skb_flow_dissect_geneve(const struct sk_buff *skb,
+			  struct flow_dissector *flow_dissector,
+			  void *target_container, const void *data,
+			  __be16 *p_proto, int *p_nhoff, int hlen,
+			  unsigned int flags)
+{
+	struct genevehdr *hdr, _hdr;
+
+	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
+				   &_hdr);
+	if (!hdr)
+		return FLOW_DISSECT_RET_OUT_BAD;
+
+	if (hdr->ver != 0)
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	*p_proto = hdr->proto_type;
+	*p_nhoff += sizeof(struct genevehdr) + (hdr->opt_len * 4);
+
+	return FLOW_DISSECT_RET_PROTO_AGAIN;
+}
+
 /**
  * __skb_flow_dissect_batadv() - dissect batman-adv header
  * @skb: sk_buff to with the batman-adv header
@@ -974,6 +998,11 @@ __skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
 					       target_container, data,
 					       p_proto, &nhoff, hlen, flags);
 		break;
+	case UDP_ENCAP_GENEVE:
+		ret = __skb_flow_dissect_geneve(skb, flow_dissector,
+						target_container, data,
+						p_proto, &nhoff, hlen, flags);
+		break;
 	default:
 		break;
 	}
-- 
2.34.1

Re: [PATCH 10/12] flow_dissector: Parse Geneve in UDP

[Willem de Bruijn]

Tom Herbert wrote:
> Parse Geneve in a UDP encapsulation
> 
> Signed-off-by: Tom Herbert <tom@herbertland.com>
> ---
>  net/core/flow_dissector.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 3766dc4d5b23..4fff60233992 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -11,6 +11,7 @@
>  #include <net/fou.h>
>  #include <net/ip.h>
>  #include <net/ipv6.h>
> +#include <net/geneve.h>
>  #include <net/gre.h>
>  #include <net/pptp.h>
>  #include <net/tipc.h>
> @@ -808,6 +809,29 @@ __skb_flow_dissect_vxlan(const struct sk_buff *skb,
>  	return FLOW_DISSECT_RET_PROTO_AGAIN;
>  }
>  
> +static enum flow_dissect_ret
> +__skb_flow_dissect_geneve(const struct sk_buff *skb,
> +			  struct flow_dissector *flow_dissector,
> +			  void *target_container, const void *data,
> +			  __be16 *p_proto, int *p_nhoff, int hlen,
> +			  unsigned int flags)
> +{
> +	struct genevehdr *hdr, _hdr;
> +
> +	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
> +				   &_hdr);
> +	if (!hdr)
> +		return FLOW_DISSECT_RET_OUT_BAD;
> +
> +	if (hdr->ver != 0)
> +		return FLOW_DISSECT_RET_OUT_GOOD;
> +
> +	*p_proto = hdr->proto_type;
> +	*p_nhoff += sizeof(struct genevehdr) + (hdr->opt_len * 4);
> +
> +	return FLOW_DISSECT_RET_PROTO_AGAIN;

Do you want to return FLOW_DISSECT_RET_OUT_GOOD if IPPROTO 59.

Per your spec: "IP protocol number 59 ("No next header") may be set to
indicate that the GUE payload does not begin with the header of an IP
protocol."

Admittedly pendantic. No idea if any implementation actually sets
this.

> +}
> +
>  /**
>   * __skb_flow_dissect_batadv() - dissect batman-adv header
>   * @skb: sk_buff to with the batman-adv header
> @@ -974,6 +998,11 @@ __skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
>  					       target_container, data,
>  					       p_proto, &nhoff, hlen, flags);
>  		break;
> +	case UDP_ENCAP_GENEVE:
> +		ret = __skb_flow_dissect_geneve(skb, flow_dissector,
> +						target_container, data,
> +						p_proto, &nhoff, hlen, flags);
> +		break;
>  	default:
>  		break;
>  	}
> -- 
> 2.34.1
> 

Re: [PATCH 10/12] flow_dissector: Parse Geneve in UDP

[Willem de Bruijn]

Willem de Bruijn wrote:
> Tom Herbert wrote:
> > Parse Geneve in a UDP encapsulation
> > 
> > Signed-off-by: Tom Herbert <tom@herbertland.com>
> > ---
> >  net/core/flow_dissector.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> > index 3766dc4d5b23..4fff60233992 100644
> > --- a/net/core/flow_dissector.c
> > +++ b/net/core/flow_dissector.c
> > @@ -11,6 +11,7 @@
> >  #include <net/fou.h>
> >  #include <net/ip.h>
> >  #include <net/ipv6.h>
> > +#include <net/geneve.h>
> >  #include <net/gre.h>
> >  #include <net/pptp.h>
> >  #include <net/tipc.h>
> > @@ -808,6 +809,29 @@ __skb_flow_dissect_vxlan(const struct sk_buff *skb,
> >  	return FLOW_DISSECT_RET_PROTO_AGAIN;
> >  }
> >  
> > +static enum flow_dissect_ret
> > +__skb_flow_dissect_geneve(const struct sk_buff *skb,
> > +			  struct flow_dissector *flow_dissector,
> > +			  void *target_container, const void *data,
> > +			  __be16 *p_proto, int *p_nhoff, int hlen,
> > +			  unsigned int flags)
> > +{
> > +	struct genevehdr *hdr, _hdr;
> > +
> > +	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
> > +				   &_hdr);
> > +	if (!hdr)
> > +		return FLOW_DISSECT_RET_OUT_BAD;
> > +
> > +	if (hdr->ver != 0)
> > +		return FLOW_DISSECT_RET_OUT_GOOD;
> > +
> > +	*p_proto = hdr->proto_type;
> > +	*p_nhoff += sizeof(struct genevehdr) + (hdr->opt_len * 4);
> > +
> > +	return FLOW_DISSECT_RET_PROTO_AGAIN;
> 
> Do you want to return FLOW_DISSECT_RET_OUT_GOOD if IPPROTO 59.
> 
> Per your spec: "IP protocol number 59 ("No next header") may be set to
> indicate that the GUE payload does not begin with the header of an IP
> protocol."
> 
> Admittedly pendantic. No idea if any implementation actually sets
> this.

And reply to the wrong patch. I meant this for GUE.

Also not sure how useful the separate __skb_direct_ip_dissect is if
then have to catch the error special case in the caller.

Re: [PATCH 10/12] flow_dissector: Parse Geneve in UDP

[Tom Herbert]

On Sat, Aug 3, 2024 at 12:13 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Tom Herbert wrote:
> > Parse Geneve in a UDP encapsulation
> >
> > Signed-off-by: Tom Herbert <tom@herbertland.com>
> > ---
> >  net/core/flow_dissector.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >
> > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> > index 3766dc4d5b23..4fff60233992 100644
> > --- a/net/core/flow_dissector.c
> > +++ b/net/core/flow_dissector.c
> > @@ -11,6 +11,7 @@
> >  #include <net/fou.h>
> >  #include <net/ip.h>
> >  #include <net/ipv6.h>
> > +#include <net/geneve.h>
> >  #include <net/gre.h>
> >  #include <net/pptp.h>
> >  #include <net/tipc.h>
> > @@ -808,6 +809,29 @@ __skb_flow_dissect_vxlan(const struct sk_buff *skb,
> >       return FLOW_DISSECT_RET_PROTO_AGAIN;
> >  }
> >
> > +static enum flow_dissect_ret
> > +__skb_flow_dissect_geneve(const struct sk_buff *skb,
> > +                       struct flow_dissector *flow_dissector,
> > +                       void *target_container, const void *data,
> > +                       __be16 *p_proto, int *p_nhoff, int hlen,
> > +                       unsigned int flags)
> > +{
> > +     struct genevehdr *hdr, _hdr;
> > +
> > +     hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
> > +                                &_hdr);
> > +     if (!hdr)
> > +             return FLOW_DISSECT_RET_OUT_BAD;
> > +
> > +     if (hdr->ver != 0)
> > +             return FLOW_DISSECT_RET_OUT_GOOD;
> > +
> > +     *p_proto = hdr->proto_type;
> > +     *p_nhoff += sizeof(struct genevehdr) + (hdr->opt_len * 4);
> > +
> > +     return FLOW_DISSECT_RET_PROTO_AGAIN;
>
> Do you want to return FLOW_DISSECT_RET_OUT_GOOD if IPPROTO 59.
>
> Per your spec: "IP protocol number 59 ("No next header") may be set to
> indicate that the GUE payload does not begin with the header of an IP
> protocol."
>
> Admittedly pendantic. No idea if any implementation actually sets
> this.

It is a legal value in an IPv6 next header field. I'll add
IPPROTO_NONXTHDR and handling in flow dissector.

Tom

>
> > +}
> > +
> >  /**
> >   * __skb_flow_dissect_batadv() - dissect batman-adv header
> >   * @skb: sk_buff to with the batman-adv header
> > @@ -974,6 +998,11 @@ __skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
> >                                              target_container, data,
> >                                              p_proto, &nhoff, hlen, flags);
> >               break;
> > +     case UDP_ENCAP_GENEVE:
> > +             ret = __skb_flow_dissect_geneve(skb, flow_dissector,
> > +                                             target_container, data,
> > +                                             p_proto, &nhoff, hlen, flags);
> > +             break;
> >       default:
> >               break;
> >       }
> > --
> > 2.34.1
> >
>
>

[PATCH 11/12] flow_dissector: Parse GUE in UDP

[Tom Herbert]

Parse both version 0 and 1 of GUE encapsulated in UDP. Add helper
function __skb_direct_ip_dissect to convert an IP header to
IPPROTO_IPIP or IPPROTO_IPV6 (by looking just at the version
number)

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/core/flow_dissector.c | 60 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 4fff60233992..7f0bf737c3db 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -832,6 +832,61 @@ __skb_flow_dissect_geneve(const struct sk_buff *skb,
 	return FLOW_DISSECT_RET_PROTO_AGAIN;
 }
 
+static __u8
+__skb_direct_ip_dissect(void *hdr)
+{
+	/* Direct encapsulation of IPv4 or IPv6 */
+
+	switch (((struct iphdr *)hdr)->version) {
+	case 4:
+		return IPPROTO_IPIP;
+	case 6:
+		return IPPROTO_IPV6;
+	default:
+		return 0;
+	}
+}
+
+static enum flow_dissect_ret
+__skb_flow_dissect_gue(const struct sk_buff *skb,
+		       struct flow_dissector *flow_dissector,
+		       void *target_container, const void *data,
+		       __u8 *p_ip_proto, int *p_nhoff,
+		       int hlen, unsigned int flags)
+{
+	struct guehdr *hdr, _hdr;
+	__u8 proto;
+
+	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
+				   &_hdr);
+	if (!hdr)
+		return FLOW_DISSECT_RET_OUT_BAD;
+
+	switch (hdr->version) {
+	case 0:
+		if (unlikely(hdr->control))
+			return FLOW_DISSECT_RET_OUT_GOOD;
+
+		*p_nhoff += sizeof(struct guehdr) + (hdr->hlen << 2);
+		*p_ip_proto = hdr->proto_ctype;
+
+		break;
+	case 1:
+		/* Direct encapsulation of IPv4 or IPv6 */
+
+		proto = __skb_direct_ip_dissect(hdr);
+		if (proto) {
+			*p_ip_proto = proto;
+			break;
+		}
+		fallthrough;
+	default:
+		return FLOW_DISSECT_RET_OUT_GOOD;
+	}
+
+	return FLOW_DISSECT_RET_IPPROTO_AGAIN;
+}
+
 /**
  * __skb_flow_dissect_batadv() - dissect batman-adv header
  * @skb: sk_buff to with the batman-adv header
@@ -988,6 +1043,11 @@ __skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
 		*p_ip_proto = fou_protocol;
 		ret = FLOW_DISSECT_RET_IPPROTO_AGAIN;
 		break;
+	case UDP_ENCAP_GUE:
+		ret = __skb_flow_dissect_gue(skb, flow_dissector,
+					     target_container, data,
+					     p_ip_proto, p_nhoff, hlen, flags);
+		break;
 	case UDP_ENCAP_SCTP:
 		*p_ip_proto = IPPROTO_SCTP;
 		ret = FLOW_DISSECT_RET_IPPROTO_AGAIN;
-- 
2.34.1

[PATCH 12/12] flow_dissector: Parse gtp in UDP

[Tom Herbert]

Parse both version 0 and 1. Call __skb_direct_ip_dissect to determine
IP version of the encapsulated packet

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/core/flow_dissector.c | 87 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 7f0bf737c3db..af197ed560b8 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -9,6 +9,7 @@
 #include <net/dsa.h>
 #include <net/dst_metadata.h>
 #include <net/fou.h>
+#include <net/gtp.h>
 #include <net/ip.h>
 #include <net/ipv6.h>
 #include <net/geneve.h>
@@ -35,6 +36,7 @@
 #include <net/pkt_cls.h>
 #include <scsi/fc/fc_fcoe.h>
 #include <uapi/linux/batadv_packet.h>
+#include <uapi/linux/gtp.h>
 #include <linux/bpf.h>
 #if IS_ENABLED(CONFIG_NF_CONNTRACK)
 #include <net/netfilter/nf_conntrack_core.h>
@@ -887,6 +889,81 @@ __skb_flow_dissect_gue(const struct sk_buff *skb,
 	return FLOW_DISSECT_RET_IPPROTO_AGAIN;
 }
 
+static enum flow_dissect_ret
+__skb_flow_dissect_gtp0(const struct sk_buff *skb,
+			struct flow_dissector *flow_dissector,
+			void *target_container, const void *data,
+			__u8 *p_ip_proto, int *p_nhoff, int hlen,
+			unsigned int flags)
+{
+	__u8 *ip_version, _ip_version, proto;
+	struct gtp0_header *hdr, _hdr;
+
+	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
+				   &_hdr);
+	if (!hdr)
+		return FLOW_DISSECT_RET_OUT_BAD;
+
+	if ((hdr->flags >> 5) != GTP_V0)
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	ip_version = skb_header_pointer(skb, *p_nhoff + sizeof(_hdr),
+					sizeof(*ip_version),
+					&_ip_version);
+	if (!ip_version)
+		return FLOW_DISSECT_RET_OUT_BAD;
+
+	proto = __skb_direct_ip_dissect(ip_version);
+	if (!proto)
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	*p_ip_proto = proto;
+	*p_nhoff += sizeof(struct gtp0_header);
+
+	return FLOW_DISSECT_RET_IPPROTO_AGAIN;
+}
+
+static enum flow_dissect_ret
+__skb_flow_dissect_gtp1u(const struct sk_buff *skb,
+			 struct flow_dissector *flow_dissector,
+			 void *target_container, const void *data,
+			 __u8 *p_ip_proto, int *p_nhoff, int hlen,
+			 unsigned int flags)
+{
+	__u8 *ip_version, _ip_version, proto;
+	struct gtp1_header *hdr, _hdr;
+	int hdrlen = sizeof(_hdr);
+
+	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
+				   &_hdr);
+	if (!hdr)
+		return FLOW_DISSECT_RET_OUT_BAD;
+
+	if ((hdr->flags >> 5) != GTP_V1)
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	if (hdr->type != GTP_TPDU)
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	if (hdr->flags & GTP1_F_MASK)
+		hdrlen += 4;
+
+	ip_version = skb_header_pointer(skb, *p_nhoff + hdrlen,
+					sizeof(*ip_version),
+					&_ip_version);
+	if (!ip_version)
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	proto = __skb_direct_ip_dissect(ip_version);
+	if (!proto)
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	*p_ip_proto = proto;
+	*p_nhoff += hdrlen;
+
+	return FLOW_DISSECT_RET_IPPROTO_AGAIN;
+}
+
 /**
  * __skb_flow_dissect_batadv() - dissect batman-adv header
  * @skb: sk_buff to with the batman-adv header
@@ -1039,6 +1116,16 @@ __skb_flow_dissect_udp(const struct sk_buff *skb, struct net *net,
 		*p_ip_proto = IPPROTO_L2TP;
 		ret = FLOW_DISSECT_RET_IPPROTO_AGAIN;
 		break;
+	case UDP_ENCAP_GTP0:
+		ret = __skb_flow_dissect_gtp0(skb, flow_dissector,
+					      target_container, data,
+					      p_ip_proto, &nhoff, hlen, flags);
+		break;
+	case UDP_ENCAP_GTP1U:
+		ret = __skb_flow_dissect_gtp1u(skb, flow_dissector,
+					       target_container, data,
+					       p_ip_proto, &nhoff, hlen, flags);
+		break;
 	case UDP_ENCAP_FOU:
 		*p_ip_proto = fou_protocol;
 		ret = FLOW_DISSECT_RET_IPPROTO_AGAIN;
-- 
2.34.1

Re: [PATCH 12/12] flow_dissector: Parse gtp in UDP

[Willem de Bruijn]

Tom Herbert wrote:
> Parse both version 0 and 1. Call __skb_direct_ip_dissect to determine
> IP version of the encapsulated packet
> 
> Signed-off-by: Tom Herbert <tom@herbertland.com>
> ---
>  net/core/flow_dissector.c | 87 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
> 
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 7f0bf737c3db..af197ed560b8 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -9,6 +9,7 @@
>  #include <net/dsa.h>
>  #include <net/dst_metadata.h>
>  #include <net/fou.h>
> +#include <net/gtp.h>
>  #include <net/ip.h>
>  #include <net/ipv6.h>
>  #include <net/geneve.h>
> @@ -35,6 +36,7 @@
>  #include <net/pkt_cls.h>
>  #include <scsi/fc/fc_fcoe.h>
>  #include <uapi/linux/batadv_packet.h>
> +#include <uapi/linux/gtp.h>
>  #include <linux/bpf.h>
>  #if IS_ENABLED(CONFIG_NF_CONNTRACK)
>  #include <net/netfilter/nf_conntrack_core.h>
> @@ -887,6 +889,81 @@ __skb_flow_dissect_gue(const struct sk_buff *skb,
>  	return FLOW_DISSECT_RET_IPPROTO_AGAIN;
>  }
>  
> +static enum flow_dissect_ret
> +__skb_flow_dissect_gtp0(const struct sk_buff *skb,
> +			struct flow_dissector *flow_dissector,
> +			void *target_container, const void *data,
> +			__u8 *p_ip_proto, int *p_nhoff, int hlen,
> +			unsigned int flags)
> +{
> +	__u8 *ip_version, _ip_version, proto;
> +	struct gtp0_header *hdr, _hdr;
> +
> +	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
> +				   &_hdr);
> +	if (!hdr)
> +		return FLOW_DISSECT_RET_OUT_BAD;
> +
> +	if ((hdr->flags >> 5) != GTP_V0)
> +		return FLOW_DISSECT_RET_OUT_GOOD;
> +
> +	ip_version = skb_header_pointer(skb, *p_nhoff + sizeof(_hdr),
> +					sizeof(*ip_version),
> +					&_ip_version);
> +	if (!ip_version)
> +		return FLOW_DISSECT_RET_OUT_BAD;
> +
> +	proto = __skb_direct_ip_dissect(ip_version);
> +	if (!proto)
> +		return FLOW_DISSECT_RET_OUT_GOOD;
> +
> +	*p_ip_proto = proto;
> +	*p_nhoff += sizeof(struct gtp0_header);
> +
> +	return FLOW_DISSECT_RET_IPPROTO_AGAIN;
> +}
> +
> +static enum flow_dissect_ret
> +__skb_flow_dissect_gtp1u(const struct sk_buff *skb,
> +			 struct flow_dissector *flow_dissector,
> +			 void *target_container, const void *data,
> +			 __u8 *p_ip_proto, int *p_nhoff, int hlen,
> +			 unsigned int flags)
> +{
> +	__u8 *ip_version, _ip_version, proto;
> +	struct gtp1_header *hdr, _hdr;
> +	int hdrlen = sizeof(_hdr);
> +
> +	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
> +				   &_hdr);
> +	if (!hdr)
> +		return FLOW_DISSECT_RET_OUT_BAD;
> +
> +	if ((hdr->flags >> 5) != GTP_V1)
> +		return FLOW_DISSECT_RET_OUT_GOOD;
> +
> +	if (hdr->type != GTP_TPDU)
> +		return FLOW_DISSECT_RET_OUT_GOOD;
> +
> +	if (hdr->flags & GTP1_F_MASK)
> +		hdrlen += 4;
> +

The only difference with +__skb_flow_dissect_gtp0 is these two
branches. Can probably deduplicate.

Also, escape early if any of the other optional field bits are set?

  #define GTP1_F_NPDU     0x01
  #define GTP1_F_SEQ      0x02
  #define GTP1_F_EXTHDR   0x04

Re: [PATCH 00/12] flow_dissector: Dissect UDP encapsulation protocols

[Willem de Bruijn]

Tom Herbert wrote:
> Add support in flow_dissector for dissecting into UDP
> encapsulations like VXLAN. __skb_flow_dissect_udp is called for
> IPPROTO_UDP. The flag FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS enables parsing
> of UDP encapsulations. If the flag is set when parsing a UDP packet then
> a socket lookup is performed. The offset of the base network header,
> either an IPv4 or IPv6 header, is tracked and passed to
> __skb_flow_dissect_udp so that it can perform the socket lookup.
> If a socket is found and it's for a UDP encapsulation (encap_type is
> set in the UDP socket) then a switch is performed on the encap_type
> value (cases are UDP_ENCAP_* values)

The main concern with the flow dissector is that its execution depends
on untrusted packets.

For this reason we added the BPF dissector for new protocols. What is
the reason to prefer adding more C code?

And somewhat academic, but: would it be different if the BPF would
ship with the kernel and autoload at boot, just like C modules?

A second concern is changing the defaults. I have not looked at this
closely, but if dissection today stops at the outer UDP header for
skb_get_hash, then we don't want to accidentally change this behavior.
Or if not accidental, call it out explicitly.

> 
> Tested: Verified fou, gue, vxlan, and geneve are properly dissected for
> IPv4 and IPv6 cases. This includes testing ETH_P_TEB case

Manually?

Re: [PATCH 00/12] flow_dissector: Dissect UDP encapsulation protocols

[Tom Herbert]

On Thu, Aug 1, 2024 at 6:20 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Tom Herbert wrote:
> > Add support in flow_dissector for dissecting into UDP
> > encapsulations like VXLAN. __skb_flow_dissect_udp is called for
> > IPPROTO_UDP. The flag FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS enables parsing
> > of UDP encapsulations. If the flag is set when parsing a UDP packet then
> > a socket lookup is performed. The offset of the base network header,
> > either an IPv4 or IPv6 header, is tracked and passed to
> > __skb_flow_dissect_udp so that it can perform the socket lookup.
> > If a socket is found and it's for a UDP encapsulation (encap_type is
> > set in the UDP socket) then a switch is performed on the encap_type
> > value (cases are UDP_ENCAP_* values)
>
> The main concern with the flow dissector is that its execution depends
> on untrusted packets.
>
> For this reason we added the BPF dissector for new protocols. What is
> the reason to prefer adding more C code?
>
> And somewhat academic, but: would it be different if the BPF would
> ship with the kernel and autoload at boot, just like C modules?

Hi Willem,

I agree with that, and believe the ultimate goal is to replace flow
dissector C code with eBPF which I still intend to work on that, but
right now I'm hoping to get support as part of obsoleting protocol
specific checksum offload on receive. We can use flow dissector to
identify the checksum in a packet marked checksum-unnecessary by a
legacy device for doing conversion to checksum-complete. This handles
the case where the device reports a valid L4 checksum in a UDP
encapsulation and the outer UDP checksum is zero.

>
> A second concern is changing the defaults. I have not looked at this
> closely, but if dissection today stops at the outer UDP header for
> skb_get_hash, then we don't want to accidentally change this behavior.
> Or if not accidental, call it out explicitly.

No defaults are being changed. Flow dissector flag
FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS needs to be set in the call to flow
dissector. In this patch set it's not being used, but as I mentioned
it will be used in subsequent patch sets for obsoleting
CHECKSUM_UNNECESSARY.

For other use cases, the flag can be optionally set. TC-flower for
instance could use this for VXLAN and Geneve parsing.

>
> >
> > Tested: Verified fou, gue, vxlan, and geneve are properly dissected for
> > IPv4 and IPv6 cases. This includes testing ETH_P_TEB case
>
> Manually?

Yes for the time being.

Tom

Re: [PATCH 00/12] flow_dissector: Dissect UDP encapsulation protocols

[Tom Herbert]

On Wed, Aug 14, 2024 at 1:28 PM Tom Herbert <tom@herbertland.com> wrote:
>
> On Thu, Aug 1, 2024 at 6:20 AM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > Tom Herbert wrote:
> > > Add support in flow_dissector for dissecting into UDP
> > > encapsulations like VXLAN. __skb_flow_dissect_udp is called for
> > > IPPROTO_UDP. The flag FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS enables parsing
> > > of UDP encapsulations. If the flag is set when parsing a UDP packet then
> > > a socket lookup is performed. The offset of the base network header,
> > > either an IPv4 or IPv6 header, is tracked and passed to
> > > __skb_flow_dissect_udp so that it can perform the socket lookup.
> > > If a socket is found and it's for a UDP encapsulation (encap_type is
> > > set in the UDP socket) then a switch is performed on the encap_type
> > > value (cases are UDP_ENCAP_* values)
> >
> > The main concern with the flow dissector is that its execution depends
> > on untrusted packets.
> >
> > For this reason we added the BPF dissector for new protocols. What is
> > the reason to prefer adding more C code?
> >
> > And somewhat academic, but: would it be different if the BPF would
> > ship with the kernel and autoload at boot, just like C modules?
>
> Hi Willem,
>
> I agree with that, and believe the ultimate goal is to replace flow
> dissector C code with eBPF which I still intend to work on that, but
> right now I'm hoping to get support as part of obsoleting protocol
> specific checksum offload on receive. We can use flow dissector to
> identify the checksum in a packet marked checksum-unnecessary by a
> legacy device for doing conversion to checksum-complete. This handles
> the case where the device reports a valid L4 checksum in a UDP
> encapsulation and the outer UDP checksum is zero.

Also, there's another wrinkle with doing this in eBPF. UDP
encapsulations are identified by port number, not a protocol number,
so we can't hardcode port numbers into eBPF like we can other protocol
constants. We'd probably need to hook into setup_udp_tunnel_sock
somehow.

Tom

>
> >
> > A second concern is changing the defaults. I have not looked at this
> > closely, but if dissection today stops at the outer UDP header for
> > skb_get_hash, then we don't want to accidentally change this behavior.
> > Or if not accidental, call it out explicitly.
>
> No defaults are being changed. Flow dissector flag
> FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS needs to be set in the call to flow
> dissector. In this patch set it's not being used, but as I mentioned
> it will be used in subsequent patch sets for obsoleting
> CHECKSUM_UNNECESSARY.
>
> For other use cases, the flag can be optionally set. TC-flower for
> instance could use this for VXLAN and Geneve parsing.
>
> >
> > >
> > > Tested: Verified fou, gue, vxlan, and geneve are properly dissected for
> > > IPv4 and IPv6 cases. This includes testing ETH_P_TEB case
> >
> > Manually?
>
> Yes for the time being.
>
> Tom

Re: [PATCH 00/12] flow_dissector: Dissect UDP encapsulation protocols

[Jakub Kicinski]

On Wed, 31 Jul 2024 10:23:20 -0700 Tom Herbert wrote:
> Add support in flow_dissector for dissecting into UDP
> encapsulations like VXLAN. __skb_flow_dissect_udp is called for
> IPPROTO_UDP. The flag FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS enables parsing
> of UDP encapsulations. If the flag is set when parsing a UDP packet then
> a socket lookup is performed. The offset of the base network header,
> either an IPv4 or IPv6 header, is tracked and passed to
> __skb_flow_dissect_udp so that it can perform the socket lookup.
> If a socket is found and it's for a UDP encapsulation (encap_type is
> set in the UDP socket) then a switch is performed on the encap_type
> value (cases are UDP_ENCAP_* values)

Appears to break build for allmodconfig
-- 
pw-bot: cr