NACK/Cmnt: [SRU][N/J][PATCH 0/2] UBUNTU: SAUCE: fan: fixes

[Aleksandr Mikhalitsyn]

On Mon, Sep 9, 2024 at 12:08 PM Stefan Bader <stefan.bader at canonical.com> wrote:
>
> On 06.09.24 23:11, Alexander Mikhalitsyn wrote:
> > BugLink: https://bugs.launchpad.net/bugs/2064176
> >
> > SRU Justification:
> >
> > [Impact]
> >
> > User can trigger a host crash on Jammy/Noble by launching
> > a container which uses Ubuntu FAN network in LXD.
> >
> > [Fix]
> >
> > A first proposed patch fixes RCU locking by releasing rcu_read_lock
> > on the skb discard codepath.
> >
> > Second patch just use a proper way (dev_core_stats_tx_dropped_inc() function)
> > to increase netdev's tx_dropped statistic value.
> >
> > [Test Plan]
> >
> > As provided by Max Asnaashari:
> >
> > # Install LXD from channel latest/stable
> > snap install lxd --channel latest/stable
> >
> > # Configure LXD
> > lxd init --auto
> >
> > # Create a FAN network
> > lxc network create lxdfan0 bridge.mode=fan ipv4.nat=true
> >
> > # Launch a container using the FAN network
> > lxc launch ubuntu-minimal:22.04 c1 --network lxdfan0
> >
> > # Try to interact with LXD
> > lxc ls
> >
> > [Where problems could occur]
> >
> > Change is local and only related to Ubuntu FAN code. I would not expect
> > any problems with this patchset.
> >
> > Link: https://github.com/canonical/lxd/issues/14025
> > Reported-by: Max Asnaashari <max.asnaashari at canonical.com>
> > Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn at canonical.com>
> >
> > Alexander Mikhalitsyn (2):
> >    UBUNTU: SAUCE: fan: release rcu_read_lock on skb discard path
> >    UBUNTU: SAUCE: fan: fix racy device stat update
> >
> >   drivers/net/vxlan/vxlan_core.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >

Dear Stefan,

>
> Each patch should have the related BugLink in it. In principle each
> submission addressing one issue but with a generic title in the bug
> report it might be ok to both fixes at the same time. Just keep in mind
> that there will be requests to verify fixes and those are per bug
> report. For that reason the SRU justification has to be in the bug
> report as well. I would insert it at the top of the description.
> Last, each commit message in the patches should be more that title and
> signing off. It does not have to be novels but enough to explain the
> issue and allow to look at a patch and have a guess what the issue it
> addresses is and whether the fix feels sane.

I have just submitted -v2. Hope now it's better
https://lists.ubuntu.com/archives/kernel-team/2024-September/153551.html

Kind regards,
Alex

>
> -Stefan

> --
> - Stefan
>