From: Shannon Nelson <hidden> Date: 2017-02-13 19:00:22
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.
v2:
- changed memory barrier fix to use smp_wmb
- put NETIF_F_SG back into the advertised ldmvsw hw_features
v3:
- the sunvnet_common module doesn't need module_init or _exit
v4:
- dropped the statistics patch
- fixed up "default" tag for SUNVNET_COMMON
Shannon Nelson (7):
sunvnet: make sunvnet common code dynamically loadable
sunvnet: update version and version printing
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 | 8 ++-
drivers/net/ethernet/sun/ldmvsw.c | 19 ++---
drivers/net/ethernet/sun/sunvnet.c | 14 +---
drivers/net/ethernet/sun/sunvnet_common.c | 117 ++++++++++++++---------------
4 files changed, 72 insertions(+), 86 deletions(-)
From: Shannon Nelson <hidden> Date: 2017-02-13 19:00:23
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 <redacted>
---
drivers/net/ethernet/sun/Kconfig | 8 ++++++--
drivers/net/ethernet/sun/sunvnet_common.c | 5 +++++
2 files changed, 11 insertions(+), 2 deletions(-)
@@ -37,6 +37,11 @@*/#define VNET_MAX_RETRIES 10+MODULE_AUTHOR("David S. Miller (davem@davemloft.net)");+MODULE_DESCRIPTION("Sun LDOM virtual network support library");+MODULE_LICENSE("GPL");+MODULE_VERSION("1.1");+staticint__vnet_tx_trigger(structvnet_port*port,u32start);staticvoidvnet_port_reset(structvnet_port*port);
From: Shannon Nelson <hidden> Date: 2017-02-13 19:00:26
From: Sowmini Varadhan <redacted>
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 <redacted>
Signed-off-by: Shannon Nelson <redacted>
---
drivers/net/ethernet/sun/sunvnet_common.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
From: Shannon Nelson <hidden> Date: 2017-02-13 19:00:27
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 <redacted>
---
drivers/net/ethernet/sun/sunvnet.c | 14 ++++----------
1 files changed, 4 insertions(+), 10 deletions(-)
From: Shannon Nelson <hidden> Date: 2017-02-13 19:00:29
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 <redacted>
---
drivers/net/ethernet/sun/sunvnet_common.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
From: Shannon Nelson <hidden> Date: 2017-02-13 19:00:35
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 <redacted>
---
drivers/net/ethernet/sun/sunvnet_common.c | 94 ++++++++++++++---------------
1 files changed, 45 insertions(+), 49 deletions(-)
@@ -738,41 +738,37 @@ static int vnet_event_napi(struct vnet_port *port, int budget)structvio_driver_state*vio=&port->vio;inttx_wakeup,err;intnpkts=0;-intevent=(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(duetoflowcontrol),restartit.-*Thisisnecessarysincevnet_port_reset()-*clearsthetxdringsandthuswemayneverget-*backaVIO_TYPE_DATAACKpacket-whichis-*thenormalmechanismtorestartthetxqueue.-*/-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(duetoflowcontrol),restartit.+*Thisisnecessarysincevnet_port_reset()+*clearsthetxdringsandthuswemayneverget+*backaVIO_TYPE_DATAACKpacket-whichis+*thenormalmechanismtorestartthetxqueue.+*/+if(netif_running(dev))+maybe_tx_wakeup(port);+port->rx_event=0;return0;}-/* 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)-gotoldc_ctrl;-event=port->rx_event;-if(!(event&LDC_EVENT_DATA_READY))-return0;-/* 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;+return0;+}err=0;tx_wakeup=0;
@@ -795,25 +791,25 @@ static int vnet_event_napi(struct vnet_port *port, int budget)pkt->start_idx=vio_dring_next(dr,port->napi_stop_idx);pkt->end_idx=-1;-gotonapi_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)){
From: Shannon Nelson <hidden> Date: 2017-02-13 19:00:48
New version and simplify the print code.
Signed-off-by: Shannon Nelson <redacted>
---
drivers/net/ethernet/sun/ldmvsw.c | 14 ++++----------
1 files changed, 4 insertions(+), 10 deletions(-)
From: Shannon Nelson <hidden> Date: 2017-02-13 19:00:54
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 <redacted>
---
drivers/net/ethernet/sun/ldmvsw.c | 5 ++---
drivers/net/ethernet/sun/sunvnet_common.c | 3 ++-
2 files changed, 4 insertions(+), 4 deletions(-)
@@ -320,7 +319,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
@@ -186,6 +186,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 */
@@ -1635,7 +1636,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;}
From: Shannon Nelson <hidden> Date: 2017-02-13 19:00:55
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 <redacted>
Signed-off-by: Shannon Nelson <redacted>
---
drivers/net/ethernet/sun/sunvnet_common.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)
From: Joe Perches <joe@perches.com> Date: 2017-02-13 19:07:00
On Mon, 2017-02-13 at 10:57 -0800, Shannon Nelson wrote:
quoted hunk
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 <redacted>
---
drivers/net/ethernet/sun/sunvnet_common.c | 94 ++++++++++++++---------------
1 files changed, 45 insertions(+), 49 deletions(-)
From: Shannon Nelson <hidden> Date: 2017-02-13 19:18:28
On 2/13/2017 11:06 AM, Joe Perches wrote:
On Mon, 2017-02-13 at 10:57 -0800, Shannon Nelson wrote:
quoted
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 <redacted>
---
drivers/net/ethernet/sun/sunvnet_common.c | 94 ++++++++++++++---------------
1 files changed, 45 insertions(+), 49 deletions(-)
@@ -738,41 +738,37 @@ static int vnet_event_napi(struct vnet_port *port, int budget)
[]
quoted
+ /* we don't expect any other bits */
+ BUG_ON(port->rx_event & ~(LDC_EVENT_DATA_READY |
+ LDC_EVENT_RESET |
+ LDC_EVENT_UP));
Is it really necessary to use BUG_ON here?
I'm carrying this from the original code because we want to know asap if
we have a low level protocol issue. It should never happen in the
field, but we want to notice it as soon as we can when doing development
and testing. In this patch I've simply made it more obvious and up
front that we're doing this test rather than having it buried in the
logic a few lines further down.
sln
From: David Miller <davem@davemloft.net> Date: 2017-02-14 18:04:47
From: Shannon Nelson <redacted>
Date: Mon, 13 Feb 2017 10:56:56 -0800
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.
v2:
- changed memory barrier fix to use smp_wmb
- put NETIF_F_SG back into the advertised ldmvsw hw_features
v3:
- the sunvnet_common module doesn't need module_init or _exit
v4:
- dropped the statistics patch
- fixed up "default" tag for SUNVNET_COMMON