Re: [PATCH net] macvlan: cap IFLA_MACVLAN_BC_QUEUE_LEN to bound broadcast backlog

[Xiang Mei]

On Tue, Jul 28, 2026 at 3:25 PM Thomas Karlsson
<thomas.karlsson@xxxxxxxxx> wrote:
>
> Hello Xiang,
>
> Fixing the permission loophole in favor of adding an arbitrary upper limit seems resonable to me and I can support that approach.
>

Thanks for your review of the proposal!

> I'm not quite sure of which approach between your suggestion and the patch proposed by Doruk Tan Ozturk would be the best. Or, if both are independently needed. I'll leave that to others to weigh in on.
>
Both are needed, though they do overlap.

My check sits in macvlan_validate(), which runs before changelink, and a
caller in a child user namespace never satisfies capable(). So it does
refuse the queue-length path Doruk describes.

It does not cover the rest of his patch. BC_CUTOFF is not a max like the
queue length: update_port_bc_cutoff() assigns port->bc_cutoff directly, so
any macvlan sets it for every other macvlan on the lower device. It decides
which multicast addresses get deferred to the broadcast queue rather than
handled inline, and a negative value switches the queue off for the whole
port. A container on your lower device could push your streams onto the
inline path, which is what the queue was added to avoid. No large value and
no flood needed. His nested-creation check is likewise not something I
touch.

His does not cover mine either. rtnl_dev_link_net_capable() returns on
net_eq() before reaching ns_capable(), and my reproducer creates the veth
and the macvlan in one namespace it owns, so no boundary is crossed. The
value is simply unbounded, and the memory it pins is charged to no
namespace. (Prove: my poc can still the kernel with his patch.)

A missing authority check and a missing resource limit, overlapping on one
attribute, neither sufficient alone.

I agree byte accounting is the better shape, and your point that a byte
limit would need the permission fix anyway is why I am sending the check
first and leaving bytes for net-next.

v2 has been posted:

https://lore.kernel.org/netdev/20260729200621.2521588-1-xmei5@xxxxxxx/T/#u

Thanks,
Xiang







