Hello,
On Fri, 21 Feb 2025, Philo Lu wrote:
> We found an issue when using bpf_redirect with ipvs NAT mode after
> commit ff70202b2d1a ("dev_forward_skb: do not scrub skb mark within
> the same name space"). Particularly, we use bpf_redirect to return
> the skb directly back to the netif it comes from, i.e., xnet is
> false in skb_scrub_packet(), and then ipvs_property is preserved
> and SNAT is skipped in the rx path.
>
> ipvs_property has been already cleared when netns is changed in
> commit 2b5ec1a5f973 ("netfilter/ipvs: clear ipvs_property flag when
> SKB net namespace changed"). This patch just clears it in spite of
> netns.
>
> Signed-off-by: Philo Lu <lulie@xxxxxxxxxxxxxxxxx>
> ---
> This is in fact a fix patch, and the issue was found after commit
> ff70202b2d1a ("dev_forward_skb: do not scrub skb mark within
> the same name space"). But I'm not sure if a "Fixes" tag should be
> added to that commit.
You can add 2b5ec1a5f973 as a Fixes tag in v2 and I'll ack it.
Nowadays, ipvs_property prevents unneeded connection
lookups while the packet traverses the hooks (different IPVS
hook handlers can see the same packet in same or next hook).
This includes the case where packet can be routed to local server,
eg. via loopback_xmit(), for example:
Packet from device:
LOCAL_IN: set ipvs_property=1
Reroute to local server ?
Then to hit the local server continue the NF hook
Otherwise, route via device:
LOCAL_OUT: use ipvs_property to avoid conn lookup
and rerouting
LOCAL_OUT:
Unexpected divert to local server by someone else?
=> LOCAL_IN: use ipvs_property to avoid conn
lookup and rerouting
Packet from local client:
LOCAL_OUT: set ipvs_property=1
Reroute to local server?
Then continue via loopback_xmit() without
skb_scrub_packet()
LOCAL_IN: use ipvs_property to avoid conn lookup and
rerouting
Hit the local server
Otherwise, route via device:
Others at LOCAL_OUT: Unexpected divert to local server by
someone else ?
=> LOCAL_IN: use ipvs_property to avoid conn
lookup and rerouting
So, we need ipvs_property set only during the normal
path where packet is received and sent, possibly to local
server via local route. We do not care if the packet leaves
this path. Currently, ipvs_property was cleared if netns
changes but it is a safe option. What matters is if packet
is routed via local route. Other paths can reset the flag
safely.
> ---
> net/core/skbuff.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 7b03b64fdcb2..b1c81687e9d8 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -6033,11 +6033,11 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet)
> skb->offload_fwd_mark = 0;
> skb->offload_l3_fwd_mark = 0;
> #endif
> + ipvs_reset(skb);
>
> if (!xnet)
> return;
>
> - ipvs_reset(skb);
> skb->mark = 0;
> skb_clear_tstamp(skb);
> }
> --
> 2.32.0.3.g01195cf9f
Regards
--
Julian Anastasov <ja@xxxxxx>
