netdev - Re: [RFC PATCH net 0/3] virtio-net: allow usage of small vrings

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]

Date:   Mon, 1 May 2023 11:41:44 +0000
From:   Alvaro Karsz <alvaro.karsz@...id-run.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>
CC:     "jasowang@...hat.com" <jasowang@...hat.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "xuanzhuo@...ux.alibaba.com" <xuanzhuo@...ux.alibaba.com>
Subject: Re: [RFC PATCH net 0/3] virtio-net: allow usage of small vrings

> > > Why the difference?
> > >
> >
> > Because the RING_SIZE < 4 case requires much more adjustments.
> >
> > * We may need to squeeze the virtio header into the headroom.
> > * We may need to squeeze the GSO header into the headroom, or block the features.
> 
> We alread do this though no?
> I think we'll need to tweak hard_header_len to guarantee it's there
> as opposed to needed_headroom ...
> 
> > * At the moment, without NETIF_F_SG, we can receive a skb with 2 segments, we may need to reduce it to 1.
> 
> You are saying clearing NETIF_F_SG does not guarantee a linear skb?
> 

I don't know..
I'm not sure what is the cause, but using this patchset, without any host GSO feature, I can get a chain of 3 descriptors.
Posing an example of a 4 entries ring during iperf3, acting as a client:
(TX descriptors)

len=86       flags 0x1         addr 0xf738d000
len=1448   flags 0x0         addr 0xf738d800
len=86       flags 0x8081   addr 0xf738e000
len=1184,   flags 0x8081  addr 0xf738e800
len=264     flags 0x8080   addr 0xf738f000
len=86       flags 0x8081   addr 0xf738f800
len=1448   flags 0x0         addr 0xf7390000
len=86       flags 0x1         addr 0xf7390800
len=1448   flags 0x0         addr 0xf7391000
len=86       flags 0x1         addr 0xf716a800
len=1448   flags 0x8080   addr 0xf716b000
len=86       flags 0x8081   addr 0xf7391800
len=1448   flags 0x8080   addr 0xf7392000

We got a chain of 3 in here.
This happens often.

Now, when negotiating host GSO features, I can get up to 4:

len=86       flags 0x1         addr 0xf71fc800
len=21328 flags 0x1         addr 0xf6e00800
len=32768 flags 0x8081   addr 0xf6e06000
len=11064 flags 0x8080   addr 0xf6e0e000
len=86       flags 0x8081   addr 0xf738b000
len=1         flags 0x8080   addr 0xf738b800
len=86       flags 0x1         addr 0xf738c000
len=21704 flags 0x1         addr 0xf738c800
len=32768 flags 0x1         addr 0xf7392000
len=10688 flags 0x0         addr 0xf739a000
len=86       flags 0x8081   addr 0xf739d000
len=22080 flags 0x8081   addr 0xf739d800
len=32768 flags 0x8081   addr 0xf73a3000
len=10312 flags 0x8080   addr 0xf73ab000

TBH, I thought that this behaviour was expected until you mentioned it,
This is why virtnet_calc_max_descs returns 3 if no host_gso feature is negotiated, and 4 otherwise.
I was thinking that we may need to use another skb to hold the TSO template (for headers generation)...

Any ideas?

> > * We may need to change all the control commands, so class,  command and command specific data will fit in a single segment.
> > * We may need to disable the control command and all the features depending on it.
> 
> well if we don't commands just fail as we can't add them right?
> no corruption or stalls ...
> 
> > * We may need to disable NAPI?
> 
> hmm why napi?
> 

I'm not sure if it's required to disable it, but I'm not sure what's the point having napi if the ring size is 1..
Will it work?

> > There may be more changes..
> >
> > I was thinking that it may be easier to start with the easier case RING_SIZE >= 4, make sure everything is working fine, then send a follow up patchset with the required adjustments for RING_SIZE < 4.
> 
> 
> it's ok but I'm just trying to figure out where does 4 come from.
> 

I guess this part was not clear, sorry..
In case of split vqs, we have ring size 2 before 4.
And as you saw, we still get chains of 3 when NETIF_F_SG is off (Which I thought was expected).