On Thu, Jun 6, 2024 at 9:45 PM Jiri Pirko <jiri@xxxxxxxxxxx> wrote: > > 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. We can keep them, but I wonder what's the expectation of the user here? The only thing so far I can imagine is the performance difference. > I personally would vote for this breakage. To carry ancient > things like this one forever does not make sense to me. Exactly. > While at it, > let's remove all virtio net module params. Thoughts? I tend to 1) drop the orphan mode, but we can have some benchmarks first 2) keep the module parameters Thanks > > > > > > >Thanks > > > >> > >> -- > >> MST > >> > > >
