Michael Ellerman | 1 Sep 2005 03:28
Picon
Gravatar

[PATCH 0/18] Updates & bug fixes for iseries_veth network driver

Hi,

This is a series of patches for the iseries_veth driver. Most of these are
pretty much unchanged since I posted them earlier:
http://www.uwsg.iu.edu/hypermail/linux/kernel/0506.3/1837.html

I've added patches to add sysfs support, and do some further code cleanups.

Please merge if they look ok.

cheers
Michael Ellerman | 1 Sep 2005 03:28
Picon
Gravatar

[PATCH 1/18] iseries_veth: Cleanup error and debug messages

Currently the iseries_veth driver prints the file name and line number in its
error messages. This isn't very useful for most users, so just print
"iseries_veth: message" instead.

 - convert uses of veth_printk() to veth_debug()/veth_error()/veth_info()
 - make terminology consistent, ie. always refer to LPAR not lpar
 - be consistent about printing return codes as %d not %x
 - make format strings fit in 80 columns

Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
---

 drivers/net/iseries_veth.c |   98 +++++++++++++++++++++++----------------------
 1 files changed, 51 insertions(+), 47 deletions(-)

Index: veth-dev2/drivers/net/iseries_veth.c
===================================================================
--- veth-dev2.orig/drivers/net/iseries_veth.c
+++ veth-dev2/drivers/net/iseries_veth.c
 <at>  <at>  -79,6 +79,8  <at>  <at> 
 #include <asm/iommu.h>
 #include <asm/vio.h>

