[PATCH net-next 0/9] sunvnet driver updates

[PATCH net-next 0/9] sunvnet driver updates

[Shannon Nelson]

The sunvnet ldom virtual network driver was due for some updates and
a bugfix or two.  These patches address a few items left over from
last year's make-over.

Shannon Nelson (8):
  sunvnet: make sunvnet common code dynamically loadable
  sunvnet: update version and version printing
  sunvnet: add driver stats for ethtool support
  sunvnet: add memory barrier before check for tx enable
  sunvnet: straighten up message event handling logic
  sunvnet: remove extra rcu_read_unlocks
  ldmvsw: update and simplify version string
  ldmvsw: disable tso and gso for bridge operations

Sowmini Varadhan (1):
  sunvnet: remove unused variable in maybe_tx_wakeup

 drivers/net/ethernet/sun/Kconfig          |    9 +-
 drivers/net/ethernet/sun/ldmvsw.c         |   82 ++++++++++++++---
 drivers/net/ethernet/sun/sunvnet.c        |   77 ++++++++++++++--
 drivers/net/ethernet/sun/sunvnet_common.c |  143 ++++++++++++++++------------
 4 files changed, 224 insertions(+), 87 deletions(-)

[PATCH net-next 1/9] sunvnet: make sunvnet common code dynamically loadable

[Shannon Nelson]

When the sunvnet_common code was split out for use by both sunvnet
and the newer ldmvsw, it was made into a static kernel library, which
limits the usefulness of sunvnet and ldmvsw as loadables, since most
of the real work is being done in the shared code.  Also, this is
simply dead code in kernels that aren't running the LDoms.

This patch makes the sunvnet_common into a dynamically loadable
module and makes sunvnet and ldmvsw dependent on sunvnet_common.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/sun/Kconfig          |    9 ++++++---
 drivers/net/ethernet/sun/sunvnet_common.c |   29 +++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/sun/Kconfig b/drivers/net/ethernet/sun/Kconfig
index b499c57..0bde375 100644
--- a/drivers/net/ethernet/sun/Kconfig
+++ b/drivers/net/ethernet/sun/Kconfig
@@ -72,20 +72,23 @@ config CASSINI
 	  <http://docs.oracle.com/cd/E19113-01/giga.ether.pci/817-4341-10/817-4341-10.pdf>.
 
 config SUNVNET_COMMON
-	bool
+	tristate "Common routines to support Sun Virtual Networking"
 	depends on SUN_LDOMS
-	default y if SUN_LDOMS
-
+	default m if SUN_LDOMS
 
 config SUNVNET
 	tristate "Sun Virtual Network support"
+	default m
 	depends on SUN_LDOMS
+	depends on SUNVNET_COMMON
 	---help---
 	  Support for virtual network devices under Sun Logical Domains.
 
 config LDMVSW
 	tristate "Sun4v LDoms Virtual Switch support"
+	default m
 	depends on SUN_LDOMS
+	depends on SUNVNET_COMMON
 	---help---
 	  Support for virtual switch devices under Sun4v Logical Domains.
 	  This driver adds a network interface for every vsw-port node
diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
index a2da2b4..77fbb23 100644
--- a/drivers/net/ethernet/sun/sunvnet_common.c
+++ b/drivers/net/ethernet/sun/sunvnet_common.c
@@ -37,6 +37,35 @@
  */
 #define	VNET_MAX_RETRIES	10
 
+#define DRV_MODULE_NAME		"sunvnet_common"
+#define DRV_MODULE_VERSION	"1.1"
+#define DRV_MODULE_RELDATE	"February 3, 2017"
+
+static char version[] =
+	DRV_MODULE_NAME " " DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")";
+MODULE_AUTHOR("David S. Miller (davem@davemloft.net)");
+MODULE_DESCRIPTION("Sun LDOM virtual network support library");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_MODULE_VERSION);
+
+static int __init sunvnet_common_init(void)
+{
+	pr_info("%s\n", version);
+	return 0;
+}
+module_init(sunvnet_common_init);
+
+static void __exit sunvnet_common_exit(void)
+{
+	/* Empty function, just here to fill the exit function pointer
+	 * slot.  In some combinations of older gcc and newer kernel,
+	 * leaving this undefined results in the kernel marking it as a
+	 * permanent module; it will show up in lsmod output as [permanent]
+	 * and not be unloadable.
+	 */
+}
+module_exit(sunvnet_common_exit);
+
 static int __vnet_tx_trigger(struct vnet_port *port, u32 start);
 static void vnet_port_reset(struct vnet_port *port);
 
-- 
1.7.1

[PATCH net-next 2/9] sunvnet: remove unused variable in maybe_tx_wakeup

[Shannon Nelson]

From: Sowmini Varadhan <sowmini.varadhan@oracle.com>

