Thu, Jun 06, 2024 at 09:56:50AM CEST, jasowang@xxxxxxxxxx wrote: >On Thu, Jun 6, 2024 at 2:05 PM Michael S. Tsirkin <mst@xxxxxxxxxx> wrote: >> >> On Thu, Jun 06, 2024 at 12:25:15PM +0800, Jason Wang wrote: >> > > If the codes of orphan mode don't have an impact when you enable >> > > napi_tx mode, please keep it if you can. >> > >> > For example, it complicates BQL implementation. >> > >> > Thanks >> >> I very much doubt sending interrupts to a VM can >> *on all benchmarks* compete with not sending interrupts. > >It should not differ too much from the physical NIC. We can have one >more round of benchmarks to see the difference. > >But if NAPI mode needs to win all of the benchmarks in order to get >rid of orphan, that would be very difficult. Considering various bugs >will be fixed by dropping skb_orphan(), it would be sufficient if most >of the benchmark doesn't show obvious differences. > >Looking at git history, there're commits that removes skb_orphan(), for example: > >commit 8112ec3b8722680251aecdcc23dfd81aa7af6340 >Author: Eric Dumazet <edumazet@xxxxxxxxxx> >Date: Fri Sep 28 07:53:26 2012 +0000 > > mlx4: dont orphan skbs in mlx4_en_xmit() > > After commit e22979d96a55d (mlx4_en: Moving to Interrupts for TX > completions) we no longer need to orphan skbs in mlx4_en_xmit() > since skb wont stay a long time in TX ring before their release. > > Orphaning skbs in ndo_start_xmit() should be avoided as much as > possible, since it breaks TCP Small Queue or other flow control > mechanisms (per socket limits) > > Signed-off-by: Eric Dumazet <edumazet@xxxxxxxxxx> > Acked-by: Yevgeny Petrilin <yevgenyp@xxxxxxxxxxxx> > Cc: Or Gerlitz <ogerlitz@xxxxxxxxxxxx> > Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx> > >> >> So yea, it's great if napi and hardware are advanced enough >> that the default can be changed, since this way virtio >> is closer to a regular nic and more or standard >> infrastructure can be used. >> >> But dropping it will go against *no breaking userspace* rule. >> Complicated? Tough. > >I don't know what kind of userspace is broken by this. Or why it is >not broken since the day we enable NAPI mode by default. There is a module option that explicitly allows user to set napi_tx=false or napi_weight=0 So if you remove this option or ignore it, both breaks the user expectation. I personally would vote for this breakage. To carry ancient things like this one forever does not make sense to me. While at it, let's remove all virtio net module params. Thoughts? > >Thanks > >> >> -- >> MST >> >
