2016-03-18 00:04:11

by David Decotigny

[permalink] [raw]
Subject: [PATCH v1 0/2] basic ioctl support for netlink sockets

From: David Decotigny <[email protected]>

This removes the requirement that ethtool be tied to the support
of a specific L3 protocol, also updates a comment.

############################################
# Patch Set Summary:

David Decotigny (2):
ethtool: minor doc update
netlink: add support for NIC driver ioctls

include/uapi/linux/ethtool.h | 6 +++---
net/netlink/af_netlink.c | 10 +++++++++-
2 files changed, 12 insertions(+), 4 deletions(-)

--
2.8.0.rc3.226.g39d4020


2016-03-18 00:04:33

by David Decotigny

[permalink] [raw]
Subject: [PATCH v1 2/2] netlink: add support for NIC driver ioctls

From: David Decotigny <[email protected]>

This patch removes the requirement that ethtool be tied to the support
of a specific L3 protocol (ethtool uses an AF_INET socket today).

Signed-off-by: David Decotigny <[email protected]>
---
net/netlink/af_netlink.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index c841679..215fc08 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1033,6 +1033,14 @@ static int netlink_getname(struct socket *sock, struct sockaddr *addr,
return 0;
}

+static int netlink_ioctl(struct socket *sock, unsigned int cmd,
+ unsigned long arg)
+{
+ /* try to hand this ioctl down to the NIC drivers.
+ */
+ return -ENOIOCTLCMD;
+}
+
static struct sock *netlink_getsockbyportid(struct sock *ssk, u32 portid)
{
struct sock *sock;
@@ -2494,7 +2502,7 @@ static const struct proto_ops netlink_ops = {
.accept = sock_no_accept,
.getname = netlink_getname,
.poll = datagram_poll,
- .ioctl = sock_no_ioctl,
+ .ioctl = netlink_ioctl,
.listen = sock_no_listen,
.shutdown = sock_no_shutdown,
.setsockopt = netlink_setsockopt,
--
2.8.0.rc3.226.g39d4020

2016-03-18 00:05:13

by David Decotigny

[permalink] [raw]
Subject: [PATCH v1 1/2] ethtool: minor doc update

From: David Decotigny <[email protected]>

Updates: commit 793cf87de9d1 ("ethtool: Set cmd field in
ETHTOOL_GLINKSETTINGS response to wrong nwords")

Signed-off-by: David Decotigny <[email protected]>
---
include/uapi/linux/ethtool.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 2835b07..9222db8 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1648,9 +1648,9 @@ enum ethtool_reset_flags {
* %ETHTOOL_GLINKSETTINGS: on entry, number of words passed by user
* (>= 0); on return, if handshake in progress, negative if
* request size unsupported by kernel: absolute value indicates
- * kernel recommended size and cmd field is 0, as well as all the
- * other fields; otherwise (handshake completed), strictly
- * positive to indicate size used by kernel and cmd field is
+ * kernel expected size and all the other fields but cmd
+ * are 0; otherwise (handshake completed), strictly positive
+ * to indicate size used by kernel and cmd field stays
* %ETHTOOL_GLINKSETTINGS, all other fields populated by driver. For
* %ETHTOOL_SLINKSETTINGS: must be valid on entry, ie. a positive
* value returned previously by %ETHTOOL_GLINKSETTINGS, otherwise
--
2.8.0.rc3.226.g39d4020

2016-03-18 00:29:47

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] ethtool: minor doc update

On Thu, 2016-03-17 at 17:03 -0700, David Decotigny wrote:
> From: David Decotigny <[email protected]>
>
> Updates: commit 793cf87de9d1 ("ethtool: Set cmd field in
>          ETHTOOL_GLINKSETTINGS response to wrong nwords")
>
> Signed-off-by: David Decotigny <[email protected]>

Reviewed-by: Ben Hutchings <[email protected]>

> ---
>  include/uapi/linux/ethtool.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 2835b07..9222db8 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1648,9 +1648,9 @@ enum ethtool_reset_flags {
>   * %ETHTOOL_GLINKSETTINGS: on entry, number of words passed by user
>   * (>= 0); on return, if handshake in progress, negative if
>   * request size unsupported by kernel: absolute value indicates
> - * kernel recommended size and cmd field is 0, as well as all the
> - * other fields; otherwise (handshake completed), strictly
> - * positive to indicate size used by kernel and cmd field is
> + * kernel expected size and all the other fields but cmd
> + * are 0; otherwise (handshake completed), strictly positive
> + * to indicate size used by kernel and cmd field stays
>   * %ETHTOOL_GLINKSETTINGS, all other fields populated by driver. For
>   * %ETHTOOL_SLINKSETTINGS: must be valid on entry, ie. a positive
>   * value returned previously by %ETHTOOL_GLINKSETTINGS, otherwise
--
Ben Hutchings
Absolutum obsoletum. (If it works, it's out of date.) - Stafford Beer


Attachments:
signature.asc (819.00 B)
This is a digitally signed message part

2016-03-20 17:31:29

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] basic ioctl support for netlink sockets

From: David Decotigny <[email protected]>
Date: Thu, 17 Mar 2016 17:03:54 -0700

> This removes the requirement that ethtool be tied to the support
> of a specific L3 protocol, also updates a comment.

You're adding an ioctl handler to netlink sockets, and it is not
at all apparent to me how this influences ethtool handling at all.

Therefore you have to explain this here and in your commit message.

2016-03-21 17:44:48

by David Decotigny

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] basic ioctl support for netlink sockets

sent updated version. commit description should have the details. what
we are doing there is mimic what some others are doing, such as udp.c:
when returning -ENOIOCTLCMD, net/core would fallback to dev_ioctl,
which implements the NIC driver ioctls, which include ethtool ioctl.

On Sun, Mar 20, 2016 at 10:31 AM, David Miller <[email protected]> wrote:
> From: David Decotigny <[email protected]>
> Date: Thu, 17 Mar 2016 17:03:54 -0700
>
>> This removes the requirement that ethtool be tied to the support
>> of a specific L3 protocol, also updates a comment.
>
> You're adding an ioctl handler to netlink sockets, and it is not
> at all apparent to me how this influences ethtool handling at all.
>
> Therefore you have to explain this here and in your commit message.