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

[Antonio Quartulli]

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.