Re: [PATCH net] vmxnet3: use gro callback when UPT is enabled

[Yunsheng Lin]

On 2023/3/15 10:27, Ronak Doshi wrote:
> 
>> On 3/14/23, 6:52 PM, "Yunsheng Lin" <linyunsheng@xxxxxxxxxx <mailto:linyunsheng@xxxxxxxxxx>> wrote:
>>
>> Does clearing the NETIF_F_GRO for netdev->features bring back the performance?
>> If no, maybe there is something need investigating.
> 
> Yes, it does. Simply using netif_receive_skb works fine.
> 
>> Checking rq->shared->updateRxProd in the driver to decide if gro is allow does not seems right to
>> me, as the netstack has used the NETIF_F_GRO checking in netif_elide_gro().
>>
> updateRxProd is NOT being used to determine if GRO is allowed. It is being used to indicate UPT is
> active, so the driver should just use GRO callback. This is as good as having only GRO callback for UPT driver
> which you were suggesting earlier.
> 
>> Does clearing NETIF_F_GRO for netdev->features during the driver init process works for your
>> case?
> 
> No this does not work as UPT mode can be enabled/disabled at runtime without guest being informed.
> This is para-virtualized driver and does not know if the guest is being run in emulation or UPT.

I think checking updateRxProd in some way means the above para-virtualized driver need to
know if the guest is being run in emulation or UPT.

I am not sure how we can handle the runtime hw capability changing thing yet, that is why
I suggested setting the hw capability during the driver init process, then user can enable
or disable GRO if need to.

Suppose user enable the software GRO using ethtool, disabling the GRO through some runtime
checking seems against the will of the user.

Also, if you are able to "add an event to notify the guest about this", I suppose the
para-virtualized driver will clear the specific bit in netdev->hw_features and
netdev->features when handling the event? does user need to be notified about this, does
user get confusion about this change without notification?

IMHO, being para-virtualized driver does not make any difference, the users do not care if
they are configuring a netdev behind a para-virtualized driver or not.

> 
>> As netdev->hw_features is for the driver to advertise the hw's capability, and the driver
>> can enable/disable specific capability by setting netdev->features during the driver init
>> process, and user can get to enable/disable specific capability using ethtool later if user
>> need to.
> 
> As I mentioned above, guest is not informed at runtime about UPT status. So, we need this
> mechanism to avoid performance penalty.
> 
> Thanks,
> Ronak
> 
> 
>