[PATCH net-next 0/2] sunvnet: add to ethtool data

[PATCH net-next 0/2] sunvnet: add to ethtool data

[Shannon Nelson]

The first patch gets some driver-specific information available for
use by the LDom management utilities through the drvinfo service: the
lowlevel vio version into firmware-version field, and, most importantly,
the remote-mac address into the bus-info field.

When creating the LDoms and their network connections, there currently
isn't a good way to match the vif netdev to the client LDom that it
services.  By having the vif device print the remote-mac address from
the mdesc used to build the connection, the userland tools can correlate
which vif services which LDom.

There was some discussion about adding a 'remote-mac' element into the
/sys/class/net/<dev>/ model, but our thinking was that this might be
too specific to our devices and not fit well into the general model.
We're open to going back to that if others in the community think it
would be a better place for this data.

The second patch implements the get_settings service.  This might seem
moot as these primarily describe a hardware port connection which doesn't
make much sense in a virtualized network connection.  However, there are a
couple of userland LDom management tools that work better if this data is
filled out.  One specific example is that the /sys/class/net/<dev>/speed
attribute becomes readable.

Shannon Nelson (2):
  ldmvsw: add vio version and remote-mac to ethtool info
  sunvnet: implement basic ethtool get_settings

 drivers/net/ethernet/sun/ldmvsw.c         |    8 ++++++++
 drivers/net/ethernet/sun/sunvnet.c        |    1 +
 drivers/net/ethernet/sun/sunvnet_common.c |   11 +++++++++++
 drivers/net/ethernet/sun/sunvnet_common.h |    2 ++
 4 files changed, 22 insertions(+), 0 deletions(-)

[PATCH net-next 1/2] ldmvsw: add vio version and remote-mac to ethtool info

[Shannon Nelson]

In the ethtool -i output print the vio version and the remote-mac
of the ldom that the vif device is serving as this vif info is
not exposed elsewhere.  The remote-mac address is most useful for
tracking which client ldom is being served by the vif.

Orabug: 26316362

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/sun/ldmvsw.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/sun/ldmvsw.c b/drivers/net/ethernet/sun/ldmvsw.c
index 8603e39..de512ef 100644
--- a/drivers/net/ethernet/sun/ldmvsw.c
+++ b/drivers/net/ethernet/sun/ldmvsw.c
@@ -62,8 +62,15 @@
 static void vsw_get_drvinfo(struct net_device *dev,
 			    struct ethtool_drvinfo *info)
 {
+	struct vnet_port *port = netdev_priv(dev);
+
 	strlcpy(info->driver, DRV_MODULE_NAME, sizeof(info->driver));
 	strlcpy(info->version, DRV_MODULE_VERSION, sizeof(info->version));
+
+	snprintf(info->fw_version, sizeof(info->fw_version),
+		 "vio %d.%d", port->vio.ver.major, port->vio.ver.minor);
+	snprintf(info->bus_info, sizeof(info->bus_info),
+		 "remote-mac %pM", port->raddr);
 }
 
 static u32 vsw_get_msglevel(struct net_device *dev)
-- 
1.7.1

Re: [PATCH net-next 1/2] ldmvsw: add vio version and remote-mac to ethtool info

[David Miller]

From: Shannon Nelson <shannon.nelson@oracle.com>
Date: Wed, 21 Jun 2017 09:09:53 -0700

> In the ethtool -i output print the vio version and the remote-mac
> of the ldom that the vif device is serving as this vif info is
> not exposed elsewhere.  The remote-mac address is most useful for
> tracking which client ldom is being served by the vif.
> 
> Orabug: 26316362
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>

Although less useful, it is exposed already via the kernel logs.

Also, this is not at all an appropriate place to expose this.  The
"driver info" ethtool command is not a place to post per-connection
or per-link information.

Probably one of the IFLA_* netlink attributes would work best for
this.  You could even use PHYS_PORT_ID for this, which gives you
32 bytes to work with.

Re: [PATCH net-next 1/2] ldmvsw: add vio version and remote-mac to ethtool info

[Shannon Nelson]

On 6/21/2017 12:05 PM, David Miller wrote:
> From: Shannon Nelson <shannon.nelson@oracle.com>
> Date: Wed, 21 Jun 2017 09:09:53 -0700
> 
>> In the ethtool -i output print the vio version and the remote-mac
>> of the ldom that the vif device is serving as this vif info is
>> not exposed elsewhere.  The remote-mac address is most useful for
>> tracking which client ldom is being served by the vif.
>>
>> Orabug: 26316362
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> 
> Although less useful, it is exposed already via the kernel logs.

