| Message ID | 20240731172332.683815-1-tom@herbertland.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | flow_dissector: Dissect UDP encapsulation protocols | expand |
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?
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
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
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