Re: [PATCH net] net: vlan: fix VLAN 0 refcount imbalance of toggling filtering during runtime

[Ido Schimmel <idosch@idosch.org>]

On Mon, Jun 30, 2025 at 09:25:42AM +0800, dongchenchen (A) wrote:
> 
> > On Thu, Jun 26, 2025 at 11:41:45AM +0800, dongchenchen (A) wrote:
> > As I understand it, there are two issues:
> > 
> > 1. VID 0 is deleted when it shouldn't. This leads to the crashes you
> > shared.
> > 
> > 2. VID 0 is not deleted when it should. This leads to memory leaks like
> > [1] with a reproducer such as [2].
> > 
> > AFAICT, your proposed patch solves the second issue, but only partially
> > addresses the first issue. The automatic addition of VID 0 is assumed to
> > be successful, but it can fail due to hardware issues or memory
> > allocation failures. I realize it is not common, but it can happen. If
> > you annotate __vlan_vid_add() [3] and inject failures [4], you will see
> > that the crashes still happen with your patch.
> 
> Hi, Ido
> Thanks for your review!
> 
> > WDYT about something like [5]? Basically, noting in the VLAN info
> 
> This fix [5] can completely solve the problem. I will send it together with
> selftest patch.

Thanks. Please add tests for both cases (memory leak and crash).

> 
> > whether VID 0 was automatically added upon NETDEV_UP and based on that
> > decide whether it should be deleted upon NETDEV_DOWN, regardless of
> > "rx-vlan-filter".
> 
> one small additional question: vlan0 will not exist if netdev set rx-vlan-filter after NETDEV_UP.
> Will this cause a difference in the processing logic for 8021q packets?

AFAICT the proposed patch does not change this behavior. Users can bring
the netdev down and then up if they want the kernel to add VID 0. My
understanding is that "rx-vlan-filter on" without VID 0 will cause
prio-tagged packets to be dropped by the underlying device.