| Message ID | 20240731172332.683815-11-tom@herbertland.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | flow_dissector: Dissect UDP encapsulation protocols | expand |
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 >
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.
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 > > > >
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; }
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(+)