netdev - Re: [PATCH net-next v3] ppp: enable TX scatter-gather

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: <a957214d-62dc-40cd-a422-73b317bc1827@redhat.com>
Date: Tue, 27 Jan 2026 13:34:03 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Qingfang Deng <dqfext@...il.com>, Andrew Lunn <andrew+netdev@...n.ch>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, linux-ppp@...r.kernel.org,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: Toke Høiland-Jørgensen <toke@...hat.com>
Subject: Re: [PATCH net-next v3] ppp: enable TX scatter-gather

On 1/23/26 2:42 AM, Qingfang Deng wrote:
> PPP channels using chan->direct_xmit prepend the PPP header to a skb and
> call dev_queue_xmit() directly. In this mode the skb does not need to be
> linear, but the PPP netdevice currently does not advertise
> scatter-gather features, causing unnecessary linearization and
> preventing GSO.
> 
> Enable NETIF_F_SG and NETIF_F_FRAGLIST on PPP devices and add an
> .ndo_fix_features() callback to disable them when the underlying PPP
> channel is not using direct_xmit (i.e. IFF_NO_QUEUE is not set in
> priv_flags). This allows the networking core to pass non-linear skbs
> directly only when it is safe to do so.
> 
> PPP compressors still require linear skbs, and their states can change
> at runtime. In order to avoid races, instead of toggling features, call
> skb_linearize() before passing buffers to them. Compressors are uncommon
> on high-speed links, so this does not affect the fast path.
> 
> Signed-off-by: Qingfang Deng <dqfext@...il.com>
> ---
> v2 -> v3:
>  toggle features only if the underlying ppp_channel changes. Call
>   skb_linearize() if compressors are in use.
>  - https://lore.kernel.org/linux-ppp/20251103031501.404141-1-dqfext@gmail.com/
> 
>  drivers/net/ppp/ppp_generic.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
> index f9f0f16c41d1..d706a175cea9 100644
> --- a/drivers/net/ppp/ppp_generic.c
> +++ b/drivers/net/ppp/ppp_generic.c
> @@ -1545,6 +1545,16 @@ ppp_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats64)
>  	dev_fetch_sw_netstats(stats64, dev->tstats);
>  }
>  
> +static netdev_features_t
> +ppp_fix_features(struct net_device *dev, netdev_features_t features)
> +{
> +	/* Don't advertise SG/FRAGLIST when IFF_NO_QUEUE is absent */
> +	if (!(dev->priv_flags & IFF_NO_QUEUE))
> +		features &= ~(NETIF_F_SG | NETIF_F_FRAGLIST);

I spent a little time trying to understanding the logic here and I think
that enabling features depending on IFF_NO_QUEUE is fragile at best.

It looks like that the IFF_NO_QUEUE bit is an inconsistent state for
multilink devices using different type of channels.
Moreover the user-space could attaching a qdisc to the ppp device after
channel initialization.

Instead you could always expose the features and linearize as needed
when transmitting on !direct_xmit channel; no need to touch the
individual channel implementation, you could do such check before
calling the ops->start_xmit() calls (possibly creating a new
wrapper/helper for that).

/P