Re: [PATCH net] xfrm: always flush state and policy upon NETDEV_DOWN/NETDEV_UNREGISTER events

[Sabrina Dubroca <sd@queasysnail.net>]

2026-01-29, 17:06:08 +0900, Tetsuo Handa wrote:
> On 2026/01/28 21:35, Leon Romanovsky wrote:
> > On Wed, Jan 28, 2026 at 07:44:02PM +0900, Tetsuo Handa wrote:
> >> On 2026/01/28 19:24, Leon Romanovsky wrote:
> >>> I think this can work, but IMHO the more robust approach is to ensure that all
> >>> states and policies are removed when the NETIF_F_HW_ESP feature bit is cleared.
> >>
> >> The transaction will become complicated, for dev->features manipulation
> >> function can fail.
> > 
> > Line above returning NOTIFY_OK, check that NETIF_F_HW_ESP is cleared,
> > and remove everything.
> 
> That answer needs more clarification. I came to get confused about what we should do.
> 
> Question 1:
> 
>   Since NETIF_F_HW_ESP is a hardware dependent flag, not all "struct net_device"
>   support NETIF_F_HW_ESP flag. Is this interpretation correct?

Yes.

> Question 2:
> 
>   Sabrina Dubroca commented
> 
>     But the current behavior ("ignore NETIF_F_HW_ESP and call
>     xdo_dev_state_add for new states anyway") has been established for
>     multiple years. Changing that now seems a bit risky.
> 
>   at https://lkml.kernel.org/r/aXd3QjzwOVm0Q9LF@krikkit .
> 
>   Is that comment saying that we have been permitting a "struct net_device"
>   to be selected by xfrm_dev_state_add() even if that "struct net_device"
>   does not support NETIF_F_HW_ESP flag. Is this interpretation correct?

Yes. You can verify this:

    echo 0 > /sys/bus/netdevsim/new_device
    dev=$(ls -1 /sys/bus/netdevsim/devices/netdevsim0/net/)
    ethtool -K $dev esp-hw-offload off  # clears NETIF_F_HW_ESP from dev->features
    ip xfrm state add src 192.168.13.1 dst 192.168.13.2 proto esp spi 0x1000 mode tunnel aead 'rfc4106(gcm(aes))' $key 128 offload crypto dev $dev dir out
    cat /sys/kernel/debug/netdevsim/netdevsim0/ports/0/ipsec
    ip xfrm state

(if you replace $dev in the "ip xfrm state add" command with some
device that can't do ipsec offload at all (no xfrmdev_ops), for
example a veth device, you'll see in the "ip xfrm state" output
something similar but without the "crypto offload" line)


> Question 3:
> 
>   Leon Romanovsky suggested that, as a more robust approach, remove all states
>   and policies when the NETIF_F_HW_ESP feature bit is cleared.
> 
>   But I consider that such approach will not work, for (according to Q2 above)
>   xfrm_dev_state_add() can be called even if the NETIF_F_HW_ESP feature bit is not
>   set. Also, I think that there is no guarantee that dev->features manipulation
>   function is called after xfrm_dev_state_add() was called.
> 
>   Therefore, we need an event that are guaranteed to be called.
>   The NETDEV_UNREGISTER event is guaranteed to be called when unregistring
>   a "struct net_device", and therefore a good place to remove all states and
>   policies.
> 
>   Is this interpretation correct?
> 
> Question 4:
> 
>   If Q1 is correct, Sabrina's comment
> 
>     Changing that now seems a bit risky.
> 
>   in Q2 might be applicable to xfrm_dev_down().
> 
>   That is, someone who is using xfrm with a !NETIF_F_HW_ESP hardware might be
>   expecting that state and policy are not flushed upon NETDEV_DOWN event.
> 
>   If there is such possibility, I think we should avoid changing xfrm_dev_down()
>   and instead re-introduce xfrm_dev_unregister(). What do you think?

True. This is possible with

    ethtool -K $dev esp-hw-offload off ; ip link set $dev down

though not with mlx5 because dev->hw_features does not contain
NETIF_F_HW_ESP in mlx5 devices (this looks like a bug in the driver).

-- 
Sabrina