Received-SPF: pass (google.com: domain of linux-kernel+bounces-184755-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Eelco Chaudron <echaudro@redhat.com>
To: Aaron Conole <aconole@redhat.com>
Cc: netdev@vger.kernel.org, dev@openvswitch.org, linux-kernel@vger.kernel.org,
 Pravin B Shelar <pshelar@ovn.org>, "David S. Miller" <davem@davemloft.net>,
 Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>,
 Paolo Abeni <pabeni@redhat.com>, Jesse Gross <jesse@nicira.com>,
 Ilya Maximets <i.maximets@ovn.org>, Simon Horman <horms@ovn.org>,
 Jaime Caamano <jcaamano@redhat.com>
Subject: Re: [PATCH v2 net] openvswitch: Set the skbuff pkt_type for proper
 pmtud support.
Date: Tue, 21 May 2024 12:09:39 +0200
Message-ID: <701FCF52-7D25-4094-9B0E-8F7AE8A68107@redhat.com>
In-Reply-To: <20240516200941.16152-1-aconole@redhat.com>
References: <20240516200941.16152-1-aconole@redhat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain


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@redhat.com>
> Reported-at: https://issues.redhat.com/browse/FDP-164
> Fixes: 58264848a5a7 ("openvswitch: Add vxlan tunneling support.")
> Signed-off-by: Aaron Conole <aconole@redhat.com>

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

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