I had thought about that, but the idea of parsing kernel logs that could 
expire away didn't thrill me.

> 
> Also, this is not at all an appropriate place to expose this.  The
> "driver info" ethtool command is not a place to post per-connection
> or per-link information.

I knew it was a bit of a stretch, but almost made sense.  However, I'd 
still like to put the vio version here - is that acceptable?

> 
> Probably one of the IFLA_* netlink attributes would work best for
> this.  You could even use PHYS_PORT_ID for this, which gives you
> 32 bytes to work with.

Hmmm - good thought.  I'll go poke at this and see what I can come up 
with.  I gather from this that you agree that /sys is not the right 
place for this information, either.

Thanks,
sln

Re: [PATCH net-next 1/2] ldmvsw: add vio version and remote-mac to ethtool info

[David Miller]

From: Shannon Nelson <shannon.nelson@oracle.com>
Date: Wed, 21 Jun 2017 13:59:35 -0700

> On 6/21/2017 12:05 PM, David Miller wrote:
>> Also, this is not at all an appropriate place to expose this.  The
>> "driver info" ethtool command is not a place to post per-connection
>> or per-link information.
> 
> I knew it was a bit of a stretch, but almost made sense.  However, I'd
> still like to put the vio version here - is that acceptable?

That part is fine.

>> Probably one of the IFLA_* netlink attributes would work best for
>> this.  You could even use PHYS_PORT_ID for this, which gives you
>> 32 bytes to work with.
> 
> Hmmm - good thought.  I'll go poke at this and see what I can come up
> with.  I gather from this that you agree that /sys is not the right
> place for this information, either.

Netlink is definitely preferred.

[PATCH net-next 2/2] sunvnet: implement basic ethtool get_settings

[Shannon Nelson]

Add the get_settings callback so that both the ldmvsw and sunvnet
drivers will give a little more information when asked for its
basic settings.  These aren't necessarily very useful, but they
make some users happier.  Also, a side effect is that the speed
attribute in /sys/class/net/<dev> is now readable, which makes
a couple of the ldom management tools happier.

Orabug: 26175474

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/sun/ldmvsw.c         |    1 +
 drivers/net/ethernet/sun/sunvnet.c        |    1 +
 drivers/net/ethernet/sun/sunvnet_common.c |   11 +++++++++++
 drivers/net/ethernet/sun/sunvnet_common.h |    2 ++
 4 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/sun/ldmvsw.c b/drivers/net/ethernet/sun/ldmvsw.c
