2021-08-16 16:19:11

by Kubalewski, Arkadiusz

[permalink] [raw]
Subject: [RFC net-next 4/7] net: add ioctl interface for recover reference clock on netdev

Previously there was no similar interface. It is required for
configuration of Synchronous Ethernet (SyncE).

User must be able to enable or disable propagation of its
PHY recovered clock signal onto available output pin.
This allows the signal to be used as frequency reference signal
for different hardware chips.

Signed-off-by: Arkadiusz Kubalewski <[email protected]>
---
include/uapi/linux/net_synce.h | 21 +++++++++++++++++++++
include/uapi/linux/sockios.h | 4 ++++
net/core/dev_ioctl.c | 6 +++++-
3 files changed, 30 insertions(+), 1 deletion(-)
create mode 100644 include/uapi/linux/net_synce.h

diff --git a/include/uapi/linux/net_synce.h b/include/uapi/linux/net_synce.h
new file mode 100644
index 000000000000..a482a7e43151
--- /dev/null
+++ b/include/uapi/linux/net_synce.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Userspace API for synchronous ethernet configuration
+ *
+ * Copyright (C) 2021 Intel Corporation
+ * Author: Arkadiusz Kubalewski <[email protected]>
+ *
+ */
+#ifndef _NET_SYNCE_H
+#define _NET_SYNCE_H
+#include <linux/types.h>
+
+/*
+ * Structure used for passing data with SIOCSSYNCE and SIOCGSYNCE ioctls
+ */
+struct synce_ref_clk_cfg {
+ __u8 pin_id;
+ _Bool enable;
+};
+
+#endif /* _NET_SYNCE_H */
diff --git a/include/uapi/linux/sockios.h b/include/uapi/linux/sockios.h
index 7d1bccbbef78..32c7d4909c31 100644
--- a/include/uapi/linux/sockios.h
+++ b/include/uapi/linux/sockios.h
@@ -153,6 +153,10 @@
#define SIOCSHWTSTAMP 0x89b0 /* set and get config */
#define SIOCGHWTSTAMP 0x89b1 /* get config */

+/* synchronous ethernet config per physical function */
+#define SIOCSSYNCE 0x89c0 /* set and get config */
+#define SIOCGSYNCE 0x89c1 /* get config */
+
/* Device private ioctl calls */

/*
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 0e87237fd871..fe21365c9492 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -406,7 +406,9 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
cmd == SIOCGMIIREG ||
cmd == SIOCSMIIREG ||
cmd == SIOCSHWTSTAMP ||
- cmd == SIOCGHWTSTAMP) {
+ cmd == SIOCGHWTSTAMP ||
+ cmd == SIOCSSYNCE ||
+ cmd == SIOCGSYNCE) {
err = dev_eth_ioctl(dev, ifr, cmd);
} else if (cmd == SIOCBONDENSLAVE ||
cmd == SIOCBONDRELEASE ||
@@ -577,6 +579,7 @@ int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr,
case SIOCBRADDIF:
case SIOCBRDELIF:
case SIOCSHWTSTAMP:
+ case SIOCSSYNCE:
if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
return -EPERM;
fallthrough;
@@ -605,6 +608,7 @@ int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr,
default:
if (cmd == SIOCWANDEV ||
cmd == SIOCGHWTSTAMP ||
+ cmd == SIOCGSYNCE ||
(cmd >= SIOCDEVPRIVATE &&
cmd <= SIOCDEVPRIVATE + 15)) {
dev_load(net, ifr->ifr_name);
--
2.24.0


2021-08-16 19:48:31

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC net-next 4/7] net: add ioctl interface for recover reference clock on netdev

On Mon, Aug 16, 2021 at 6:18 PM Arkadiusz Kubalewski
<[email protected]> wrote:

> +/*
> + * Structure used for passing data with SIOCSSYNCE and SIOCGSYNCE ioctls
> + */
> +struct synce_ref_clk_cfg {
> + __u8 pin_id;
> + _Bool enable;
> +};

