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

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]

Message-ID: <f4c9a29f-a5c6-464a-a659-c7ffeaf123c1@openvpn.net>
Date: Mon, 17 Mar 2025 10:23:55 +0100
From: Antonio Quartulli <antonio@...nvpn.net>
To: Qingfang Deng <dqfext@...il.com>
Cc: andrew+netdev@...n.ch, donald.hunter@...il.com, edumazet@...gle.com,
 horms@...nel.org, kuba@...nel.org, linux-kernel@...r.kernel.org,
 linux-kselftest@...r.kernel.org, netdev@...r.kernel.org, pabeni@...hat.com,
 ryazanov.s.a@...il.com, sd@...asysnail.net, shaw.leon@...il.com,
 shuah@...nel.org
Subject: Re: [PATCH net-next v23 03/23] ovpn: add basic interface
 creation/destruction/management routines

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.