>
> Your simulations do reveal that byte based accounting probably would have been a better approach to begin with. Such a solution would possibly have covered more use-cases by default too, as high packet rate is often associated with small packet sizes.
>
> But that could be considered separately after this loophole has been patched. Altering a byte based limit would benefit from the same permission fix anyway as I see it.
>
> Best regards,
> Thomas
>
> ________________________________________
> From: Xiang Mei <xmei5@xxxxxxx>
> Sent: Sunday, 26 July 2026 03:20
> To: Thomas Karlsson <thomas.karlsson@xxxxxxxxx>
> Cc: Andrew Lunn <andrew+netdev@xxxxxxx>; David S . Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni <pabeni@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx <netdev@xxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; AutonomousCodeSecurity@xxxxxxxxxxxxx <AutonomousCodeSecurity@xxxxxxxxxxxxx>; tgopinath@xxxxxxxxxxxxxxxxxxx <tgopinath@xxxxxxxxxxxxxxxxxxx>; kys@xxxxxxxxxxxxx <kys@xxxxxxxxxxxxx>
> Subject: Re: [PATCH net] macvlan: cap IFLA_MACVLAN_BC_QUEUE_LEN to bound broadcast backlog
>
> On Mon, Jul 6, 2026 at 4:25 PM Thomas Karlsson
> <thomas.karlsson@xxxxxxxxx> wrote:
> >
> > Hello Xiang,
> >
> > Let me elaborate on our use-case with a little example calculation. Lets say medium latency real-time audio sent at 1ms per packet. This would amount to 1000 packets per second per stream. Scaling that to 600-800 unique audio streams would result in 6-800k pps. This would be a typical use case for our servers. The macvlan BC queue len need adjustment to be able to swallow short scheduling/processing delays in these workloads or packet loss start to show.
> >
> > Switching to even lower latency (although not recommended) or increasing the number of unique streams could of course increase the load even further.
> >
> > As for a recommended upper limit, I'm not sure. Like I said, we use 100k today and are currently running our workload extremely stable with that. Do we have some margin in that number? Probably, but I'm not able to quantify exactly how much.
> >
> > To account for someone using it even more aggressively, 128 or 256k could perhaps be a resonable upper limit?
> >
> > Best regards,
> > Thomas
> >
>
> Thanks Thomas, I tested those. They suggest adding a limit is limited,
> I would like to discuss an alternative before sending a v2.
>
> The memory the backlog pins depends on three things the caller picks:
> the slot count, the frame size (the queue counts skbs, not bytes, and a
> self-created veth takes ETH_MAX_MTU), and the number of lower devices
> (the limit is per port, and nothing counts ports). They multiply. On a
> 3.5G guest, from an unprivileged user+net namespace:
>
>   131072 slots, 9000-byte frames             -> survived, 104 MB free
>   262144 slots, 9000-byte frames             -> OOM, host dead
>     4096 slots, 65000-byte frames, 64 ports  -> MemAvailable 0
>
> No value is both safe on a small host and large enough for real
> multicast workloads: the two are ~25x apart, and the safe line depends
> on the victim's RAM.
>
> So I am considering a permission check instead. What separates the two
> cases is the caller: a production server is configured by real root.
>
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -1335,6 +1335,21 @@ static int macvlan_validate(struct nlattr
> *tb[], struct nlattr *data[],
>   if (!data)
>   return 0;
>
> + /* bc_queue_len_used is the max of every bc_queue_len_req on the port
> + * and the only bound on port->bc_queue.  rtnetlink checks CAP_NET_ADMIN
> + * against the target netns only, which a user namespace owner holds
> + * over a lower device it created itself, so require privilege in the
> + * initial user namespace to exceed the default.
> + */
> + if (data[IFLA_MACVLAN_BC_QUEUE_LEN] &&
> +    nla_get_u32(data[IFLA_MACVLAN_BC_QUEUE_LEN]) >
> + MACVLAN_DEFAULT_BC_QUEUE_LEN &&
> +    !capable(CAP_NET_ADMIN)) {
> + NL_SET_ERR_MSG_ATTR(extack, data[IFLA_MACVLAN_BC_QUEUE_LEN],
> +    "bc_queue_len above the default requires CAP_NET_ADMIN in the
> initial user namespace");
> + return -EPERM;
> + }
> +
>   if (data[IFLA_MACVLAN_FLAGS] &&
>      nla_get_u16(data[IFLA_MACVLAN_FLAGS]) & ~(MACVLAN_FLAG_NOPROMISC |
>        MACVLAN_FLAG_NODST))
>
> ->validate() runs for both RTM_NEWLINK and changelink before any state
> changes, so one check covers both. With it, 0xffffffff and 100000 are
> refused from a namespace while real root still gets 100000.
>
> It does not fix everything: 1000 skbs of ETH_MAX_MTU is ~65 MB per port,
> and nothing counts ports. At the untouched default I still got a 1500 MB
> workload OOM-killed given enough ports and large frames, identically on
> patched and unpatched kernels since the check is inert there. Bounding
> the backlog by bytes rather than skb count would cover that, and would
> let small-frame workloads queue more rather than less; that looks like
> net-next material to me.
>
> Separate from what Doruk Tan Ozturk reported: his check is against the
> lower device's netns and does not cover this case, where the caller does
> administer that device. Both seem wanted.
>
> Does this look like a reasonable direction, or should I go straight to
> byte-based accounting? I am also unsure whether gating a link attribute
> on init-userns privilege is acceptable here.
>
> Thanks,
> Xiang
> > ________________________________________
> > From: Xiang Mei <xmei5@xxxxxxx>
> > Sent: Tuesday, 7 July 2026 00:34
> > To: Thomas Karlsson <thomas.karlsson@xxxxxxxxx>
> > Cc: Andrew Lunn <andrew+netdev@xxxxxxx>; David S . Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni <pabeni@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx <netdev@xxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; AutonomousCodeSecurity@xxxxxxxxxxxxx <AutonomousCodeSecurity@xxxxxxxxxxxxx>; tgopinath@xxxxxxxxxxxxxxxxxxx <tgopinath@xxxxxxxxxxxxxxxxxxx>; kys@xxxxxxxxxxxxx <kys@xxxxxxxxxxxxx>
> > Subject: Re: [PATCH net] macvlan: cap IFLA_MACVLAN_BC_QUEUE_LEN to bound broadcast backlog
> >
> > On Mon, Jul 6, 2026 at 3:31 PM Thomas Karlsson
> > <thomas.karlsson@xxxxxxxxx> wrote:
> > >
> > > Dear all,
> > >
> > > While restricting a lower max than MAX U32 may be warranted the proposed limit of 4096 is way too small for high throughput multicast traffic.
> > >
> > > We currently run many production servers with the BC queue len set to a value of 100 000.
> > >
> >
> > Thanks, Thomas, for your feedback. 4096 could be small.
> > We would like to hear your suggested values, and we would like to do
> > some tests with the value you suggest.
> >
> > Xiang
> >
> >
> > >
> > > Best regards,
> > > Thomas Karlsson
> > >
> > >
> > > Sent from Outlook for Android
> > > ________________________________
> > > From: Xiang Mei (Microsoft) <xmei5@xxxxxxx>
> > > Sent: Monday, 06 July 2026 23:25:56
> > > To: Andrew Lunn <andrew+netdev@xxxxxxx>; David S . Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni <pabeni@xxxxxxxxxx>
> > > Cc: Thomas Karlsson <thomas.karlsson@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx <netdev@xxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; AutonomousCodeSecurity@xxxxxxxxxxxxx <AutonomousCodeSecurity@xxxxxxxxxxxxx>; tgopinath@xxxxxxxxxxxxxxxxxxx <tgopinath@xxxxxxxxxxxxxxxxxxx>; kys@xxxxxxxxxxxxx <kys@xxxxxxxxxxxxx>; Xiang Mei (Microsoft) <xmei5@xxxxxxx>
> > > Subject: [PATCH net] macvlan: cap IFLA_MACVLAN_BC_QUEUE_LEN to bound broadcast backlog
> > >
> > > The netlink policy for IFLA_MACVLAN_BC_QUEUE_LEN accepts any u32, and the
> > > value becomes port->bc_queue_len_used, the only cap on how many skbs
> > > macvlan_broadcast_enqueue() may queue on port->bc_queue. An unprivileged
> > > user owning a user+net namespace (CAP_NET_ADMIN is checked only against
> > > the target netns) can set it to 0xffffffff, so the backlog check never
> > > trips and every broadcast frame is skb_clone()'d with GFP_ATOMIC and
> > > queued. When RX softirq outpaces the bc_work drain (e.g. many ALLMULTI
> > > macvlans under a broadcast flood), the queue grows unbounded and
> > > OOMs/panics the host.
> > >
> > > Bound the value with NLA_POLICY_MAX() at 4096, 4x the default of 1000.
> > > This keeps headroom above the default while capping the backlog; values
> > > above the cap are now rejected with -ERANGE at netlink parse time.
> > >
> > >   Out of memory: Killed process 141 (su) UID:0
> > >   Kernel panic - not syncing: System is deadlocked on memory
> > >   Call Trace:
> > >    vpanic (kernel/panic.c:650)
> > >    panic (kernel/panic.c:787)
> > >    out_of_memory (mm/oom_kill.c:1166)
> > >    __alloc_frozen_pages_noprof (mm/page_alloc.c:4914)
> > >    alloc_pages_mpol (mm/mempolicy.c:2490)
> > >    folio_alloc_noprof (mm/mempolicy.c:2591)
> > >    filemap_fault (mm/filemap.c:3565)
> > >
> > > Fixes: d4bff72c8401 ("macvlan: Support for high multicast packet rate")
> > > Reported-by: AutonomousCodeSecurity@xxxxxxxxxxxxx
> > > Signed-off-by: Xiang Mei (Microsoft) <xmei5@xxxxxxx>
> > > ---
> > >  drivers/net/macvlan.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> > > index c40fa331836b..d4cede6393b0 100644
> > > --- a/drivers/net/macvlan.c
> > > +++ b/drivers/net/macvlan.c
> > > @@ -37,6 +37,7 @@
> > >  #define MACVLAN_HASH_BITS       8
> > >  #define MACVLAN_HASH_SIZE       (1<<MACVLAN_HASH_BITS)
> > >  #define MACVLAN_DEFAULT_BC_QUEUE_LEN    1000
> > > +#define MACVLAN_MAX_BC_QUEUE_LEN       4096
> > >
> > >  #define MACVLAN_F_PASSTHRU      1
> > >  #define MACVLAN_F_ADDRCHANGE    2
> > > @@ -1755,7 +1756,7 @@ static const struct nla_policy macvlan_policy[IFLA_MACVLAN_MAX + 1] = {
> > >          [IFLA_MACVLAN_MACADDR] = { .type = NLA_BINARY, .len = MAX_ADDR_LEN },
> > >          [IFLA_MACVLAN_MACADDR_DATA] = { .type = NLA_NESTED },
> > >          [IFLA_MACVLAN_MACADDR_COUNT] = { .type = NLA_U32 },
> > > -       [IFLA_MACVLAN_BC_QUEUE_LEN] = { .type = NLA_U32 },
> > > +       [IFLA_MACVLAN_BC_QUEUE_LEN] = NLA_POLICY_MAX(NLA_U32, MACVLAN_MAX_BC_QUEUE_LEN),
> > >          [IFLA_MACVLAN_BC_QUEUE_LEN_USED] = { .type = NLA_REJECT },
> > >          [IFLA_MACVLAN_BC_CUTOFF] = { .type = NLA_S32 },
> > >  };
> > > --
> > > 2.43.0
> > >