I'm not sure if there are any guarantees about the size and alignment of _Bool,
maybe better use __u8 here as well, if only for clarity.

> +#endif /* _NET_SYNCE_H */
> diff --git a/include/uapi/linux/sockios.h b/include/uapi/linux/sockios.h
> index 7d1bccbbef78..32c7d4909c31 100644
> --- a/include/uapi/linux/sockios.h
> +++ b/include/uapi/linux/sockios.h
> @@ -153,6 +153,10 @@
> #define SIOCSHWTSTAMP 0x89b0 /* set and get config */
> #define SIOCGHWTSTAMP 0x89b1 /* get config */
>
> +/* synchronous ethernet config per physical function */
> +#define SIOCSSYNCE 0x89c0 /* set and get config */
> +#define SIOCGSYNCE 0x89c1 /* get config */

I understand that these are traditionally using the old-style 16-bit
numbers, but is there any reason to keep doing that rather than
making them modern like this?

#define SIOCSSYNCE _IOWR(0x89, 0xc0, struct synce_ref_clk_cfg)
/* set and get config */
#define SIOCGSYNCE _IOR(0x89, 0xc1, struct synce_ref_clk_cfg)
/* get config */

Arnd

2021-08-17 10:37:23

by Kubalewski, Arkadiusz

[permalink] [raw]
Subject: RE: [RFC net-next 4/7] net: add ioctl interface for recover reference clock on netdev

>On Mon, Aug 16, 2021 at 6:18 PM Arkadiusz Kubalewski
><[email protected]> wrote:
>
>> +/*
>> + * Structure used for passing data with SIOCSSYNCE and SIOCGSYNCE ioctls
>> + */
>> +struct synce_ref_clk_cfg {
>> + __u8 pin_id;
>> + _Bool enable;
>> +};
>
>I'm not sure if there are any guarantees about the size and alignment of _Bool,
>maybe better use __u8 here as well, if only for clarity.
>

Sure, will fix that in next patch, seems reasonable

>> +#endif /* _NET_SYNCE_H */
>> diff --git a/include/uapi/linux/sockios.h b/include/uapi/linux/sockios.h
>> index 7d1bccbbef78..32c7d4909c31 100644
>> --- a/include/uapi/linux/sockios.h
>> +++ b/include/uapi/linux/sockios.h
>> @@ -153,6 +153,10 @@
>> #define SIOCSHWTSTAMP 0x89b0 /* set and get config */
>> #define SIOCGHWTSTAMP 0x89b1 /* get config */
>>
>> +/* synchronous ethernet config per physical function */
>> +#define SIOCSSYNCE 0x89c0 /* set and get config */
>> +#define SIOCGSYNCE 0x89c1 /* get config */
>
>I understand that these are traditionally using the old-style 16-bit
>numbers, but is there any reason to keep doing that rather than
>making them modern like this?

Personally I would try to keep it one way, just for consistency,
but you might be right - making it modern way is better option.
If no other objections to this comment I am going to change it according to
Arnd's suggestion in next patch.

>
>#define SIOCSSYNCE _IOWR(0x89, 0xc0, struct synce_ref_clk_cfg)
>/* set and get config */
>#define SIOCGSYNCE _IOR(0x89, 0xc1, struct synce_ref_clk_cfg)
>/* get config */
>
> Arnd
>

Thank you,
Arkadiusz

2021-08-22 01:28:41

by Richard Cochran

[permalink] [raw]
Subject: Re: [RFC net-next 4/7] net: add ioctl interface for recover reference clock on netdev

On Mon, Aug 16, 2021 at 06:07:14PM +0200, Arkadiusz Kubalewski wrote:

> +/*
> + * Structure used for passing data with SIOCSSYNCE and SIOCGSYNCE ioctls
> + */
> +struct synce_ref_clk_cfg {
> + __u8 pin_id;

How can the user know what values of 'pin_id' are valid and useful?

> + _Bool enable;
> +};

Thanks,
Richard