netdev - Re: [PATCH bpf 3/5] netkit: Fix syncing peer device mtu with primary

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: <984f7580-890d-4644-b8ad-144505a882e4@blackwall.org>
Date: Fri, 24 May 2024 17:15:14 +0300
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: Daniel Borkmann <daniel@...earbox.net>, martin.lau@...nel.org
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, Joe Stringer <joe@...ium.io>
Subject: Re: [PATCH bpf 3/5] netkit: Fix syncing peer device mtu with primary

On 5/24/24 16:01, Daniel Borkmann wrote:
> Implement the ndo_change_mtu callback in netkit in order to align the MTU
> to the primary device. This is needed in order to sync MTUs to the latter
> from the control plane (e.g. Cilium) which does not have access into the
> Pod's netns.
> 
> Fixes: 35dfaad7188c ("netkit, bpf: Add bpf programmable net device")
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
> Cc: Joe Stringer <joe@...ium.io>
> ---
>  drivers/net/netkit.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 

This one has unexpected behaviour IMO. If the app sets the MTU and we
silently overwrite, then it may continue working and thinking the MTU
was changed leading to unexpected problems. I think it'd be better to
keep the MTU synced explicitly (e.g. when set on main device, then
set it on peer as well) and error out when trying to set it without
the proper capabilities.