* [PATCH net-next v2 0/2] virtio_net: add ethtool get/set settings support
@ 2016-02-02 14:33 Nikolay Aleksandrov
2016-02-02 14:33 ` [PATCH net-next v2 1/2] ethtool: add speed/duplex validation functions Nikolay Aleksandrov
2016-02-02 14:33 ` [PATCH net-next v2 2/2] virtio_net: add ethtool support for set and get of settings Nikolay Aleksandrov
0 siblings, 2 replies; 7+ messages in thread
From: Nikolay Aleksandrov @ 2016-02-02 14:33 UTC (permalink / raw)
To: netdev; +Cc: mst, roopa, davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Hi,
Patch 1 adds ethtool speed/duplex validation functions which check if the
value is defined. Patch 2 adds support for ethtool (get|set)_settings and
uses the validation functions to check the user-supplied values.
v2: split in 2 patches to allow everyone to make use of the validation
functions and allow virtio_net devices to be half duplex
Cheers,
Nik
Nikolay Aleksandrov (2):
ethtool: add speed/duplex validation functions
virtio_net: add ethtool support for set and get of settings
drivers/net/virtio_net.c | 41 +++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/ethtool.h | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
--
2.4.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next v2 1/2] ethtool: add speed/duplex validation functions
2016-02-02 14:33 [PATCH net-next v2 0/2] virtio_net: add ethtool get/set settings support Nikolay Aleksandrov
@ 2016-02-02 14:33 ` Nikolay Aleksandrov
2016-02-02 14:35 ` Michael S. Tsirkin
2016-02-02 14:33 ` [PATCH net-next v2 2/2] virtio_net: add ethtool support for set and get of settings Nikolay Aleksandrov
1 sibling, 1 reply; 7+ messages in thread
From: Nikolay Aleksandrov @ 2016-02-02 14:33 UTC (permalink / raw)
To: netdev; +Cc: mst, roopa, davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Add functions which check if the speed/duplex are defined.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: new patch
include/uapi/linux/ethtool.h | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 57fa39005e79..b2e180181629 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1319,11 +1319,45 @@ enum ethtool_sfeatures_retval_bits {
#define SPEED_UNKNOWN -1
+static inline int ethtool_validate_speed(__u32 speed)
+{
+ switch (speed) {
+ case SPEED_10:
+ case SPEED_100:
+ case SPEED_1000:
+ case SPEED_2500:
+ case SPEED_5000:
+ case SPEED_10000:
+ case SPEED_20000:
+ case SPEED_25000:
+ case SPEED_40000:
+ case SPEED_50000:
+ case SPEED_56000:
+ case SPEED_100000:
+ case SPEED_UNKNOWN:
+ return 1;
+ }
+
+ return 0;
+}
+
/* Duplex, half or full. */
#define DUPLEX_HALF 0x00
#define DUPLEX_FULL 0x01
#define DUPLEX_UNKNOWN 0xff
+static inline int ethtool_validate_duplex(__u8 duplex)
+{
+ switch (duplex) {
+ case DUPLEX_HALF:
+ case DUPLEX_FULL:
+ case DUPLEX_UNKNOWN:
+ return 1;
+ }
+
+ return 0;
+}
+
/* Which connector port. */
#define PORT_TP 0x00
#define PORT_AUI 0x01
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next v2 2/2] virtio_net: add ethtool support for set and get of settings
2016-02-02 14:33 [PATCH net-next v2 0/2] virtio_net: add ethtool get/set settings support Nikolay Aleksandrov
2016-02-02 14:33 ` [PATCH net-next v2 1/2] ethtool: add speed/duplex validation functions Nikolay Aleksandrov
@ 2016-02-02 14:33 ` Nikolay Aleksandrov
2016-02-02 14:54 ` Michael S. Tsirkin
1 sibling, 1 reply; 7+ messages in thread
From: Nikolay Aleksandrov @ 2016-02-02 14:33 UTC (permalink / raw)
To: netdev; +Cc: mst, roopa, davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
This patch allows the user to set and retrieve speed and duplex of the
virtio_net device via ethtool. Having this functionality is very helpful
for simulating different environments and also enables the virtio_net
device to participate in operations where proper speed and duplex are
required (e.g. currently bonding lacp mode requires full duplex). Custom
speed and duplex are not allowed, the user-supplied settings are validated
before applying.
Example:
$ ethtool eth1
Settings for eth1:
...
Speed: Unknown!
Duplex: Unknown! (255)
$ ethtool -s eth1 speed 1000 duplex full
$ ethtool eth1
Settings for eth1:
...
Speed: 1000Mb/s
Duplex: Full
Based on a patch by Roopa Prabhu.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: use the new ethtool speed/duplex validation functions and allow half
duplex to be set
drivers/net/virtio_net.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 767ab11a6e9f..88fdb162c5f5 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -146,6 +146,10 @@ struct virtnet_info {
virtio_net_ctrl_ack ctrl_status;
u8 ctrl_promisc;
u8 ctrl_allmulti;
+
+ /* Ethtool settings */
+ u8 duplex;
+ u32 speed;
};
struct padded_vnet_hdr {
@@ -1376,6 +1380,39 @@ static void virtnet_get_channels(struct net_device *dev,
channels->other_count = 0;
}
+static int virtnet_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ u32 speed = ethtool_cmd_speed(cmd);
+
+ /* don't allow custom speed and duplex */
+ if (!ethtool_validate_speed(speed) ||
+ !ethtool_validate_duplex(cmd->duplex))
+ return -EINVAL;
+ vi->speed = speed;
+ vi->duplex = cmd->duplex;
+
+ return 0;
+}
+
+static int virtnet_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+
+ ethtool_cmd_speed_set(cmd, vi->speed);
+ cmd->duplex = vi->duplex;
+
+ return 0;
+}
+
+static void virtnet_init_settings(struct net_device *dev)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+
+ vi->speed = SPEED_UNKNOWN;
+ vi->duplex = DUPLEX_UNKNOWN;
+}
+
static const struct ethtool_ops virtnet_ethtool_ops = {
.get_drvinfo = virtnet_get_drvinfo,
.get_link = ethtool_op_get_link,
@@ -1383,6 +1420,8 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
.set_channels = virtnet_set_channels,
.get_channels = virtnet_get_channels,
.get_ts_info = ethtool_op_get_ts_info,
+ .get_settings = virtnet_get_settings,
+ .set_settings = virtnet_set_settings,
};
#define MIN_MTU 68
@@ -1855,6 +1894,8 @@ static int virtnet_probe(struct virtio_device *vdev)
netif_set_real_num_tx_queues(dev, vi->curr_queue_pairs);
netif_set_real_num_rx_queues(dev, vi->curr_queue_pairs);
+ virtnet_init_settings(dev);
+
err = register_netdev(dev);
if (err) {
pr_debug("virtio_net: registering device failed\n");
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/2] ethtool: add speed/duplex validation functions
2016-02-02 14:33 ` [PATCH net-next v2 1/2] ethtool: add speed/duplex validation functions Nikolay Aleksandrov
@ 2016-02-02 14:35 ` Michael S. Tsirkin
0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2016-02-02 14:35 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, roopa, davem, Nikolay Aleksandrov
On Tue, Feb 02, 2016 at 03:33:39PM +0100, Nikolay Aleksandrov wrote:
> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>
> Add functions which check if the speed/duplex are defined.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> v2: new patch
>
> include/uapi/linux/ethtool.h | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 57fa39005e79..b2e180181629 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1319,11 +1319,45 @@ enum ethtool_sfeatures_retval_bits {
>
> #define SPEED_UNKNOWN -1
>
> +static inline int ethtool_validate_speed(__u32 speed)
> +{
> + switch (speed) {
> + case SPEED_10:
> + case SPEED_100:
> + case SPEED_1000:
> + case SPEED_2500:
> + case SPEED_5000:
> + case SPEED_10000:
> + case SPEED_20000:
> + case SPEED_25000:
> + case SPEED_40000:
> + case SPEED_50000:
> + case SPEED_56000:
> + case SPEED_100000:
> + case SPEED_UNKNOWN:
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> /* Duplex, half or full. */
> #define DUPLEX_HALF 0x00
> #define DUPLEX_FULL 0x01
> #define DUPLEX_UNKNOWN 0xff
>
> +static inline int ethtool_validate_duplex(__u8 duplex)
> +{
> + switch (duplex) {
> + case DUPLEX_HALF:
> + case DUPLEX_FULL:
> + case DUPLEX_UNKNOWN:
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> /* Which connector port. */
> #define PORT_TP 0x00
> #define PORT_AUI 0x01
> --
> 2.4.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 2/2] virtio_net: add ethtool support for set and get of settings
2016-02-02 14:33 ` [PATCH net-next v2 2/2] virtio_net: add ethtool support for set and get of settings Nikolay Aleksandrov
@ 2016-02-02 14:54 ` Michael S. Tsirkin
2016-02-02 14:59 ` Nikolay Aleksandrov
0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2016-02-02 14:54 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, roopa, davem, Nikolay Aleksandrov
On Tue, Feb 02, 2016 at 03:33:40PM +0100, Nikolay Aleksandrov wrote:
> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>
> This patch allows the user to set and retrieve speed and duplex of the
> virtio_net device via ethtool. Having this functionality is very helpful
> for simulating different environments and also enables the virtio_net
> device to participate in operations where proper speed and duplex are
> required (e.g. currently bonding lacp mode requires full duplex). Custom
> speed and duplex are not allowed, the user-supplied settings are validated
> before applying.
>
> Example:
> $ ethtool eth1
> Settings for eth1:
> ...
> Speed: Unknown!
> Duplex: Unknown! (255)
> $ ethtool -s eth1 speed 1000 duplex full
> $ ethtool eth1
> Settings for eth1:
> ...
> Speed: 1000Mb/s
> Duplex: Full
>
> Based on a patch by Roopa Prabhu.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> v2: use the new ethtool speed/duplex validation functions and allow half
> duplex to be set
>
> drivers/net/virtio_net.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 767ab11a6e9f..88fdb162c5f5 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -146,6 +146,10 @@ struct virtnet_info {
> virtio_net_ctrl_ack ctrl_status;
> u8 ctrl_promisc;
> u8 ctrl_allmulti;
> +
> + /* Ethtool settings */
> + u8 duplex;
> + u32 speed;
> };
>
> struct padded_vnet_hdr {
> @@ -1376,6 +1380,39 @@ static void virtnet_get_channels(struct net_device *dev,
> channels->other_count = 0;
> }
>
> +static int virtnet_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> +{
> + struct virtnet_info *vi = netdev_priv(dev);
need an empty line here.
> + u32 speed = ethtool_cmd_speed(cmd);
> +
> + /* don't allow custom speed and duplex */
> + if (!ethtool_validate_speed(speed) ||
> + !ethtool_validate_duplex(cmd->duplex))
> + return -EINVAL;
There's a problem here: rest of the fields are ignored.
Here's one way to validate user doesn't attempt to set them:
struct ethtool_cmd diff1 = cmd;
struct ethtool_cmd diff2 = {};
ethtool_cmd_speed_set(&diff1, 0);
diff1.duplex = 0;
if (memcmp(&diff1, &diff2, sizeof diff1))
return -EINVAL;
> + vi->speed = speed;
> + vi->duplex = cmd->duplex;
> +
> + return 0;
> +}
> +
> +static int virtnet_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> +{
> + struct virtnet_info *vi = netdev_priv(dev);
> +
> + ethtool_cmd_speed_set(cmd, vi->speed);
> + cmd->duplex = vi->duplex;
I think we should at least set port to PORT_OTHER.
> +
> + return 0;
> +}
> +
> +static void virtnet_init_settings(struct net_device *dev)
> +{
> + struct virtnet_info *vi = netdev_priv(dev);
> +
> + vi->speed = SPEED_UNKNOWN;
> + vi->duplex = DUPLEX_UNKNOWN;
> +}
> +
> static const struct ethtool_ops virtnet_ethtool_ops = {
> .get_drvinfo = virtnet_get_drvinfo,
> .get_link = ethtool_op_get_link,
> @@ -1383,6 +1420,8 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
> .set_channels = virtnet_set_channels,
> .get_channels = virtnet_get_channels,
> .get_ts_info = ethtool_op_get_ts_info,
> + .get_settings = virtnet_get_settings,
> + .set_settings = virtnet_set_settings,
> };
>
> #define MIN_MTU 68
> @@ -1855,6 +1894,8 @@ static int virtnet_probe(struct virtio_device *vdev)
> netif_set_real_num_tx_queues(dev, vi->curr_queue_pairs);
> netif_set_real_num_rx_queues(dev, vi->curr_queue_pairs);
>
> + virtnet_init_settings(dev);
> +
> err = register_netdev(dev);
> if (err) {
> pr_debug("virtio_net: registering device failed\n");
> --
> 2.4.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 2/2] virtio_net: add ethtool support for set and get of settings
2016-02-02 14:54 ` Michael S. Tsirkin
@ 2016-02-02 14:59 ` Nikolay Aleksandrov
2016-02-02 15:36 ` Michael S. Tsirkin
0 siblings, 1 reply; 7+ messages in thread
From: Nikolay Aleksandrov @ 2016-02-02 14:59 UTC (permalink / raw)
To: Michael S. Tsirkin, Nikolay Aleksandrov; +Cc: netdev, roopa, davem
On 02/02/2016 03:54 PM, Michael S. Tsirkin wrote:
> On Tue, Feb 02, 2016 at 03:33:40PM +0100, Nikolay Aleksandrov wrote:
>> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>
>> This patch allows the user to set and retrieve speed and duplex of the
>> virtio_net device via ethtool. Having this functionality is very helpful
>> for simulating different environments and also enables the virtio_net
>> device to participate in operations where proper speed and duplex are
>> required (e.g. currently bonding lacp mode requires full duplex). Custom
>> speed and duplex are not allowed, the user-supplied settings are validated
>> before applying.
>>
>> Example:
>> $ ethtool eth1
>> Settings for eth1:
>> ...
>> Speed: Unknown!
>> Duplex: Unknown! (255)
>> $ ethtool -s eth1 speed 1000 duplex full
>> $ ethtool eth1
>> Settings for eth1:
>> ...
>> Speed: 1000Mb/s
>> Duplex: Full
>>
>> Based on a patch by Roopa Prabhu.
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>> v2: use the new ethtool speed/duplex validation functions and allow half
>> duplex to be set
>>
>> drivers/net/virtio_net.c | 41 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 41 insertions(+)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 767ab11a6e9f..88fdb162c5f5 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -146,6 +146,10 @@ struct virtnet_info {
>> virtio_net_ctrl_ack ctrl_status;
>> u8 ctrl_promisc;
>> u8 ctrl_allmulti;
>> +
>> + /* Ethtool settings */
>> + u8 duplex;
>> + u32 speed;
>> };
>>
>> struct padded_vnet_hdr {
>> @@ -1376,6 +1380,39 @@ static void virtnet_get_channels(struct net_device *dev,
>> channels->other_count = 0;
>> }
>>
>> +static int virtnet_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
>> +{
>> + struct virtnet_info *vi = netdev_priv(dev);
>
> need an empty line here.
>
The next line is a local variable declaration, you want a new line between
the two local variable declarations ?
>> + u32 speed = ethtool_cmd_speed(cmd);
>> +
>> + /* don't allow custom speed and duplex */
>> + if (!ethtool_validate_speed(speed) ||
>> + !ethtool_validate_duplex(cmd->duplex))
>> + return -EINVAL;
>
> There's a problem here: rest of the fields are ignored.
> Here's one way to validate user doesn't attempt to set them:
>
Hmm, yes, they'll silently get ignored without an error. Good point,
and I'll modify as necessary.
Thanks!
> struct ethtool_cmd diff1 = cmd;
> struct ethtool_cmd diff2 = {};
>
> ethtool_cmd_speed_set(&diff1, 0);
> diff1.duplex = 0;
>
> if (memcmp(&diff1, &diff2, sizeof diff1))
> return -EINVAL;
>
>
>> + vi->speed = speed;
>> + vi->duplex = cmd->duplex;
>> +
>> + return 0;
>> +}
>> +
>> +static int virtnet_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
>> +{
>> + struct virtnet_info *vi = netdev_priv(dev);
>> +
>> + ethtool_cmd_speed_set(cmd, vi->speed);
>> + cmd->duplex = vi->duplex;
>
>
> I think we should at least set port to PORT_OTHER.
>
>> +
>> + return 0;
>> +}
>> +
>> +static void virtnet_init_settings(struct net_device *dev)
>> +{
>> + struct virtnet_info *vi = netdev_priv(dev);
>> +
>> + vi->speed = SPEED_UNKNOWN;
>> + vi->duplex = DUPLEX_UNKNOWN;
>> +}
>> +
>> static const struct ethtool_ops virtnet_ethtool_ops = {
>> .get_drvinfo = virtnet_get_drvinfo,
>> .get_link = ethtool_op_get_link,
>> @@ -1383,6 +1420,8 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>> .set_channels = virtnet_set_channels,
>> .get_channels = virtnet_get_channels,
>> .get_ts_info = ethtool_op_get_ts_info,
>> + .get_settings = virtnet_get_settings,
>> + .set_settings = virtnet_set_settings,
>> };
>>
>> #define MIN_MTU 68
>> @@ -1855,6 +1894,8 @@ static int virtnet_probe(struct virtio_device *vdev)
>> netif_set_real_num_tx_queues(dev, vi->curr_queue_pairs);
>> netif_set_real_num_rx_queues(dev, vi->curr_queue_pairs);
>>
>> + virtnet_init_settings(dev);
>> +
>> err = register_netdev(dev);
>> if (err) {
>> pr_debug("virtio_net: registering device failed\n");
>> --
>> 2.4.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 2/2] virtio_net: add ethtool support for set and get of settings
2016-02-02 14:59 ` Nikolay Aleksandrov
@ 2016-02-02 15:36 ` Michael S. Tsirkin
0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2016-02-02 15:36 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: Nikolay Aleksandrov, netdev, roopa, davem
On Tue, Feb 02, 2016 at 03:59:52PM +0100, Nikolay Aleksandrov wrote:
> On 02/02/2016 03:54 PM, Michael S. Tsirkin wrote:
> > On Tue, Feb 02, 2016 at 03:33:40PM +0100, Nikolay Aleksandrov wrote:
> >> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> >>
> >> This patch allows the user to set and retrieve speed and duplex of the
> >> virtio_net device via ethtool. Having this functionality is very helpful
> >> for simulating different environments and also enables the virtio_net
> >> device to participate in operations where proper speed and duplex are
> >> required (e.g. currently bonding lacp mode requires full duplex). Custom
> >> speed and duplex are not allowed, the user-supplied settings are validated
> >> before applying.
> >>
> >> Example:
> >> $ ethtool eth1
> >> Settings for eth1:
> >> ...
> >> Speed: Unknown!
> >> Duplex: Unknown! (255)
> >> $ ethtool -s eth1 speed 1000 duplex full
> >> $ ethtool eth1
> >> Settings for eth1:
> >> ...
> >> Speed: 1000Mb/s
> >> Duplex: Full
> >>
> >> Based on a patch by Roopa Prabhu.
> >>
> >> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> >> ---
> >> v2: use the new ethtool speed/duplex validation functions and allow half
> >> duplex to be set
> >>
> >> drivers/net/virtio_net.c | 41 +++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 41 insertions(+)
> >>
> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> index 767ab11a6e9f..88fdb162c5f5 100644
> >> --- a/drivers/net/virtio_net.c
> >> +++ b/drivers/net/virtio_net.c
> >> @@ -146,6 +146,10 @@ struct virtnet_info {
> >> virtio_net_ctrl_ack ctrl_status;
> >> u8 ctrl_promisc;
> >> u8 ctrl_allmulti;
> >> +
> >> + /* Ethtool settings */
> >> + u8 duplex;
> >> + u32 speed;
> >> };
> >>
> >> struct padded_vnet_hdr {
> >> @@ -1376,6 +1380,39 @@ static void virtnet_get_channels(struct net_device *dev,
> >> channels->other_count = 0;
> >> }
> >>
> >> +static int virtnet_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> >> +{
> >> + struct virtnet_info *vi = netdev_priv(dev);
> >
> > need an empty line here.
> >
>
> The next line is a local variable declaration, you want a new line between
> the two local variable declarations ?
Oh, you are right.
> >> + u32 speed = ethtool_cmd_speed(cmd);
> >> +
> >> + /* don't allow custom speed and duplex */
> >> + if (!ethtool_validate_speed(speed) ||
> >> + !ethtool_validate_duplex(cmd->duplex))
> >> + return -EINVAL;
> >
> > There's a problem here: rest of the fields are ignored.
> > Here's one way to validate user doesn't attempt to set them:
> >
>
> Hmm, yes, they'll silently get ignored without an error. Good point,
> and I'll modify as necessary.
>
> Thanks!
>
> > struct ethtool_cmd diff1 = cmd;
> > struct ethtool_cmd diff2 = {};
> >
> > ethtool_cmd_speed_set(&diff1, 0);
> > diff1.duplex = 0;
> >
> > if (memcmp(&diff1, &diff2, sizeof diff1))
> > return -EINVAL;
> >
> >
> >> + vi->speed = speed;
> >> + vi->duplex = cmd->duplex;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int virtnet_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> >> +{
> >> + struct virtnet_info *vi = netdev_priv(dev);
> >> +
> >> + ethtool_cmd_speed_set(cmd, vi->speed);
> >> + cmd->duplex = vi->duplex;
> >
> >
> > I think we should at least set port to PORT_OTHER.
> >
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static void virtnet_init_settings(struct net_device *dev)
> >> +{
> >> + struct virtnet_info *vi = netdev_priv(dev);
> >> +
> >> + vi->speed = SPEED_UNKNOWN;
> >> + vi->duplex = DUPLEX_UNKNOWN;
> >> +}
> >> +
> >> static const struct ethtool_ops virtnet_ethtool_ops = {
> >> .get_drvinfo = virtnet_get_drvinfo,
> >> .get_link = ethtool_op_get_link,
> >> @@ -1383,6 +1420,8 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
> >> .set_channels = virtnet_set_channels,
> >> .get_channels = virtnet_get_channels,
> >> .get_ts_info = ethtool_op_get_ts_info,
> >> + .get_settings = virtnet_get_settings,
> >> + .set_settings = virtnet_set_settings,
> >> };
> >>
> >> #define MIN_MTU 68
> >> @@ -1855,6 +1894,8 @@ static int virtnet_probe(struct virtio_device *vdev)
> >> netif_set_real_num_tx_queues(dev, vi->curr_queue_pairs);
> >> netif_set_real_num_rx_queues(dev, vi->curr_queue_pairs);
> >>
> >> + virtnet_init_settings(dev);
> >> +
> >> err = register_netdev(dev);
> >> if (err) {
> >> pr_debug("virtio_net: registering device failed\n");
> >> --
> >> 2.4.3
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-02-02 15:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-02 14:33 [PATCH net-next v2 0/2] virtio_net: add ethtool get/set settings support Nikolay Aleksandrov
2016-02-02 14:33 ` [PATCH net-next v2 1/2] ethtool: add speed/duplex validation functions Nikolay Aleksandrov
2016-02-02 14:35 ` Michael S. Tsirkin
2016-02-02 14:33 ` [PATCH net-next v2 2/2] virtio_net: add ethtool support for set and get of settings Nikolay Aleksandrov
2016-02-02 14:54 ` Michael S. Tsirkin
2016-02-02 14:59 ` Nikolay Aleksandrov
2016-02-02 15:36 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.