Message ID | 20241209-b4-ovpn-v14-0-ea243cf16417@openvpn.net |
---|---|
Headers | show |
Series | Introducing OpenVPN Data Channel Offload | expand |
On Mon, Dec 09, 2024 at 09:53:17AM +0100, Antonio Quartulli wrote: > Packets received over the socket are forwarded to the user device. > > Implementation is UDP only. TCP will be added by a later patch. > > Note: no decryption/decapsulation exists yet, packets are forwarded as > they arrive without much processing. > > Signed-off-by: Antonio Quartulli <antonio@openvpn.net> ... > diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c > index c0e7aa289ad3345fcd91e7c890f70961300c356f..975392fc39bc4c0107a07a53795afecd88d72c53 100644 > --- a/drivers/net/ovpn/udp.c > +++ b/drivers/net/ovpn/udp.c > @@ -23,9 +23,83 @@ > #include "bind.h" > #include "io.h" > #include "peer.h" > +#include "proto.h" > #include "socket.h" > #include "udp.h" > > +/** > + * ovpn_udp_encap_recv - Start processing a received UDP packet. > + * @sk: socket over which the packet was received > + * @skb: the received packet > + * > + * If the first byte of the payload is DATA_V2, the packet is further processed, > + * otherwise it is forwarded to the UDP stack for delivery to user space. > + * > + * Return: > + * 0 if skb was consumed or dropped > + * >0 if skb should be passed up to userspace as UDP (packet not consumed) > + * <0 if skb should be resubmitted as proto -N (packet not consumed) > + */ > +static int ovpn_udp_encap_recv(struct sock *sk, struct sk_buff *skb) > +{ > + struct ovpn_priv *ovpn; > + struct ovpn_peer *peer; > + u32 peer_id; > + u8 opcode; > + > + ovpn = ovpn_from_udp_sock(sk); > + if (unlikely(!ovpn)) { > + net_err_ratelimited("%s: cannot obtain ovpn object from UDP socket\n", > + netdev_name(ovpn->dev)); Hi Antonio, If we reach here then ovpn is NULL. But the like above dereferences it. Flagged by Smatch. > + goto drop_noovpn; > + } ...
On Mon, Dec 09, 2024 at 09:53:31AM +0100, Antonio Quartulli wrote: > The ovpn-cli tool can be compiled and used as selftest for the ovpn > kernel module. > > [NOTE: it depends on libmedtls for decoding base64-encoded keys] > > ovpn-cli implements the netlink and RTNL APIs and can thus be integrated > in any script for more automated testing. > > Along with the tool, 4 scripts are provided that perform basic > functionality tests by means of network namespaces. > These scripts take part to the kselftest automation. > > The output of the scripts, which will appear in the kselftest > reports, is a list of steps performed by the scripts plus some > output coming from the execution of `ping`, `iperf` and `ovpn-cli` > itself. > In general it is useful only in case of failure, in order to > understand which step has failed and why. > > Cc: linux-kselftest@vger.kernel.org > Signed-off-by: Antonio Quartulli <antonio@openvpn.net> > Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> ... > +/** > + * Helper function used to easily add attributes to a rtnl message > + */ Hi Antonio, This comment starts with a '/**' but is otherwise not formatted as a Kernel doc. Probably it is best to simply start the comment with '/*'. Likewise elsewhere in this patch. Flagged by ./scripts/kernel-doc -none > +static int ovpn_addattr(struct nlmsghdr *n, int maxlen, int type, > + const void *data, int alen) ...
On 10/12/2024 17:44, Simon Horman wrote: > On Mon, Dec 09, 2024 at 09:53:17AM +0100, Antonio Quartulli wrote: >> Packets received over the socket are forwarded to the user device. >> >> Implementation is UDP only. TCP will be added by a later patch. >> >> Note: no decryption/decapsulation exists yet, packets are forwarded as >> they arrive without much processing. >> >> Signed-off-by: Antonio Quartulli <antonio@openvpn.net> > > ... > >> diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c >> index c0e7aa289ad3345fcd91e7c890f70961300c356f..975392fc39bc4c0107a07a53795afecd88d72c53 100644 >> --- a/drivers/net/ovpn/udp.c >> +++ b/drivers/net/ovpn/udp.c >> @@ -23,9 +23,83 @@ >> #include "bind.h" >> #include "io.h" >> #include "peer.h" >> +#include "proto.h" >> #include "socket.h" >> #include "udp.h" >> >> +/** >> + * ovpn_udp_encap_recv - Start processing a received UDP packet. >> + * @sk: socket over which the packet was received >> + * @skb: the received packet >> + * >> + * If the first byte of the payload is DATA_V2, the packet is further processed, >> + * otherwise it is forwarded to the UDP stack for delivery to user space. >> + * >> + * Return: >> + * 0 if skb was consumed or dropped >> + * >0 if skb should be passed up to userspace as UDP (packet not consumed) >> + * <0 if skb should be resubmitted as proto -N (packet not consumed) >> + */ >> +static int ovpn_udp_encap_recv(struct sock *sk, struct sk_buff *skb) >> +{ >> + struct ovpn_priv *ovpn; >> + struct ovpn_peer *peer; >> + u32 peer_id; >> + u8 opcode; >> + >> + ovpn = ovpn_from_udp_sock(sk); >> + if (unlikely(!ovpn)) { >> + net_err_ratelimited("%s: cannot obtain ovpn object from UDP socket\n", >> + netdev_name(ovpn->dev)); > > Hi Antonio, > > If we reach here then ovpn is NULL. > But the like above dereferences it. > > Flagged by Smatch. Hi Simon, Thanks for pointing this out. I will get this fixed and add smatch to my test battery. Regards, > >> + goto drop_noovpn; >> + } > > ... >
On 10/12/2024 17:47, Simon Horman wrote: > On Mon, Dec 09, 2024 at 09:53:31AM +0100, Antonio Quartulli wrote: >> The ovpn-cli tool can be compiled and used as selftest for the ovpn >> kernel module. >> >> [NOTE: it depends on libmedtls for decoding base64-encoded keys] >> >> ovpn-cli implements the netlink and RTNL APIs and can thus be integrated >> in any script for more automated testing. >> >> Along with the tool, 4 scripts are provided that perform basic >> functionality tests by means of network namespaces. >> These scripts take part to the kselftest automation. >> >> The output of the scripts, which will appear in the kselftest >> reports, is a list of steps performed by the scripts plus some >> output coming from the execution of `ping`, `iperf` and `ovpn-cli` >> itself. >> In general it is useful only in case of failure, in order to >> understand which step has failed and why. >> >> Cc: linux-kselftest@vger.kernel.org >> Signed-off-by: Antonio Quartulli <antonio@openvpn.net> >> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> > > ... > >> +/** >> + * Helper function used to easily add attributes to a rtnl message >> + */ > > Hi Antonio, > > This comment starts with a '/**' but is otherwise not formatted as > a Kernel doc. Probably it is best to simply start the comment with '/*'. > > Likewise elsewhere in this patch. Will fix all instances of this issue. > > Flagged by ./scripts/kernel-doc -none Darn, I have been running kernel-doc only against drivers/net/ovpn. Thanks for pointing this out. Regards, > >> +static int ovpn_addattr(struct nlmsghdr *n, int maxlen, int type, >> + const void *data, int alen) > > ...