Re: [PATCH net-next v23 03/23] ovpn: add basic interface creation/destruction/management routines

[Antonio Quartulli <antonio@openvpn.net>]

Hi Qingfang and thanks for chiming in,

On 17/03/2025 07:09, Qingfang Deng wrote:
> Hi Antonio,
> 
> On Wed, 12 Mar 2025 21:54:32 +0100, Antonio Quartulli Wrote:
>> diff --git a/drivers/net/ovpn/main.c b/drivers/net/ovpn/main.c
>> index 28133e7e15e74b8a4a937ed03f70d9f83d7a14c8..e71183e6f42cd801861caaec9eb0f6828b64cda9 100644
>> --- a/drivers/net/ovpn/main.c
>> +++ b/drivers/net/ovpn/main.c
>> @@ -10,14 +10,42 @@
>>   #include <linux/genetlink.h>
>>   #include <linux/module.h>
>>   #include <linux/netdevice.h>
>> +#include <linux/inetdevice.h>
>> +#include <net/ip.h>
>>   #include <net/rtnetlink.h>
>> -#include <uapi/linux/ovpn.h>
>> +#include <uapi/linux/if_arp.h>
>>   
>>   #include "ovpnpriv.h"
>>   #include "main.h"
>>   #include "netlink.h"
>> +#include "io.h"
>> +#include "proto.h"
>> +
>> +static int ovpn_net_open(struct net_device *dev)
>> +{
>> +	netif_tx_start_all_queues(dev);
> 
> This is not required as the virtual interface does not have a queue
> (marked as IFF_NO_QUEUE).

ACK

> 
>> +	return 0;
>> +}
>> +
>> +static int ovpn_net_stop(struct net_device *dev)
>> +{
>> +	netif_tx_stop_all_queues(dev);
> 
> Same as above.

ACK

> 
>> +	return 0;
>> +}
>>   
>>   static const struct net_device_ops ovpn_netdev_ops = {
>> +	.ndo_open		= ovpn_net_open,
>> +	.ndo_stop		= ovpn_net_stop,
>> +	.ndo_start_xmit		= ovpn_net_xmit,
>> +};
>> +
>> +static const struct device_type ovpn_type = {
>> +	.name = OVPN_FAMILY_NAME,
>> +};
>> +
>> +static const struct nla_policy ovpn_policy[IFLA_OVPN_MAX + 1] = {
>> +	[IFLA_OVPN_MODE] = NLA_POLICY_RANGE(NLA_U8, OVPN_MODE_P2P,
>> +					    OVPN_MODE_MP),
>>   };
>>   
>>   /**
>> @@ -31,44 +59,120 @@ bool ovpn_dev_is_valid(const struct net_device *dev)
>>   	return dev->netdev_ops == &ovpn_netdev_ops;
>>   }
>>   
>> +static void ovpn_setup(struct net_device *dev)
>> +{
>> +	netdev_features_t feat = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_RXCSUM |
> 
> Do not advertise NETIF_F_HW_CSUM or NETIF_F_RXCSUM, as TX/RX checksum is
> not handled in hardware.

The idea behind these flags was that the OpenVPN protocol will take care 
of authenticating packets, thus substituting what the CSUM would do here.
For this I wanted to avoid the stack to spend time computing the CSUM in 
software.

I believe wireguard sets those flags for the same reason.

Does it make sense to you?

> 
>> +				 NETIF_F_GSO | NETIF_F_GSO_SOFTWARE |
>> +				 NETIF_F_HIGHDMA;


Regards,

-- 
Antonio Quartulli
OpenVPN Inc.