netdev - Re: [PATCH v2 net] openvswitch: Set the skbuff pkt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <701FCF52-7D25-4094-9B0E-8F7AE8A68107@redhat.com>
Date: Tue, 21 May 2024 12:09:39 +0200
From: Eelco Chaudron <echaudro@...hat.com>
To: Aaron Conole <aconole@...hat.com>
Cc: netdev@...r.kernel.org, dev@...nvswitch.org, linux-kernel@...r.kernel.org,
 Pravin B Shelar <pshelar@....org>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Jesse Gross <jesse@...ira.com>,
 Ilya Maximets <i.maximets@....org>, Simon Horman <horms@....org>,
 Jaime Caamano <jcaamano@...hat.com>
Subject: Re: [PATCH v2 net] openvswitch: Set the skbuff pkt_type for proper
 pmtud support.



On 16 May 2024, at 22:09, Aaron Conole wrote:

> Open vSwitch is originally intended to switch at layer 2, only dealing with
> Ethernet frames.  With the introduction of l3 tunnels support, it crossed
> into the realm of needing to care a bit about some routing details when
> making forwarding decisions.  If an oversized packet would need to be
> fragmented during this forwarding decision, there is a chance for pmtu
> to get involved and generate a routing exception.  This is gated by the
> skbuff->pkt_type field.
>
> When a flow is already loaded into the openvswitch module this field is
> set up and transitioned properly as a packet moves from one port to
> another.  In the case that a packet execute is invoked after a flow is
> newly installed this field is not properly initialized.  This causes the
> pmtud mechanism to omit sending the required exception messages across
> the tunnel boundary and a second attempt needs to be made to make sure
> that the routing exception is properly setup.  To fix this, we set the
> outgoing packet's pkt_type to PACKET_OUTGOING, since it can only get
> to the openvswitch module via a port device or packet command.
>
> Even for bridge ports as users, the pkt_type needs to be reset when
> doing the transmit as the packet is truly outgoing and routing needs
> to get involved post packet transformations, in the case of
> VXLAN/GENEVE/udp-tunnel packets.  In general, the pkt_type on output
> gets ignored, since we go straight to the driver, but in the case of
> tunnel ports they go through IP routing layer.
>
> This issue is periodically encountered in complex setups, such as large
> openshift deployments, where multiple sets of tunnel traversal occurs.
> A way to recreate this is with the ovn-heater project that can setup
> a networking environment which mimics such large deployments.  We need
> larger environments for this because we need to ensure that flow
> misses occur.  In these environment, without this patch, we can see:
>
>   ./ovn_cluster.sh start
>   podman exec ovn-chassis-1 ip r a 170.168.0.5/32 dev eth1 mtu 1200
>   podman exec ovn-chassis-1 ip netns exec sw01p1 ip r flush cache
>   podman exec ovn-chassis-1 ip netns exec sw01p1 \
>          ping 21.0.0.3 -M do -s 1300 -c2
>   PING 21.0.0.3 (21.0.0.3) 1300(1328) bytes of data.
>   From 21.0.0.3 icmp_seq=2 Frag needed and DF set (mtu = 1142)
>
>   --- 21.0.0.3 ping statistics ---
>   ...
>
> Using tcpdump, we can also see the expected ICMP FRAG_NEEDED message is not
> sent into the server.
>
> With this patch, setting the pkt_type, we see the following:
>
>   podman exec ovn-chassis-1 ip netns exec sw01p1 \
>          ping 21.0.0.3 -M do -s 1300 -c2
>   PING 21.0.0.3 (21.0.0.3) 1300(1328) bytes of data.
>   From 21.0.0.3 icmp_seq=1 Frag needed and DF set (mtu = 1222)
>   ping: local error: message too long, mtu=1222
>
>   --- 21.0.0.3 ping statistics ---
>   ...
>
> In this case, the first ping request receives the FRAG_NEEDED message and
> a local routing exception is created.
>
> Tested-by: Jaime Caamano <jcaamano@...hat.com>
> Reported-at: https://issues.redhat.com/browse/FDP-164
> Fixes: 58264848a5a7 ("openvswitch: Add vxlan tunneling support.")
> Signed-off-by: Aaron Conole <aconole@...hat.com>

Thanks for the additional comments and detailed commit message. The change looks good to me.

Acked-by: Eelco Chaudron <echaudro@...hat.com>