+#undef DEBUG
+
 #include "iseries_veth.h"

 MODULE_AUTHOR("Kyle Lucke <klucke <at> us.ibm.com>");
 <at>  <at>  -176,11 +178,18  <at>  <at>  static void veth_timed_ack(unsigned long
  * Utility functions
(Continue reading)

Michael Ellerman | 1 Sep 2005 03:28
Picon
Gravatar

[PATCH 2/18] iseries_veth: Remove a FIXME WRT deletion of the ack_timer

The iseries_veth driver has a timer which we use to send acks. When the
connection is reset or stopped we need to delete the timer.

Currently we only call del_timer() when resetting a connection, which means
the timer might run again while the connection is being re-setup. As it turns
out that's ok, because the flags the timer consults have been reset.

It's cleaner though to call del_timer_sync() once we've dropped the lock,
although the timer may still run between us dropping the lock and calling
del_timer_sync(), but as above that's ok.

Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
---

 drivers/net/iseries_veth.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

Index: veth-dev2/drivers/net/iseries_veth.c
===================================================================
--- veth-dev2.orig/drivers/net/iseries_veth.c
+++ veth-dev2/drivers/net/iseries_veth.c
 <at>  <at>  -450,13 +450,15  <at>  <at>  static void veth_statemachine(void *p)
 	if (cnx->state & VETH_STATE_RESET) {
 		int i;

-		del_timer(&cnx->ack_timer);
-
 		if (cnx->state & VETH_STATE_OPEN)
 			HvCallEvent_closeLpEventPath(cnx->remote_lp,
 						     HvLpEvent_Type_VirtualLan);
(Continue reading)

Michael Ellerman | 1 Sep 2005 03:29
Picon
Gravatar

[PATCH 13/18] iseries_veth: Fix bogus counting of TX errors

There's a number of problems with the way iseries_veth counts TX errors.

Firstly it counts conditions which aren't really errors as TX errors. This
includes if we don't have a connection struct for the other LPAR, or if the
other LPAR is currently down (or just doesn't want to talk to us). Neither
of these should count as TX errors.

Secondly, it counts one TX error for each LPAR that fails to accept the packet.
This can lead to TX error counts higher than the total number of packets sent
through the interface. This is confusing for users.

This patch fixes that behaviour. The non-error conditions are no longer
counted, and we introduce a new and I think saner meaning to the TX counts.

If a packet is successfully transmitted to any LPAR then it is transmitted
and tx_packets is incremented by 1.

If there is an error transmitting a packet to any LPAR then that is counted
as one error, ie. tx_errors is incremented by 1.

Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
---

 drivers/net/iseries_veth.c |   47 ++++++++++++++++++---------------------------
 1 files changed, 19 insertions(+), 28 deletions(-)

Index: veth-dev2/drivers/net/iseries_veth.c
===================================================================
--- veth-dev2.orig/drivers/net/iseries_veth.c
+++ veth-dev2/drivers/net/iseries_veth.c
(Continue reading)

Michael Ellerman | 1 Sep 2005 03:29
Picon
Gravatar

[PATCH 14/18] iseries_veth: Add sysfs support for connection structs

To aid in field debugging, add sysfs support for iseries_veth's connection
structures. At the moment this is all read-only, however we could think about
adding write support for some attributes in future.

Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
---

 drivers/net/iseries_veth.c |   94 +++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 90 insertions(+), 4 deletions(-)

Index: veth-dev2/drivers/net/iseries_veth.c
===================================================================
--- veth-dev2.orig/drivers/net/iseries_veth.c
+++ veth-dev2/drivers/net/iseries_veth.c
 <at>  <at>  -182,10 +182,6  <at>  <at>  static void veth_release_connection(stru
 static void veth_timed_ack(unsigned long ptr);
 static void veth_timed_reset(unsigned long ptr);

-static struct kobj_type veth_lpar_connection_ktype = {
-	.release	= veth_release_connection
-};
-
 /*
  * Utility functions
  */
 <at>  <at>  -280,6 +276,81  <at>  <at>  static int veth_allocate_events(HvLpInde
 }

 /*
+ * sysfs support
(Continue reading)

Michael Ellerman | 1 Sep 2005 03:29
Picon
Gravatar

[PATCH 9/18] iseries_veth: Use kobjects to track lifecycle of connection structs

The iseries_veth driver can attach to multiple vlans, which correspond to
multiple net devices. However there is only 1 connection between each LPAR,
so the connection structure may be shared by multiple net devices.

This makes module removal messy, because we can't deallocate the connections
until we know there are no net devices still using them. The solution is to
use ref counts on the connections, so we can delete them (actually stop) as
soon as the ref count hits zero.

This patch fixes (part of) a bug we were seeing with IPv6 sending probes to
a dead LPAR, which would then hang us forever due to leftover skbs.

Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
---

 drivers/net/iseries_veth.c |  121 ++++++++++++++++++++++++++++++---------------
 1 files changed, 83 insertions(+), 38 deletions(-)

Index: veth-dev2/drivers/net/iseries_veth.c
===================================================================
--- veth-dev2.orig/drivers/net/iseries_veth.c
+++ veth-dev2/drivers/net/iseries_veth.c
 <at>  <at>  -129,6 +129,7  <at>  <at>  struct veth_lpar_connection {
 	int num_events;
 	struct VethCapData local_caps;

+	struct kobject kobject;
 	struct timer_list ack_timer;

 	spinlock_t lock;
(Continue reading)

Michael Ellerman | 1 Sep 2005 03:29
Picon
Gravatar

[PATCH 6/18] iseries_veth: Replace lock-protected atomic with an ordinary variable

The iseries_veth driver uses atomic ops to manipulate the in_use field of
one of its per-connection structures. However all references to the
flag occur while the connection's lock is held, so the atomic ops aren't
necessary.

Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
---

 drivers/net/iseries_veth.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

Index: veth-dev2/drivers/net/iseries_veth.c
===================================================================
--- veth-dev2.orig/drivers/net/iseries_veth.c
+++ veth-dev2/drivers/net/iseries_veth.c
 <at>  <at>  -117,7 +117,7  <at>  <at>  struct veth_msg {
 	struct veth_msg *next;
 	struct VethFramesData data;
 	int token;
-	unsigned long in_use;
+	int in_use;
 	struct sk_buff *skb;
 	struct device *dev;
 };
 <at>  <at>  -957,6 +957,8  <at>  <at>  static int veth_transmit_to_one(struct s
 		goto drop;
 	}

+	msg->in_use = 1;
+
(Continue reading)

Michael Ellerman | 1 Sep 2005 03:29
Picon
Gravatar

[PATCH 7/18] iseries_veth: Only call dma_unmap_single() if dma_map_single() succeeded

The iseries_veth driver unconditionally calls dma_unmap_single() even
when the corresponding dma_map_single() may have failed.

Rework the code a bit to keep the return value from dma_unmap_single()
around, and then check if it's a dma_mapping_error() before we do
the dma_unmap_single().

Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
---

 drivers/net/iseries_veth.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

Index: veth-dev2/drivers/net/iseries_veth.c
===================================================================
--- veth-dev2.orig/drivers/net/iseries_veth.c
+++ veth-dev2/drivers/net/iseries_veth.c
 <at>  <at>  -931,7 +931,6  <at>  <at>  static int veth_transmit_to_one(struct s
 	struct veth_lpar_connection *cnx = veth_cnx[rlp];
 	struct veth_port *port = (struct veth_port *) dev->priv;
 	HvLpEvent_Rc rc;
-	u32 dma_address, dma_length;
 	struct veth_msg *msg = NULL;
 	int err = 0;
 	unsigned long flags;
 <at>  <at>  -959,20 +958,19  <at>  <at>  static int veth_transmit_to_one(struct s

 	msg->in_use = 1;

-	dma_length = skb->len;
(Continue reading)

Michael Ellerman | 1 Sep 2005 03:29
Picon
Gravatar

[PATCH 5/18] iseries_veth: Remove redundant message stack lock

The iseries_veth driver keeps a stack of messages for each connection
and a lock to protect the stack. However there is also a per-connection lock
which makes the message stack lock redundant.

Remove the message stack lock and document the fact that callers of the
stack-manipulation functions must hold the connection's lock.

Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
---

 drivers/net/iseries_veth.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

Index: veth-dev2/drivers/net/iseries_veth.c
===================================================================
--- veth-dev2.orig/drivers/net/iseries_veth.c
+++ veth-dev2/drivers/net/iseries_veth.c
 <at>  <at>  -143,7 +143,6  <at>  <at>  struct veth_lpar_connection {
 	struct VethCapData remote_caps;
 	u32 ack_timeout;

-	spinlock_t msg_stack_lock;
 	struct veth_msg *msg_stack_head;
 };

 <at>  <at>  -190,27 +189,23  <at>  <at>  static void veth_timed_ack(unsigned long
 #define veth_debug(fmt, args...) do {} while (0)
 #endif

+/* You must hold the connection's lock when you call this function. */
(Continue reading)

Michael Ellerman | 1 Sep 2005 03:29
Picon
Gravatar

[PATCH 4/18] iseries_veth: Fix broken promiscuous handling

Due to a logic bug, once promiscuous mode is enabled in the iseries_veth
driver it is never disabled.

The driver keeps two flags, promiscuous and all_mcast which have exactly the
same effect. This is because we only ever receive packets destined for us,
or multicast packets. So consolidate them into one promiscuous flag for
simplicity.

Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
---

 drivers/net/iseries_veth.c |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

Index: veth-dev2/drivers/net/iseries_veth.c
===================================================================
--- veth-dev2.orig/drivers/net/iseries_veth.c
+++ veth-dev2/drivers/net/iseries_veth.c
 <at>  <at>  -159,7 +159,6  <at>  <at>  struct veth_port {

 	rwlock_t mcast_gate;
 	int promiscuous;
-	int all_mcast;
 	int num_mcast;
 	u64 mcast_addr[VETH_MAX_MCAST];
 };
 <at>  <at>  -756,17 +755,15  <at>  <at>  static void veth_set_multicast_list(stru

 	write_lock_irqsave(&port->mcast_gate, flags);

(Continue reading)


Gmane