The vio_dring_state *dr variable is unused in maybe_tx_wakeup().
As the comments indicate, we call maybe_tx_wakeup() whenever we
get a STOPPED LDC message on the port. If the queue is stopped,
we want to wake it up so that we will send another START message
at the next TX and trigger the consumer to drain the dring.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/sun/sunvnet_common.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
index 77fbb23..63e3cec 100644
--- a/drivers/net/ethernet/sun/sunvnet_common.c
+++ b/drivers/net/ethernet/sun/sunvnet_common.c
@@ -743,12 +743,8 @@ static void maybe_tx_wakeup(struct vnet_port *port)
 	txq = netdev_get_tx_queue(VNET_PORT_TO_NET_DEVICE(port),
 				  port->q_index);
 	__netif_tx_lock(txq, smp_processor_id());
-	if (likely(netif_tx_queue_stopped(txq))) {
-		struct vio_dring_state *dr;
-
-		dr = &port->vio.drings[VIO_DRIVER_TX_RING];
+	if (likely(netif_tx_queue_stopped(txq)))
 		netif_tx_wake_queue(txq);
-	}
 	__netif_tx_unlock(txq);
 }
 
-- 
1.7.1

[PATCH net-next 3/9] sunvnet: update version and version printing

[Shannon Nelson]

There have been several changes since the first version of this code, so
we bump the version number.  While we're at it, we can simplify the
version printing a bit and drop a couple lines of code.

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

diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index 38d0e42..883525a 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -38,11 +38,11 @@
 #define VNET_TX_TIMEOUT			(5 * HZ)
 
 #define DRV_MODULE_NAME		"sunvnet"
-#define DRV_MODULE_VERSION	"1.0"
-#define DRV_MODULE_RELDATE	"June 25, 2007"
+#define DRV_MODULE_VERSION	"2.0"
+#define DRV_MODULE_RELDATE	"February 3, 2017"
 
 static char version[] =
-	DRV_MODULE_NAME ".c:v" DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")\n";
+	DRV_MODULE_NAME " " DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")";
 MODULE_AUTHOR("David S. Miller (davem@davemloft.net)");
 MODULE_DESCRIPTION("Sun LDOM virtual network driver");
 MODULE_LICENSE("GPL");
@@ -300,11 +300,6 @@ static struct vio_driver_ops vnet_vio_ops = {
 	.handshake_complete	= sunvnet_handshake_complete_common,
 };
 
-static void print_version(void)
-{
-	printk_once(KERN_INFO "%s", version);
-}
-
 const char *remote_macaddr_prop = "remote-mac-address";
 
 static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
@@ -317,8 +312,6 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 	int len, i, err, switch_port;
 	u64 node;
 
-	print_version();
-
 	hp = mdesc_grab();
 
 	node = vio_vdev_node(hp, vdev);
@@ -453,6 +446,7 @@ static struct vio_driver vnet_port_driver = {
 
 static int __init vnet_init(void)
 {
+	pr_info("%s\n", version);
 	return vio_register_driver(&vnet_port_driver);
 }
 
-- 
1.7.1

[PATCH net-next 4/9] sunvnet: add driver stats for ethtool support

[Shannon Nelson]

Since we're collecting some stats in the driver code, let's support use
of the ethtool driver stats facility in both sunvnet and ldmvsw.

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

diff --git a/drivers/net/ethernet/sun/ldmvsw.c b/drivers/net/ethernet/sun/ldmvsw.c
index bf5818a..7c5dab4 100644
--- a/drivers/net/ethernet/sun/ldmvsw.c
+++ b/drivers/net/ethernet/sun/ldmvsw.c
@@ -81,11 +81,74 @@ static void vsw_set_msglevel(struct net_device *dev, u32 value)
 	port->vp->msg_enable = value;
 }
 