index de512ef..0ac389b 100644
--- a/drivers/net/ethernet/sun/ldmvsw.c
+++ b/drivers/net/ethernet/sun/ldmvsw.c
@@ -88,6 +88,7 @@ static void vsw_set_msglevel(struct net_device *dev, u32 value)
 }
 
 static const struct ethtool_ops vsw_ethtool_ops = {
+	.get_settings		= sunvnet_get_settings,
 	.get_drvinfo		= vsw_get_drvinfo,
 	.get_msglevel		= vsw_get_msglevel,
 	.set_msglevel		= vsw_set_msglevel,
diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index 75b167e..d16c351 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -189,6 +189,7 @@ static void vnet_get_ethtool_stats(struct net_device *dev,
 }
 
 static const struct ethtool_ops vnet_ethtool_ops = {
+	.get_settings		= sunvnet_get_settings,
 	.get_drvinfo		= vnet_get_drvinfo,
 	.get_msglevel		= vnet_get_msglevel,
 	.set_msglevel		= vnet_set_msglevel,
diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
index 9e86833..6ca27b0 100644
--- a/drivers/net/ethernet/sun/sunvnet_common.c
+++ b/drivers/net/ethernet/sun/sunvnet_common.c
@@ -1759,3 +1759,14 @@ void sunvnet_port_rm_txq_common(struct vnet_port *port)
 	port->q_index = 0;
 }
 EXPORT_SYMBOL_GPL(sunvnet_port_rm_txq_common);
+
+int sunvnet_get_settings(struct net_device *netdev,
+			 struct ethtool_cmd *ecmd)
+{
+	ethtool_cmd_speed_set(ecmd, 0);
+	ecmd->duplex = DUPLEX_FULL;
+	ecmd->port = PORT_NONE;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(sunvnet_get_settings);
diff --git a/drivers/net/ethernet/sun/sunvnet_common.h b/drivers/net/ethernet/sun/sunvnet_common.h
index b20d6fa..c566ec9 100644
--- a/drivers/net/ethernet/sun/sunvnet_common.h
+++ b/drivers/net/ethernet/sun/sunvnet_common.h
@@ -151,5 +151,7 @@ int sunvnet_start_xmit_common(struct sk_buff *skb, struct net_device *dev,
 bool sunvnet_port_is_up_common(struct vnet_port *vnet);
 void sunvnet_port_add_txq_common(struct vnet_port *port);
 void sunvnet_port_rm_txq_common(struct vnet_port *port);
+int sunvnet_get_settings(struct net_device *netdev,
+			 struct ethtool_cmd *ecmd);
 
 #endif /* _SUNVNETCOMMON_H */
-- 
1.7.1

Re: [PATCH net-next 2/2] sunvnet: implement basic ethtool get_settings

[David Miller]

From: Shannon Nelson <shannon.nelson@oracle.com>
Date: Wed, 21 Jun 2017 09:09:54 -0700

> Add the get_settings callback so that both the ldmvsw and sunvnet
> drivers will give a little more information when asked for its
> basic settings.  These aren't necessarily very useful, but they
> make some users happier.  Also, a side effect is that the speed
> attribute in /sys/class/net/<dev> is now readable, which makes
> a couple of the ldom management tools happier.
> 
> Orabug: 26175474
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>

I would set the speed to something other than zero, and also
consider the ramifications of this change upon things like
'bonding' and 'team'.

Re: [PATCH net-next 2/2] sunvnet: implement basic ethtool get_settings

[Shannon Nelson]

On 6/21/2017 12:06 PM, David Miller wrote:
> From: Shannon Nelson <shannon.nelson@oracle.com>
> Date: Wed, 21 Jun 2017 09:09:54 -0700
> 
>> Add the get_settings callback so that both the ldmvsw and sunvnet
>> drivers will give a little more information when asked for its
>> basic settings.  These aren't necessarily very useful, but they
>> make some users happier.  Also, a side effect is that the speed
>> attribute in /sys/class/net/<dev> is now readable, which makes
>> a couple of the ldom management tools happier.
>>
>> Orabug: 26175474
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> 
> I would set the speed to something other than zero, and also

I kept looking at that and wasn't sure which value to pretend with.  I 
looked at what was reported in the Solaris world and saw '0' so went 
with that.  I suppose using SPEED_10000 shouldn't hurt anything, and is 
close enough to observed client-to-client speeds.

> consider the ramifications of this change upon things like
> 'bonding' and 'team'.

The request I was responding to with this was really more targeted at 
the ldmvsw (host) side of the connection, the vif device, which I 
believe only ever gets connected to the vsw bridge.  You're right, tho', 
because the eth0 in the ldom client *does* get sucked into teams.  I may 
drop the sunvnet/client side of this patch and stick with just the 
host/ldmvsw side.

Thanks,
sln

Re: [PATCH net-next 2/2] sunvnet: implement basic ethtool get_settings

[David Miller]

From: Shannon Nelson <shannon.nelson@oracle.com>
Date: Wed, 21 Jun 2017 13:59:45 -0700

> On 6/21/2017 12:06 PM, David Miller wrote:
>> From: Shannon Nelson <shannon.nelson@oracle.com>
>> Date: Wed, 21 Jun 2017 09:09:54 -0700
>> 
>>> Add the get_settings callback so that both the ldmvsw and sunvnet
>>> drivers will give a little more information when asked for its
>>> basic settings.  These aren't necessarily very useful, but they
>>> make some users happier.  Also, a side effect is that the speed
>>> attribute in /sys/class/net/<dev> is now readable, which makes
>>> a couple of the ldom management tools happier.
>>>
>>> Orabug: 26175474
>>>
>>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>> I would set the speed to something other than zero, and also
> 
> I kept looking at that and wasn't sure which value to pretend with.  I
> looked at what was reported in the Solaris world and saw '0' so went
> with that.  I suppose using SPEED_10000 shouldn't hurt anything, and
> is close enough to observed client-to-client speeds.
> 
>> consider the ramifications of this change upon things like
>> 'bonding' and 'team'.
> 
> The request I was responding to with this was really more targeted at
> the ldmvsw (host) side of the connection, the vif device, which I
> believe only ever gets connected to the vsw bridge.  You're right,
> tho', because the eth0 in the ldom client *does* get sucked into
> teams.  I may drop the sunvnet/client side of this patch and stick
> with just the host/ldmvsw side.

Usually if possible you propagate the speed of the underlying physical
layer, if applicable.

You can see what some other software devices do in the case of ethtool
link speed settings.