netdev - Re: [RFC PATCH net-next] pktgen: Introducing a parameter for non-shared skb testing

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: <CAKhg4tL+stODiv8hG0YWmU8zCKR4CsDOEvv7XD-S9PMdas5i_w@mail.gmail.com>
Date: Thu, 7 Sep 2023 11:54:02 +0800
From: Liang Chen <liangchen.linux@...il.com>
To: Benjamin Poirier <benjamin.poirier@...il.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, 
	pabeni@...hat.com, netdev@...r.kernel.org
Subject: Re: [RFC PATCH net-next] pktgen: Introducing a parameter for
 non-shared skb testing

On Thu, Sep 7, 2023 at 6:32 AM Benjamin Poirier
<benjamin.poirier@...il.com> wrote:
>
> On 2023-09-06 18:35 +0800, Liang Chen wrote:
> > Currently, skbs generated by pktgen always have their reference count
> > incremented before transmission, leading to two issues:
> >   1. Only the code paths for shared skbs can be tested.
> >   2. Skbs can only be released by pktgen.
> > To enhance testing comprehensiveness, introducing the "skb_single_user"
> > parameter, which allows skbs with a reference count of 1 to be
> > transmitted. So we can test non-shared skbs and code paths where skbs
> > are released within the network stack.
>
> If my understanding of the code is correct, pktgen operates in the same
> way with parameter clone_skb = 0 and clone_skb = 1.
>

Yeah. pktgen seems to treat user count of 2 as not shared, as long as
the skb is not reused for burst or clone_skb. In that case the only
thing left to do with the skb is to check if user count is
decremented.

> clone_skb = 0 is already meant to work on devices that don't support
> shared skbs (see IFF_TX_SKB_SHARING check in pktgen_if_write()). Instead
> of introducing a new option for your purpose, how about changing
> pktgen_xmit() to send "not shared" skbs when clone_skb == 0?
>

Using clone_skb = 0 to enforce non-sharing makes sense to me. However,
we are a bit concerned that such a change would affect existing users
who have been assuming the current behavior.

> Note that for devices without IFF_TX_SKB_SHARING, it would no longer be
> possible to have pktgen free skbs. Is that important?

It seems that only the "count" capability depends on that. In fact,
this patch is attempting to allow skb release within the network stack
when possible. BTW, to strictly obey the IFF_TX_SKB_SHARING flag,
perhaps the "count" capability can be implemented by supplying a
destructor to skbs.