+static const struct {
+	const char string[ETH_GSTRING_LEN];
+} ethtool_stats_keys[] = {
+	{ "rx_packets" },
+	{ "tx_packets" },
+	{ "rx_bytes" },
+	{ "tx_bytes" },
+	{ "rx_errors" },
+	{ "tx_errors" },
+	{ "rx_dropped" },
+	{ "tx_dropped" },
+	{ "multicast" },
+	{ "rx_length_errors" },
+	{ "rx_frame_errors" },
+	{ "rx_missed_errors" },
+	{ "tx_carrier_errors" },
+};
+
+static int vsw_get_sset_count(struct net_device *dev, int sset)
+{
+	switch (sset) {
+	case ETH_SS_STATS:
+		return ARRAY_SIZE(ethtool_stats_keys);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static void vsw_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
+{
+	switch (stringset) {
+	case ETH_SS_STATS:
+		memcpy(buf, &ethtool_stats_keys, sizeof(ethtool_stats_keys));
+		break;
+	default:
+		WARN_ON(1);
+		break;
+	}
+}
+
+static void vsw_get_ethtool_stats(struct net_device *dev,
+				  struct ethtool_stats *estats, u64 *data)
+{
+	int i = 0;
+
+	data[i++] = dev->stats.rx_packets;
+	data[i++] = dev->stats.tx_packets;
+	data[i++] = dev->stats.rx_bytes;
+	data[i++] = dev->stats.tx_bytes;
+	data[i++] = dev->stats.rx_errors;
+	data[i++] = dev->stats.tx_errors;
+	data[i++] = dev->stats.rx_dropped;
+	data[i++] = dev->stats.tx_dropped;
+	data[i++] = dev->stats.multicast;
+	data[i++] = dev->stats.rx_length_errors;
+	data[i++] = dev->stats.rx_frame_errors;
+	data[i++] = dev->stats.rx_missed_errors;
+	data[i++] = dev->stats.tx_carrier_errors;
+}
+
 static const struct ethtool_ops vsw_ethtool_ops = {
 	.get_drvinfo		= vsw_get_drvinfo,
 	.get_msglevel		= vsw_get_msglevel,
 	.set_msglevel		= vsw_set_msglevel,
 	.get_link		= ethtool_op_get_link,
+	.get_sset_count		= vsw_get_sset_count,
+	.get_strings		= vsw_get_strings,
+	.get_ethtool_stats	= vsw_get_ethtool_stats,
 };
 
 static LIST_HEAD(vnet_list);
diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index 883525a..9e385d1 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -77,11 +77,74 @@ static void vnet_set_msglevel(struct net_device *dev, u32 value)
 	vp->msg_enable = value;
 }
 
+static const struct {
+	const char string[ETH_GSTRING_LEN];
+} ethtool_stats_keys[] = {
+	{ "rx_packets" },
+	{ "tx_packets" },
+	{ "rx_bytes" },
+	{ "tx_bytes" },
+	{ "rx_errors" },
+	{ "tx_errors" },
+	{ "rx_dropped" },
+	{ "tx_dropped" },
+	{ "multicast" },
+	{ "rx_length_errors" },
+	{ "rx_frame_errors" },
+	{ "rx_missed_errors" },
+	{ "tx_carrier_errors" },
+};
+
+static int vnet_get_sset_count(struct net_device *dev, int sset)
+{
+	switch (sset) {
+	case ETH_SS_STATS:
+		return ARRAY_SIZE(ethtool_stats_keys);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static void vnet_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
+{
+	switch (stringset) {
+	case ETH_SS_STATS:
+		memcpy(buf, &ethtool_stats_keys, sizeof(ethtool_stats_keys));
+		break;
+	default:
+		WARN_ON(1);
+		break;
+	}
+}
+
+static void vnet_get_ethtool_stats(struct net_device *dev,
+				   struct ethtool_stats *estats, u64 *data)
+{
+	int i = 0;
+
+	data[i++] = dev->stats.rx_packets;
+	data[i++] = dev->stats.tx_packets;
+	data[i++] = dev->stats.rx_bytes;
+	data[i++] = dev->stats.tx_bytes;
+	data[i++] = dev->stats.rx_errors;
+	data[i++] = dev->stats.tx_errors;
+	data[i++] = dev->stats.rx_dropped;
+	data[i++] = dev->stats.tx_dropped;
+	data[i++] = dev->stats.multicast;
+	data[i++] = dev->stats.rx_length_errors;
+	data[i++] = dev->stats.rx_frame_errors;
+	data[i++] = dev->stats.rx_missed_errors;
+	data[i++] = dev->stats.tx_carrier_errors;
+}
+
 static const struct ethtool_ops vnet_ethtool_ops = {
 	.get_drvinfo		= vnet_get_drvinfo,
 	.get_msglevel		= vnet_get_msglevel,
 	.set_msglevel		= vnet_set_msglevel,
 	.get_link		= ethtool_op_get_link,
+	.get_sset_count		= vnet_get_sset_count,
+	.get_strings		= vnet_get_strings,
+	.get_ethtool_stats	= vnet_get_ethtool_stats,
 };
 
 static LIST_HEAD(vnet_list);
diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
index 63e3cec..5d0d386 100644
--- a/drivers/net/ethernet/sun/sunvnet_common.c
+++ b/drivers/net/ethernet/sun/sunvnet_common.c
@@ -433,6 +433,8 @@ static int vnet_rx_one(struct vnet_port *port, struct vio_net_desc *desc)
 
 	skb->ip_summed = port->switch_port ? CHECKSUM_NONE : CHECKSUM_PARTIAL;
 
+	if (unlikely(is_multicast_ether_addr(eth_hdr(skb)->h_dest)))
+		dev->stats.multicast++;
 	dev->stats.rx_packets++;
 	dev->stats.rx_bytes += len;
 	napi_gro_receive(&port->napi, skb);
-- 
1.7.1

[PATCH net-next 5/9] sunvnet: add memory barrier before check for tx enable

[Shannon Nelson]

In order to allow the underlying LDC and outstanding memory operations
to potentially catch up with the driver's Tx requests, add a memory
barrier before checking again for available tx descriptors.

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

diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
index 5d0d386..98e758e 100644
--- a/drivers/net/ethernet/sun/sunvnet_common.c
+++ b/drivers/net/ethernet/sun/sunvnet_common.c
@@ -1467,6 +1467,7 @@ ldc_start_done:
 	dr->prod = (dr->prod + 1) & (VNET_TX_RING_SIZE - 1);
 	if (unlikely(vnet_tx_dring_avail(dr) < 1)) {
 		netif_tx_stop_queue(txq);
+		dma_wmb();
 		if (vnet_tx_dring_avail(dr) > VNET_TX_WAKEUP_THRESH(dr))
 			netif_tx_wake_queue(txq);
 	}
-- 
1.7.1

Re: [PATCH net-next 5/9] sunvnet: add memory barrier before check for tx enable

[Eric Dumazet]

On Fri, 2017-02-03 at 09:42 -0800, Shannon Nelson wrote:
> In order to allow the underlying LDC and outstanding memory operations
> to potentially catch up with the driver's Tx requests, add a memory
> barrier before checking again for available tx descriptors.
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
>  drivers/net/ethernet/sun/sunvnet_common.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
> index 5d0d386..98e758e 100644
> --- a/drivers/net/ethernet/sun/sunvnet_common.c
> +++ b/drivers/net/ethernet/sun/sunvnet_common.c
> @@ -1467,6 +1467,7 @@ ldc_start_done:
>  	dr->prod = (dr->prod + 1) & (VNET_TX_RING_SIZE - 1);
>  	if (unlikely(vnet_tx_dring_avail(dr) < 1)) {
>  		netif_tx_stop_queue(txq);
> +		dma_wmb();

This does not look right.

I believe you need smp_rmb() here.

>  		if (vnet_tx_dring_avail(dr) > VNET_TX_WAKEUP_THRESH(dr))
>  			netif_tx_wake_queue(txq);
>  	}

Re: [PATCH net-next 5/9] sunvnet: add memory barrier before check for tx enable

[Shannon Nelson]

On 2/3/2017 9:56 AM, Eric Dumazet wrote:
> On Fri, 2017-02-03 at 09:42 -0800, Shannon Nelson wrote:
>> In order to allow the underlying LDC and outstanding memory operations
>> to potentially catch up with the driver's Tx requests, add a memory
>> barrier before checking again for available tx descriptors.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>> ---
>>  drivers/net/ethernet/sun/sunvnet_common.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
>> index 5d0d386..98e758e 100644
>> --- a/drivers/net/ethernet/sun/sunvnet_common.c
>> +++ b/drivers/net/ethernet/sun/sunvnet_common.c
>> @@ -1467,6 +1467,7 @@ ldc_start_done:
>>  	dr->prod = (dr->prod + 1) & (VNET_TX_RING_SIZE - 1);
>>  	if (unlikely(vnet_tx_dring_avail(dr) < 1)) {
>>  		netif_tx_stop_queue(txq);
>> +		dma_wmb();
>
> This does not look right.
>
> I believe you need smp_rmb() here.

Well, it probably should be dma_rmb(), since regardless of the number of 
cores we think we have, we're communicating with a peer ldom that has 
its own core(s).  Either way, on sparc they all seem to boil down to the 
same bit of asm, but using the "rmb" part makes more logical sense. 
I'll respin with dma_rmb().

Good catch, thanks,
sln


>
>>  		if (vnet_tx_dring_avail(dr) > VNET_TX_WAKEUP_THRESH(dr))
>>  			netif_tx_wake_queue(txq);
>>  	}
>
>

Re: [PATCH net-next 5/9] sunvnet: add memory barrier before check for tx enable

[David Miller]

From: Shannon Nelson <shannon.nelson@oracle.com>
Date: Fri, 3 Feb 2017 13:20:43 -0800

> On 2/3/2017 9:56 AM, Eric Dumazet wrote:
>> On Fri, 2017-02-03 at 09:42 -0800, Shannon Nelson wrote:
>>> In order to allow the underlying LDC and outstanding memory operations
>>> to potentially catch up with the driver's Tx requests, add a memory
>>> barrier before checking again for available tx descriptors.
>>>
>>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>>> ---
>>>  drivers/net/ethernet/sun/sunvnet_common.c |    1 +
>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/sun/sunvnet_common.c
>>> b/drivers/net/ethernet/sun/sunvnet_common.c
>>> index 5d0d386..98e758e 100644
>>> --- a/drivers/net/ethernet/sun/sunvnet_common.c
>>> +++ b/drivers/net/ethernet/sun/sunvnet_common.c
>>> @@ -1467,6 +1467,7 @@ ldc_start_done:
>>>  	dr->prod = (dr->prod + 1) & (VNET_TX_RING_SIZE - 1);
>>>  	if (unlikely(vnet_tx_dring_avail(dr) < 1)) {
>>>  		netif_tx_stop_queue(txq);
>>> +		dma_wmb();
>>
>> This does not look right.
>>
>> I believe you need smp_rmb() here.
> 
> Well, it probably should be dma_rmb(), since regardless of the number
> of cores we think we have, we're communicating with a peer ldom that
> has its own core(s).  Either way, on sparc they all seem to boil down
> to the same bit of asm, but using the "rmb" part makes more logical
> sense. I'll respin with dma_rmb().

DMA barriers are for ordering between CPUs and devices.

SMP barriers are for ordering between CPUs, which is your situation
here.

It is completely inappropriate to use DMA barriers in a virutalization
device driver.

Re: [PATCH net-next 5/9] sunvnet: add memory barrier before check for tx enable

[Eric Dumazet]

On Fri, 2017-02-03 at 13:20 -0800, Shannon Nelson wrote:
> On 2/3/2017 9:56 AM, Eric Dumazet wrote:
> > On Fri, 2017-02-03 at 09:42 -0800, Shannon Nelson wrote:
> >> In order to allow the underlying LDC and outstanding memory operations
> >> to potentially catch up with the driver's Tx requests, add a memory
> >> barrier before checking again for available tx descriptors.
> >>
> >> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> >> ---
> >>  drivers/net/ethernet/sun/sunvnet_common.c |    1 +
> >>  1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
> >> index 5d0d386..98e758e 100644
> >> --- a/drivers/net/ethernet/sun/sunvnet_common.c
> >> +++ b/drivers/net/ethernet/sun/sunvnet_common.c
> >> @@ -1467,6 +1467,7 @@ ldc_start_done:
> >>  	dr->prod = (dr->prod + 1) & (VNET_TX_RING_SIZE - 1);
> >>  	if (unlikely(vnet_tx_dring_avail(dr) < 1)) {
> >>  		netif_tx_stop_queue(txq);
> >> +		dma_wmb();
> >
> > This does not look right.
> >
> > I believe you need smp_rmb() here.
> 
> Well, it probably should be dma_rmb(), since regardless of the number of 
> cores we think we have, we're communicating with a peer ldom that has 
> its own core(s).  Either way, on sparc they all seem to boil down to the 
> same bit of asm, but using the "rmb" part makes more logical sense. 
> I'll respin with dma_rmb().
> 

Transmit completion might happen on another cpu, regardless of ldom.

Therefore you need smp_rmb() here ( like mellanox/mlx4/en_tx.c) , or
even smp_mb() as bnx2x does.

dma_rmb() is never used in this context.

Re: [PATCH net-next 5/9] sunvnet: add memory barrier before check for tx enable

[Shannon Nelson]

On 2/3/2017 2:11 PM, Eric Dumazet wrote:
>
> Transmit completion might happen on another cpu, regardless of ldom.
>
> Therefore you need smp_rmb() here ( like mellanox/mlx4/en_tx.c) , or
> even smp_mb() as bnx2x does.
>
> dma_rmb() is never used in this context.
>

In that case, it looks like there are a couple other similar issues in 
this code that need attention, that cropped up when the new dma_*mb() 
interface was added.  I'll see what I can do with those as well.

The comments and code a few lines above, some of DaveM's original driver 
code, seem to dissuade us from the SMP version and originally used the 
bare wmb().  Perhaps the bare rmb() should be used here?  Again, I 
suppose it doesn't matter much as it looks like they all boil down to 
the same bit of asm, at least on sparc, which is all that matters for 
this driver.

Thanks,
sln

[PATCH net-next 6/9] sunvnet: straighten up message event handling logic

[Shannon Nelson]

The use of gotos for handling the incoming events made this code
harder to read and support than it should be.  This patch straightens
out and clears up the logic.

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

diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
index 98e758e..160b35a 100644
--- a/drivers/net/ethernet/sun/sunvnet_common.c
+++ b/drivers/net/ethernet/sun/sunvnet_common.c
@@ -764,41 +764,37 @@ static int vnet_event_napi(struct vnet_port *port, int budget)
 	struct vio_driver_state *vio = &port->vio;
 	int tx_wakeup, err;
 	int npkts = 0;
-	int event = (port->rx_event & LDC_EVENT_RESET);
-
-ldc_ctrl:
-	if (unlikely(event == LDC_EVENT_RESET ||
-		     event == LDC_EVENT_UP)) {
-		vio_link_state_change(vio, event);
-
-		if (event == LDC_EVENT_RESET) {
-			vnet_port_reset(port);
-			vio_port_up(vio);
-
-			/* If the device is running but its tx queue was
-			 * stopped (due to flow control), restart it.
-			 * This is necessary since vnet_port_reset()
-			 * clears the tx drings and thus we may never get
-			 * back a VIO_TYPE_DATA ACK packet - which is
-			 * the normal mechanism to restart the tx queue.
-			 */
-			if (netif_running(dev))
-				maybe_tx_wakeup(port);
-		}
+
+	/* we don't expect any other bits */
+	BUG_ON(port->rx_event & ~(LDC_EVENT_DATA_READY |
+				  LDC_EVENT_RESET |
+				  LDC_EVENT_UP));
+
+	/* RESET takes precedent over any other event */
+	if (port->rx_event & LDC_EVENT_RESET) {
+		vio_link_state_change(vio, LDC_EVENT_RESET);
+		vnet_port_reset(port);
+		vio_port_up(vio);
+
+		/* If the device is running but its tx queue was
+		 * stopped (due to flow control), restart it.
+		 * This is necessary since vnet_port_reset()
+		 * clears the tx drings and thus we may never get
+		 * back a VIO_TYPE_DATA ACK packet - which is
+		 * the normal mechanism to restart the tx queue.
+		 */
+		if (netif_running(dev))
+			maybe_tx_wakeup(port);
+
 		port->rx_event = 0;
 		return 0;
 	}
-	/* We may have multiple LDC events in rx_event. Unroll send_events() */
-	event = (port->rx_event & LDC_EVENT_UP);
-	port->rx_event &= ~(LDC_EVENT_RESET | LDC_EVENT_UP);
-	if (event == LDC_EVENT_UP)
-		goto ldc_ctrl;
-	event = port->rx_event;
-	if (!(event & LDC_EVENT_DATA_READY))
-		return 0;
 
-	/* we dont expect any other bits than RESET, UP, DATA_READY */
-	BUG_ON(event != LDC_EVENT_DATA_READY);
+	if (port->rx_event & LDC_EVENT_UP) {
+		vio_link_state_change(vio, LDC_EVENT_UP);
+		port->rx_event = 0;
+		return 0;
+	}
 
 	err = 0;
 	tx_wakeup = 0;
@@ -821,25 +817,25 @@ ldc_ctrl:
 			pkt->start_idx = vio_dring_next(dr,
 							port->napi_stop_idx);
 			pkt->end_idx = -1;
-			goto napi_resume;
-		}
-		err = ldc_read(vio->lp, &msgbuf, sizeof(msgbuf));
-		if (unlikely(err < 0)) {
-			if (err == -ECONNRESET)
-				vio_conn_reset(vio);
-			break;
+		} else {
+			err = ldc_read(vio->lp, &msgbuf, sizeof(msgbuf));
+			if (unlikely(err < 0)) {
+				if (err == -ECONNRESET)
+					vio_conn_reset(vio);
+				break;
+			}
+			if (err == 0)
+				break;
+			viodbg(DATA, "TAG [%02x:%02x:%04x:%08x]\n",
+			       msgbuf.tag.type,
+			       msgbuf.tag.stype,
+			       msgbuf.tag.stype_env,
+			       msgbuf.tag.sid);
+			err = vio_validate_sid(vio, &msgbuf.tag);
+			if (err < 0)
+				break;
 		}
-		if (err == 0)
-			break;
-		viodbg(DATA, "TAG [%02x:%02x:%04x:%08x]\n",
-		       msgbuf.tag.type,
-		       msgbuf.tag.stype,
-		       msgbuf.tag.stype_env,
-		       msgbuf.tag.sid);
-		err = vio_validate_sid(vio, &msgbuf.tag);
-		if (err < 0)
-			break;
-napi_resume:
+
 		if (likely(msgbuf.tag.type == VIO_TYPE_DATA)) {
 			if (msgbuf.tag.stype == VIO_SUBTYPE_INFO) {
 				if (!sunvnet_port_is_up_common(port)) {
-- 
1.7.1

[PATCH net-next 7/9] sunvnet: remove extra rcu_read_unlocks

[Shannon Nelson]

The RCU read lock is grabbed first thing in sunvnet_start_xmit_common()
so it always needs to be released.  This removes the conditional release
in the dropped packet error path and removes a couple of superfluous
calls in the middle of the code.

Reported-by: Bijan Mottahedeh <bijan.mottahedeh@oracle.com>
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/sun/sunvnet_common.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
index 160b35a..a479a58 100644
--- a/drivers/net/ethernet/sun/sunvnet_common.c
+++ b/drivers/net/ethernet/sun/sunvnet_common.c
@@ -1279,10 +1279,8 @@ int sunvnet_start_xmit_common(struct sk_buff *skb, struct net_device *dev,
 
 	rcu_read_lock();
 	port = vnet_tx_port(skb, dev);
-	if (unlikely(!port)) {
-		rcu_read_unlock();
+	if (unlikely(!port))
 		goto out_dropped;
-	}
 
 	if (skb_is_gso(skb) && skb->len > port->tsolen) {
 		err = vnet_handle_offloads(port, skb, vnet_tx_port);
@@ -1307,7 +1305,6 @@ int sunvnet_start_xmit_common(struct sk_buff *skb, struct net_device *dev,
 			fl4.saddr = ip_hdr(skb)->saddr;
 
 			rt = ip_route_output_key(dev_net(dev), &fl4);
-			rcu_read_unlock();
 			if (!IS_ERR(rt)) {
 				skb_dst_set(skb, &rt->dst);
 				icmp_send(skb, ICMP_DEST_UNREACH,
@@ -1481,8 +1478,7 @@ out_dropped:
 				jiffies + VNET_CLEAN_TIMEOUT);
 	else if (port)
 		del_timer(&port->clean_timer);
-	if (port)
-		rcu_read_unlock();
+	rcu_read_unlock();
 	if (skb)
 		dev_kfree_skb(skb);
 	vnet_free_skbs(freeskbs);
-- 
1.7.1

[PATCH net-next 8/9] ldmvsw: update and simplify version string

[Shannon Nelson]

New version and simplify the print code.

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

diff --git a/drivers/net/ethernet/sun/ldmvsw.c b/drivers/net/ethernet/sun/ldmvsw.c
index 7c5dab4..552c0a9 100644
--- a/drivers/net/ethernet/sun/ldmvsw.c
+++ b/drivers/net/ethernet/sun/ldmvsw.c
@@ -42,11 +42,11 @@
 static u8 vsw_port_hwaddr[ETH_ALEN] = {0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
 
 #define DRV_MODULE_NAME		"ldmvsw"
-#define DRV_MODULE_VERSION	"1.0"
-#define DRV_MODULE_RELDATE	"Jan 15, 2016"
+#define DRV_MODULE_VERSION	"1.1"
+#define DRV_MODULE_RELDATE	"February 3, 2017"
 
 static char version[] =
-	DRV_MODULE_NAME ".c:v" DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")\n";
+	DRV_MODULE_NAME " " DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")";
 MODULE_AUTHOR("Oracle");
 MODULE_DESCRIPTION("Sun4v LDOM Virtual Switch Driver");
 MODULE_LICENSE("GPL");
@@ -320,11 +320,6 @@ static struct vio_driver_ops vsw_vio_ops = {
 	.handshake_complete	= sunvnet_handshake_complete_common,
 };
 
-static void print_version(void)
-{
-	printk_once(KERN_INFO "%s", version);
-}
-
 static const char *remote_macaddr_prop = "remote-mac-address";
 static const char *id_prop = "id";
 
@@ -341,8 +336,6 @@ static int vsw_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 	u64 handle;
 	u64 node;
 
-	print_version();
-
 	hp = mdesc_grab();
 
 	node = vio_vdev_node(hp, vdev);
@@ -527,6 +520,7 @@ static struct vio_driver vsw_port_driver = {
 
 static int __init vsw_init(void)
 {
+	pr_info("%s\n", version);
 	return vio_register_driver(&vsw_port_driver);
 }
 
-- 
1.7.1

[PATCH net-next 9/9] ldmvsw: disable tso and gso for bridge operations

[Shannon Nelson]

The ldmvsw driver is specifically for supporting the ldom virtual
networking by running in the primary ldom and using the LDC to connect
the remaining ldoms to the outside world via a bridge.  With TSO and GSO
supported while connected the bridge, things tend to misbehave as seen in
our case by delayed packets, enough to begin triggering retransmits and
affecting overall throughput.  By turning off advertised support for TSO
and GSO we restore stable traffic flow through the bridge.

Orabug: 23293104

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

diff --git a/drivers/net/ethernet/sun/ldmvsw.c b/drivers/net/ethernet/sun/ldmvsw.c
index 552c0a9..bd2cfbc 100644
--- a/drivers/net/ethernet/sun/ldmvsw.c
+++ b/drivers/net/ethernet/sun/ldmvsw.c
@@ -299,8 +299,7 @@ static struct net_device *vsw_alloc_netdev(u8 hwaddr[],
 	dev->ethtool_ops = &vsw_ethtool_ops;
 	dev->watchdog_timeo = VSW_TX_TIMEOUT;
 
-	dev->hw_features = NETIF_F_TSO | NETIF_F_GSO | NETIF_F_GSO_SOFTWARE |
-			   NETIF_F_HW_CSUM | NETIF_F_SG;
+	dev->hw_features = NETIF_F_HW_CSUM;
 	dev->features = dev->hw_features;
 
 	SET_NETDEV_DEV(dev, &vdev->dev);
@@ -390,7 +389,7 @@ static int vsw_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 	port->vp = vp;
 	port->dev = dev;
 	port->switch_port = 1;
-	port->tso = true;
+	port->tso = false; /* no tso in vsw, misbehaves in bridge */
 	port->tsolen = 0;
 
 	/* Mark the port as belonging to ldmvsw which directs the
diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
index a479a58..3d31c80 100644
--- a/drivers/net/ethernet/sun/sunvnet_common.c
+++ b/drivers/net/ethernet/sun/sunvnet_common.c
@@ -210,6 +210,7 @@ static int handle_attr_info(struct vio_driver_state *vio,
 	} else {
 		pkt->cflags &= ~VNET_LSO_IPV4_CAPAB;
 		pkt->ipv4_lso_maxlen = 0;
+		port->tsolen = 0;
 	}
 
 	/* for version >= 1.6, ACK packet mode we support */
@@ -1685,7 +1686,7 @@ static void vnet_port_reset(struct vnet_port *port)
 	del_timer(&port->clean_timer);
 	sunvnet_port_free_tx_bufs_common(port);
 	port->rmtu = 0;
-	port->tso = true;
+	port->tso = (port->vsw == 0);  /* no tso in vsw, misbehaves in bridge */
 	port->tsolen = 0;
 }
 
-- 
1.7.1

Re: [PATCH net-next 9/9] ldmvsw: disable tso and gso for bridge operations

[Eric Dumazet]

On Fri, 2017-02-03 at 09:42 -0800, Shannon Nelson wrote:
> The ldmvsw driver is specifically for supporting the ldom virtual
> networking by running in the primary ldom and using the LDC to connect
> the remaining ldoms to the outside world via a bridge.  With TSO and GSO
> supported while connected the bridge, things tend to misbehave as seen in
> our case by delayed packets, enough to begin triggering retransmits and
> affecting overall throughput.  By turning off advertised support for TSO
> and GSO we restore stable traffic flow through the bridge.
> 
> Orabug: 23293104
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
>  drivers/net/ethernet/sun/ldmvsw.c         |    5 ++---
>  drivers/net/ethernet/sun/sunvnet_common.c |    3 ++-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sun/ldmvsw.c b/drivers/net/ethernet/sun/ldmvsw.c
> index 552c0a9..bd2cfbc 100644
> --- a/drivers/net/ethernet/sun/ldmvsw.c
> +++ b/drivers/net/ethernet/sun/ldmvsw.c
> @@ -299,8 +299,7 @@ static struct net_device *vsw_alloc_netdev(u8 hwaddr[],
>  	dev->ethtool_ops = &vsw_ethtool_ops;
>  	dev->watchdog_timeo = VSW_TX_TIMEOUT;
>  
> -	dev->hw_features = NETIF_F_TSO | NETIF_F_GSO | NETIF_F_GSO_SOFTWARE |
> -			   NETIF_F_HW_CSUM | NETIF_F_SG;
> +	dev->hw_features = NETIF_F_HW_CSUM;


You also removed NETIF_F_SG

Re: [PATCH net-next 9/9] ldmvsw: disable tso and gso for bridge operations

[Shannon Nelson]

On 2/3/2017 9:59 AM, Eric Dumazet wrote:
> On Fri, 2017-02-03 at 09:42 -0800, Shannon Nelson wrote:
>> The ldmvsw driver is specifically for supporting the ldom virtual
>> networking by running in the primary ldom and using the LDC to connect
>> the remaining ldoms to the outside world via a bridge.  With TSO and GSO
>> supported while connected the bridge, things tend to misbehave as seen in
>> our case by delayed packets, enough to begin triggering retransmits and
>> affecting overall throughput.  By turning off advertised support for TSO
>> and GSO we restore stable traffic flow through the bridge.
>>
>> Orabug: 23293104
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>> ---
>>  drivers/net/ethernet/sun/ldmvsw.c         |    5 ++---
>>  drivers/net/ethernet/sun/sunvnet_common.c |    3 ++-
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/sun/ldmvsw.c b/drivers/net/ethernet/sun/ldmvsw.c
>> index 552c0a9..bd2cfbc 100644
>> --- a/drivers/net/ethernet/sun/ldmvsw.c
>> +++ b/drivers/net/ethernet/sun/ldmvsw.c
>> @@ -299,8 +299,7 @@ static struct net_device *vsw_alloc_netdev(u8 hwaddr[],
>>  	dev->ethtool_ops = &vsw_ethtool_ops;
>>  	dev->watchdog_timeo = VSW_TX_TIMEOUT;
>>
>> -	dev->hw_features = NETIF_F_TSO | NETIF_F_GSO | NETIF_F_GSO_SOFTWARE |
>> -			   NETIF_F_HW_CSUM | NETIF_F_SG;
>> +	dev->hw_features = NETIF_F_HW_CSUM;
>
>
> You also removed NETIF_F_SG

Hmmm - yep, I'll put that back in for the respin.

Thanks,
sln