* [PATCH] add HAVE_UNREGISTER_NETDEV_QUEUE define for out-of-tree modules
From: Benjamin LaHaise @ 2009-11-10 22:00 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Hello,
I have an out of tree network driver (GPL'd) that would like to make use of
the unregister_netdevice_queue interface for bulk unregister of devices.
Add a #define HAVE_UNREGISTER_NETDEV_QUEUE to allow the source to detect
that the kernel supports this.
-ben
Signed-off-by: Benjamin LaHaise <bcrl@lhnet.ca
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 465add6..f9e8e0e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1117,6 +1117,7 @@ extern int dev_close(struct net_device *dev);
extern void dev_disable_lro(struct net_device *dev);
extern int dev_queue_xmit(struct sk_buff *skb);
extern int register_netdevice(struct net_device *dev);
+#define HAVE_UNREGISTER_NETDEV_QUEUE
extern void unregister_netdevice_queue(struct net_device *dev,
struct list_head *head);
extern void unregister_netdevice_many(struct list_head *head);
^ permalink raw reply related
* Re: Possible bug: SO_TIMESTAMPING 2.6.30+
From: Christopher Zimmermann @ 2009-11-10 22:03 UTC (permalink / raw)
Cc: Marcus D. Leech, netdev
In-Reply-To: <20091110091252.1667d27d@pundit>
On Tue, 10 Nov 2009 09:12:52 +0100
Christopher Zimmermann <madroach@zakweb.de> wrote:
> On Mon, 09 Nov 2009 19:40:30 -0500
> "Marcus D. Leech" <mleech@ripnet.com> wrote:
>
>
> > I know that Patrick Ohly has essentially moved on from doing the
> > SO_TIMESTAMPING stuff, so
> > who's maintaining it now?
>
> I worked on it a month ago or so and have a patchset from Patick Ohly
> and some changes by me which fix software timestamping and make the
> ioctl interface to the hardware more flexible (keeping backwards
> compatibility). Patches are attached.
> Still I never tried IPv6 and don't think the patches do anything about
> it.
> It would be nice to know weather software tx timestamps work
> with/without the patches.
>
>
> Christopher
Have a look at commit 51f31cabe3ce5345b51e4a4f82138b38c4d5dc91.
It adds support for SO_TIMESTAMPING in the transport layer, but only
for the ipv4 variants.
Good news is, it does look like it was simple to do the same to
ipv6/udp.c. Have fun.
ip: support for TX timestamps on UDP and RAW sockets
Instructions for time stamping outgoing packets are take from the
socket layer and later copied into the new skb.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
But only in ipv4/udp.c:
-------------------------------- net/ipv4/udp.c
-------------------------------- index c47c989..4bd178a 100644
@@ -596,6 +596,7 @@ int udp_sendmsg(struct kiocb *iocb, struct sock
*sk, struct msghdr *msg, return -EOPNOTSUPP;
ipc.opt = NULL;
+ ipc.shtx.flags = 0;
if (up->pending) {
/*
@@ -643,6 +644,9 @@ int udp_sendmsg(struct kiocb *iocb, struct sock
*sk, struct msghdr *msg, ipc.addr = inet->saddr;
ipc.oif = sk->sk_bound_dev_if;
+ err = sock_tx_timestamp(msg, sk, &ipc.shtx);
+ if (err)
+ return err;
if (msg->msg_controllen) {
err = ip_cmsg_send(sock_net(sk), msg, &ipc);
if (err)
^ permalink raw reply
* Re: sunrpc port allocation and IANA reserved list
From: Trond Myklebust @ 2009-11-10 22:14 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Chris Friesen, netdev, Linux kernel
In-Reply-To: <1257888720.2834.30.camel@achroite.uk.solarflarecom.com>
On Tue, 2009-11-10 at 21:32 +0000, Ben Hutchings wrote:
> On Tue, 2009-11-10 at 15:06 -0600, Chris Friesen wrote:
> > On 11/10/2009 02:26 PM, Trond Myklebust wrote:
> > > Just use /proc/sys/sunrpc/{max,min}_resvport interface to restrict the
> > > range used to a safer one. That's what it is for...
>
> Unless I'm much mistaken, that only affects in-kernel SunRPC users.
There shouldn't be that many non-kernel sunrpc users that require
privileged ports.
The exceptions I can think of are
* the 'mount' program, if you are using the legacy binary mount
interface. Use text mounts instead...
* rpc.statd might also use privileged ports when booting the
system when it needs to notify servers of the reboot. You can
set that port number using the '-o' option.
Others? Do any of the NIS services require it?
Trond
^ permalink raw reply
* [PATCH 0/4] vbus: venet macvlan support
From: Patrick Mullaney @ 2009-11-10 22:27 UTC (permalink / raw)
To: alacrityvm-devel; +Cc: linux-kernel, kaber, arnd, bridge, evb, netdev
(Applies to alacrityvm.git/master:34534534)
This patchset implements a vbus venet device with a
macvlan backend.
These patches allow an alacrityvm guest to send and receive
directly over a macvlan, avoiding the bridge entirely.
This driver inherits all of the benefits of the work done
to date on vbus/venet driver(SAR offloading, zero-copy
in the guest->host path, configurable tx-complete mitigation,
interrupt coalescing at the vbus level). Some of the work to
re-factor and share the common code between venet-tap and
venet-macvlan was done prior because it should be generally
useful to anyone wanting to implement a venet type of device.
Once the driver is built and installed, you may use it
just like you would a venet-tap device. In order to
instantiate a venet-macvlan, there are just two differences
from the procedure to instantiating a venet-tap. In order
to create the venet-macvlan device, just:
echo venet-macvlan > /config/vbus/devices/<device-name>/type
and
echo "lower-devicename" > /sys/vbus/devices/<device-name>/ll_ifname
where lower-devicename is something like eth0, eth1, eth2 etc.
The second step associates the lower-devicename, usually
a physical device, with the venet-macvlan device being created.
This step must be perform prior to enabling the venet-macvlan
device.
After that, a guest can make use of the venet-macvlan in
exactly the same manner as a venet-tap. In fact, the guest
actually sees venet-tap and venet-macvlan as identical
types of the devices on the vbus.
Using the venet-macvlan driver will reduce some overhead by
eliminating the linux bridge from the send and receive
paths. For a lightly loaded network segment and system,
we have measured this to be aproximately 1-3 us per side
depending on what hardware is involved.
Since this driver layered over the macvlan driver, it will
have that same limitations as the macvlan driver. For example,
forwarding between macvlan devices on the same host is not
supported. This driver targeted toward VEPA environments as
described by the 'Edge Virtual Bridging' working group.
---
Patrick Mullaney (4):
venet-macvlan: add new driver to connect a venet to a macvlan netdevice
venetdev: support common venet netdev routines
macvlan: allow in-kernel modules to create and manage macvlan devices
macvlan: derived from Arnd Bergmann's patch for macvtap
drivers/net/macvlan.c | 105 +++--
drivers/net/vbus-enet.c | 8
include/linux/macvlan.h | 43 ++
include/linux/venet.h | 5
kernel/vbus/devices/venet/Kconfig | 11 +
kernel/vbus/devices/venet/Makefile | 10 -
kernel/vbus/devices/venet/device.c | 53 ++-
kernel/vbus/devices/venet/macvlan.c | 598 +++++++++++++++++++++++++++++++
kernel/vbus/devices/venet/venetdevice.h | 12 +
9 files changed, 785 insertions(+), 60 deletions(-)
create mode 100644 include/linux/macvlan.h
create mode 100644 kernel/vbus/devices/venet/macvlan.c
^ permalink raw reply
* [PATCH 1/4] macvlan: derived from Arnd Bergmann's patch for macvtap
From: Patrick Mullaney @ 2009-11-10 22:27 UTC (permalink / raw)
To: alacrityvm-devel; +Cc: linux-kernel, kaber, arnd, bridge, evb, netdev
In-Reply-To: <20091110222632.24100.14884.stgit@mimic.site>
This is in the series because this has not gone upstream yet and
the subsequent patches depend on it. This patch includes only the
basic framework for overriding the receive path and the macvlan header
was moved to allow modules outside of driver/net to use it.
Signed-off-by: Patrick Mullaney <pmullaney@novell.com>
---
drivers/net/macvlan.c | 39 +++++++++++++++------------------------
include/linux/macvlan.h | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+), 24 deletions(-)
create mode 100644 include/linux/macvlan.h
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 99eed9f..0a389b8 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -30,22 +30,7 @@
#include <linux/if_macvlan.h>
#include <net/rtnetlink.h>
-#define MACVLAN_HASH_SIZE (1 << BITS_PER_BYTE)
-
-struct macvlan_port {
- struct net_device *dev;
- struct hlist_head vlan_hash[MACVLAN_HASH_SIZE];
- struct list_head vlans;
-};
-
-struct macvlan_dev {
- struct net_device *dev;
- struct list_head list;
- struct hlist_node hlist;
- struct macvlan_port *port;
- struct net_device *lowerdev;
-};
-
+#include <linux/macvlan.h>
static struct macvlan_dev *macvlan_hash_lookup(const struct macvlan_port *port,
const unsigned char *addr)
@@ -135,7 +120,7 @@ static void macvlan_broadcast(struct sk_buff *skb,
else
nskb->pkt_type = PACKET_MULTICAST;
- netif_rx(nskb);
+ vlan->receive(nskb);
}
}
}
@@ -180,11 +165,11 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
skb->dev = dev;
skb->pkt_type = PACKET_HOST;
- netif_rx(skb);
+ vlan->receive(skb);
return NULL;
}
-static int macvlan_start_xmit(struct sk_buff *skb, struct net_device *dev)
+int macvlan_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
const struct macvlan_dev *vlan = netdev_priv(dev);
unsigned int len = skb->len;
@@ -202,6 +187,7 @@ static int macvlan_start_xmit(struct sk_buff *skb, struct net_device *dev)
}
return NETDEV_TX_OK;
}
+EXPORT_SYMBOL_GPL(macvlan_start_xmit);
static int macvlan_hard_header(struct sk_buff *skb, struct net_device *dev,
unsigned short type, const void *daddr,
@@ -412,7 +398,7 @@ static const struct net_device_ops macvlan_netdev_ops = {
.ndo_validate_addr = eth_validate_addr,
};
-static void macvlan_setup(struct net_device *dev)
+void macvlan_setup(struct net_device *dev)
{
ether_setup(dev);
@@ -423,6 +409,7 @@ static void macvlan_setup(struct net_device *dev)
dev->ethtool_ops = &macvlan_ethtool_ops;
dev->tx_queue_len = 0;
}
+EXPORT_SYMBOL_GPL(macvlan_setup);
static int macvlan_port_create(struct net_device *dev)
{
@@ -472,7 +459,7 @@ static void macvlan_transfer_operstate(struct net_device *dev)
}
}
-static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
+int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
{
if (tb[IFLA_ADDRESS]) {
if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN)
@@ -482,9 +469,10 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
}
return 0;
}
+EXPORT_SYMBOL_GPL(macvlan_validate);
-static int macvlan_newlink(struct net_device *dev,
- struct nlattr *tb[], struct nlattr *data[])
+int macvlan_newlink(struct net_device *dev,
+ struct nlattr *tb[], struct nlattr *data[])
{
struct macvlan_dev *vlan = netdev_priv(dev);
struct macvlan_port *port;
@@ -524,6 +512,7 @@ static int macvlan_newlink(struct net_device *dev,
vlan->lowerdev = lowerdev;
vlan->dev = dev;
vlan->port = port;
+ vlan->receive = netif_rx;
err = register_netdevice(dev);
if (err < 0)
@@ -533,8 +522,9 @@ static int macvlan_newlink(struct net_device *dev,
macvlan_transfer_operstate(dev);
return 0;
}
+EXPORT_SYMBOL_GPL(macvlan_newlink);
-static void macvlan_dellink(struct net_device *dev)
+void macvlan_dellink(struct net_device *dev)
{
struct macvlan_dev *vlan = netdev_priv(dev);
struct macvlan_port *port = vlan->port;
@@ -545,6 +535,7 @@ static void macvlan_dellink(struct net_device *dev)
if (list_empty(&port->vlans))
macvlan_port_destroy(port->dev);
}
+EXPORT_SYMBOL_GPL(macvlan_dellink);
static struct rtnl_link_ops macvlan_link_ops __read_mostly = {
.kind = "macvlan",
diff --git a/include/linux/macvlan.h b/include/linux/macvlan.h
new file mode 100644
index 0000000..3f3c6c3
--- /dev/null
+++ b/include/linux/macvlan.h
@@ -0,0 +1,37 @@
+#ifndef _MACVLAN_H
+#define _MACVLAN_H
+
+#include <linux/netdevice.h>
+#include <linux/netlink.h>
+#include <linux/list.h>
+
+#define MACVLAN_HASH_SIZE (1 << BITS_PER_BYTE)
+
+struct macvlan_port {
+ struct net_device *dev;
+ struct hlist_head vlan_hash[MACVLAN_HASH_SIZE];
+ struct list_head vlans;
+};
+
+struct macvlan_dev {
+ struct net_device *dev;
+ struct list_head list;
+ struct hlist_node hlist;
+ struct macvlan_port *port;
+ struct net_device *lowerdev;
+
+ int (*receive)(struct sk_buff *skb);
+};
+
+extern int macvlan_start_xmit(struct sk_buff *skb, struct net_device *dev);
+
+extern void macvlan_setup(struct net_device *dev);
+
+extern int macvlan_validate(struct nlattr *tb[], struct nlattr *data[]);
+
+extern int macvlan_newlink(struct net_device *dev,
+ struct nlattr *tb[], struct nlattr *data[]);
+
+extern void macvlan_dellink(struct net_device *dev);
+
+#endif /* _MACVLAN_H */
^ permalink raw reply related
* [PATCH 2/4] macvlan: allow in-kernel modules to create and manage macvlan devices
From: Patrick Mullaney @ 2009-11-10 22:27 UTC (permalink / raw)
To: alacrityvm-devel; +Cc: linux-kernel, kaber, arnd, bridge, evb, netdev
In-Reply-To: <20091110222632.24100.14884.stgit@mimic.site>
The macvlan driver didn't allow for creation/deletion of devices
by other in-kernel modules. This patch provides common routines
for both in-kernel and netlink based management. This patch
also enables macvlan device support for gro for lower level
devices that support gro.
Signed-off-by: Patrick Mullaney <pmullaney@novell.com>
---
drivers/net/macvlan.c | 72 +++++++++++++++++++++++++++++++----------------
include/linux/macvlan.h | 6 ++++
2 files changed, 53 insertions(+), 25 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 0a389b8..6b98b26 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -208,7 +208,7 @@ static const struct header_ops macvlan_hard_header_ops = {
.cache_update = eth_header_cache_update,
};
-static int macvlan_open(struct net_device *dev)
+int macvlan_open(struct net_device *dev)
{
struct macvlan_dev *vlan = netdev_priv(dev);
struct net_device *lowerdev = vlan->lowerdev;
@@ -235,7 +235,7 @@ out:
return err;
}
-static int macvlan_stop(struct net_device *dev)
+int macvlan_stop(struct net_device *dev)
{
struct macvlan_dev *vlan = netdev_priv(dev);
struct net_device *lowerdev = vlan->lowerdev;
@@ -316,7 +316,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key;
#define MACVLAN_FEATURES \
(NETIF_F_SG | NETIF_F_ALL_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \
NETIF_F_GSO | NETIF_F_TSO | NETIF_F_UFO | NETIF_F_GSO_ROBUST | \
- NETIF_F_TSO_ECN | NETIF_F_TSO6)
+ NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO)
#define MACVLAN_STATE_MASK \
((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT))
@@ -440,7 +440,7 @@ static void macvlan_port_destroy(struct net_device *dev)
kfree(port);
}
-static void macvlan_transfer_operstate(struct net_device *dev)
+void macvlan_transfer_operstate(struct net_device *dev)
{
struct macvlan_dev *vlan = netdev_priv(dev);
const struct net_device *lowerdev = vlan->lowerdev;
@@ -458,6 +458,7 @@ static void macvlan_transfer_operstate(struct net_device *dev)
netif_carrier_off(dev);
}
}
+EXPORT_SYMBOL_GPL(macvlan_transfer_operstate);
int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
{
@@ -471,11 +472,47 @@ int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
}
EXPORT_SYMBOL_GPL(macvlan_validate);
-int macvlan_newlink(struct net_device *dev,
- struct nlattr *tb[], struct nlattr *data[])
+int macvlan_link_lowerdev(struct net_device *dev,
+ struct net_device *lowerdev)
{
struct macvlan_dev *vlan = netdev_priv(dev);
struct macvlan_port *port;
+ int err = 0;
+
+ if (lowerdev->macvlan_port == NULL) {
+ err = macvlan_port_create(lowerdev);
+ if (err < 0)
+ return err;
+ }
+ port = lowerdev->macvlan_port;
+
+ vlan->lowerdev = lowerdev;
+ vlan->dev = dev;
+ vlan->port = port;
+ vlan->receive = netif_rx;
+
+ macvlan_init(dev);
+
+ list_add_tail(&vlan->list, &port->vlans);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(macvlan_link_lowerdev);
+
+void macvlan_unlink_lowerdev(struct net_device *dev)
+{
+ struct macvlan_dev *vlan = netdev_priv(dev);
+ struct macvlan_port *port = vlan->port;
+
+ list_del(&vlan->list);
+
+ if (list_empty(&port->vlans))
+ macvlan_port_destroy(port->dev);
+}
+EXPORT_SYMBOL_GPL(macvlan_unlink_lowerdev);
+
+int macvlan_newlink(struct net_device *dev,
+ struct nlattr *tb[], struct nlattr *data[])
+{
struct net_device *lowerdev;
int err;
@@ -502,23 +539,14 @@ int macvlan_newlink(struct net_device *dev,
if (!tb[IFLA_ADDRESS])
random_ether_addr(dev->dev_addr);
- if (lowerdev->macvlan_port == NULL) {
- err = macvlan_port_create(lowerdev);
- if (err < 0)
- return err;
- }
- port = lowerdev->macvlan_port;
-
- vlan->lowerdev = lowerdev;
- vlan->dev = dev;
- vlan->port = port;
- vlan->receive = netif_rx;
+ err = macvlan_link_lowerdev(dev, lowerdev);
+ if (err < 0)
+ return err;
err = register_netdevice(dev);
if (err < 0)
return err;
- list_add_tail(&vlan->list, &port->vlans);
macvlan_transfer_operstate(dev);
return 0;
}
@@ -526,14 +554,8 @@ EXPORT_SYMBOL_GPL(macvlan_newlink);
void macvlan_dellink(struct net_device *dev)
{
- struct macvlan_dev *vlan = netdev_priv(dev);
- struct macvlan_port *port = vlan->port;
-
- list_del(&vlan->list);
+ macvlan_unlink_lowerdev(dev);
unregister_netdevice(dev);
-
- if (list_empty(&port->vlans))
- macvlan_port_destroy(port->dev);
}
EXPORT_SYMBOL_GPL(macvlan_dellink);
diff --git a/include/linux/macvlan.h b/include/linux/macvlan.h
index 3f3c6c3..cf8738a 100644
--- a/include/linux/macvlan.h
+++ b/include/linux/macvlan.h
@@ -24,6 +24,12 @@ struct macvlan_dev {
};
extern int macvlan_start_xmit(struct sk_buff *skb, struct net_device *dev);
+extern int macvlan_link_lowerdev(struct net_device *dev,
+ struct net_device *lowerdev);
+
+extern void macvlan_unlink_lowerdev(struct net_device *dev);
+
+extern void macvlan_transfer_operstate(struct net_device *dev);
extern void macvlan_setup(struct net_device *dev);
^ permalink raw reply related
* [PATCH 3/4] venetdev: support common venet netdev routines
From: Patrick Mullaney @ 2009-11-10 22:28 UTC (permalink / raw)
To: alacrityvm-devel; +Cc: linux-kernel, kaber, arnd, bridge, evb, netdev
In-Reply-To: <20091110222632.24100.14884.stgit@mimic.site>
This patch breaks out common netdev routines that allow
a device to pass venetdev pointer as opposed to assuming
it is the priv member of the netdevice.
Signed-off-by: Patrick Mullaney <pmullaney@novell.com>
---
kernel/vbus/devices/venet/device.c | 43 ++++++++++++++++++++++++++-----
kernel/vbus/devices/venet/venetdevice.h | 5 ++++
2 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/kernel/vbus/devices/venet/device.c b/kernel/vbus/devices/venet/device.c
index d49ba7f..9fd94ca 100644
--- a/kernel/vbus/devices/venet/device.c
+++ b/kernel/vbus/devices/venet/device.c
@@ -228,9 +228,8 @@ venetdev_txq_notify_dec(struct venetdev *priv)
*/
int
-venetdev_netdev_open(struct net_device *dev)
+venetdev_open(struct venetdev *priv)
{
- struct venetdev *priv = netdev_priv(dev);
unsigned long flags;
BUG_ON(priv->netif.link);
@@ -260,7 +259,7 @@ venetdev_netdev_open(struct net_device *dev)
priv->netif.link = true;
if (!priv->vbus.link)
- netif_carrier_off(dev);
+ netif_carrier_off(priv->netif.dev);
spin_unlock_irqrestore(&priv->lock, flags);
@@ -268,9 +267,16 @@ venetdev_netdev_open(struct net_device *dev)
}
int
-venetdev_netdev_stop(struct net_device *dev)
+venetdev_netdev_open(struct net_device *dev)
{
struct venetdev *priv = netdev_priv(dev);
+
+ return venetdev_open(priv);
+}
+
+int
+venetdev_stop(struct venetdev *priv)
+{
unsigned long flags;
int needs_stop = false;
@@ -296,6 +302,14 @@ venetdev_netdev_stop(struct net_device *dev)
return 0;
}
+int
+venetdev_netdev_stop(struct net_device *dev)
+{
+ struct venetdev *priv = netdev_priv(dev);
+
+ return venetdev_stop(priv);
+}
+
/*
* Configuration changes (passed on by ifconfig)
*/
@@ -1541,10 +1555,10 @@ venetdev_apply_backpressure(struct venetdev *priv)
* the netif flow control is still managed by the actual consumer,
* thereby avoiding the creation of an extra servo-loop to the equation.
*/
+
int
-venetdev_netdev_tx(struct sk_buff *skb, struct net_device *dev)
+venetdev_xmit(struct sk_buff *skb, struct venetdev *priv)
{
- struct venetdev *priv = netdev_priv(dev);
struct ioq *ioq = NULL;
unsigned long flags;
@@ -1585,6 +1599,15 @@ flowcontrol:
return NETDEV_TX_BUSY;
}
+int
+venetdev_netdev_tx(struct sk_buff *skb, struct net_device *dev)
+{
+ struct venetdev *priv = netdev_priv(dev);
+
+ return venetdev_xmit(skb, priv);
+}
+
+
/*
* Ioctl commands
*/
@@ -1599,10 +1622,16 @@ venetdev_netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
* Return statistics to the caller
*/
struct net_device_stats *
+venetdev_get_stats(struct venetdev *priv)
+{
+ return &priv->netif.stats;
+}
+
+struct net_device_stats *
venetdev_netdev_stats(struct net_device *dev)
{
struct venetdev *priv = netdev_priv(dev);
- return &priv->netif.stats;
+ return venetdev_get_stats(priv);
}
/*
diff --git a/kernel/vbus/devices/venet/venetdevice.h b/kernel/vbus/devices/venet/venetdevice.h
index 9a60a2e..71c9f0f 100644
--- a/kernel/vbus/devices/venet/venetdevice.h
+++ b/kernel/vbus/devices/venet/venetdevice.h
@@ -142,6 +142,11 @@ int venetdev_netdev_ioctl(struct net_device *dev, struct ifreq *rq,
int cmd);
struct net_device_stats *venetdev_netdev_stats(struct net_device *dev);
+int venetdev_open(struct venetdev *dev);
+int venetdev_stop(struct venetdev *dev);
+int venetdev_xmit(struct sk_buff *skb, struct venetdev *dev);
+struct net_device_stats *venetdev_get_stats(struct venetdev *dev);
+
static inline void venetdev_netdev_unregister(struct venetdev *priv)
{
if (priv->netif.enabled) {
^ permalink raw reply related
* [PATCH 4/4] venet-macvlan: add new driver to connect a venet to a macvlan netdevice
From: Patrick Mullaney @ 2009-11-10 22:28 UTC (permalink / raw)
To: alacrityvm-devel; +Cc: linux-kernel, kaber, arnd, bridge, evb, netdev
In-Reply-To: <20091110222632.24100.14884.stgit@mimic.site>
This driver implements a macvlan device as a venet device that can
be connected to vbus. Since it is a macvlan device, it provides
a more direct path to the underlying adapter by avoiding the
bridge.
Signed-off-by: Patrick Mullaney <pmullaney@novell.com>
---
drivers/net/vbus-enet.c | 8
include/linux/venet.h | 5
kernel/vbus/devices/venet/Kconfig | 11 +
kernel/vbus/devices/venet/Makefile | 10 -
kernel/vbus/devices/venet/device.c | 10 -
kernel/vbus/devices/venet/macvlan.c | 598 +++++++++++++++++++++++++++++++
kernel/vbus/devices/venet/venetdevice.h | 7
7 files changed, 642 insertions(+), 7 deletions(-)
create mode 100644 kernel/vbus/devices/venet/macvlan.c
diff --git a/drivers/net/vbus-enet.c b/drivers/net/vbus-enet.c
index 29b388f..9985020 100644
--- a/drivers/net/vbus-enet.c
+++ b/drivers/net/vbus-enet.c
@@ -832,6 +832,14 @@ vbus_enet_tx_start(struct sk_buff *skb, struct net_device *dev)
vsg->cookie = (u64)(unsigned long)skb;
vsg->len = skb->len;
+ vsg->phdr.transport = skb_transport_header(skb) - skb->head;
+ vsg->phdr.network = skb_network_header(skb) - skb->head;
+
+ if (skb_mac_header_was_set(skb))
+ vsg->phdr.mac = skb_mac_header(skb) - skb->head;
+ else
+ vsg->phdr.mac = ~0U;
+
if (skb->ip_summed == CHECKSUM_PARTIAL) {
vsg->flags |= VENET_SG_FLAG_NEEDS_CSUM;
vsg->csum.start = skb->csum_start - skb_headroom(skb);
diff --git a/include/linux/venet.h b/include/linux/venet.h
index 0578d79..4e5fdf4 100644
--- a/include/linux/venet.h
+++ b/include/linux/venet.h
@@ -78,6 +78,11 @@ struct venet_sg {
__u16 hdrlen;
__u16 size;
} gso;
+ struct {
+ __u32 mac; /* mac offset */
+ __u32 network; /* network offset */
+ __u32 transport; /* transport offset */
+ } phdr;
__u32 count; /* nr of iovs */
struct venet_iov iov[1];
};
diff --git a/kernel/vbus/devices/venet/Kconfig b/kernel/vbus/devices/venet/Kconfig
index 4f89afb..c3b1ac6 100644
--- a/kernel/vbus/devices/venet/Kconfig
+++ b/kernel/vbus/devices/venet/Kconfig
@@ -20,3 +20,14 @@ config VBUS_VENETTAP
If unsure, say N
+config VBUS_VENETMACV
+ tristate "Virtual-Bus Ethernet MACVLAN Device"
+ depends on VBUS_DEVICES && MACVLAN
+ select VBUS_VENETDEV
+ default n
+ help
+ Provides a vbus based virtual ethernet adapter with a macvlan
+ device as its backend.
+
+ If unsure, say N
+
diff --git a/kernel/vbus/devices/venet/Makefile b/kernel/vbus/devices/venet/Makefile
index 185d825..5bf7cb4 100644
--- a/kernel/vbus/devices/venet/Makefile
+++ b/kernel/vbus/devices/venet/Makefile
@@ -1,7 +1,7 @@
-venet-device-objs += device.o
-ifneq ($(CONFIG_VBUS_VENETTAP),n)
-venet-device-objs += tap.o
-endif
+venet-tap-objs := device.o tap.o
+venet-macvlan-objs := device.o macvlan.o
+
+obj-$(CONFIG_VBUS_VENETTAP) += venet-tap.o
+obj-$(CONFIG_VBUS_VENETMACV) += venet-macvlan.o
-obj-$(CONFIG_VBUS_VENETDEV) += venet-device.o
diff --git a/kernel/vbus/devices/venet/device.c b/kernel/vbus/devices/venet/device.c
index 9fd94ca..a30df94 100644
--- a/kernel/vbus/devices/venet/device.c
+++ b/kernel/vbus/devices/venet/device.c
@@ -776,6 +776,12 @@ venetdev_sg_import(struct venetdev *priv, void *ptr, int len)
return NULL;
}
+ if (vsg->phdr.mac != ~0U)
+ skb_set_mac_header(skb, vsg->phdr.mac);
+
+ skb_set_network_header(skb, vsg->phdr.network);
+ skb_set_transport_header(skb, vsg->phdr.transport);
+
if (vsg->flags & VENET_SG_FLAG_GSO) {
struct skb_shared_info *sinfo = skb_shinfo(skb);
@@ -2250,7 +2256,7 @@ host_mac_show(struct vbus_device *dev, struct vbus_device_attribute *attr,
struct vbus_device_attribute attr_hmac =
__ATTR_RO(host_mac);
-static ssize_t
+ssize_t
cmac_store(struct vbus_device *dev, struct vbus_device_attribute *attr,
const char *buf, size_t count)
{
@@ -2282,7 +2288,7 @@ cmac_store(struct vbus_device *dev, struct vbus_device_attribute *attr,
return count;
}
-static ssize_t
+ssize_t
client_mac_show(struct vbus_device *dev, struct vbus_device_attribute *attr,
char *buf)
{
diff --git a/kernel/vbus/devices/venet/macvlan.c b/kernel/vbus/devices/venet/macvlan.c
new file mode 100644
index 0000000..8724e26
--- /dev/null
+++ b/kernel/vbus/devices/venet/macvlan.c
@@ -0,0 +1,598 @@
+/*
+ * venet-macvlan - A Vbus based 802.x virtual network device that utilizes
+ * a macvlan device as the backend
+ *
+ * Copyright (C) 2009 Novell, Patrick Mullaney <pmullaney@novell.com>
+ *
+ * Based on the venet-tap driver from Gregory Haskins
+ *
+ * This file is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/moduleparam.h>
+
+#include <linux/sched.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/types.h>
+#include <linux/interrupt.h>
+#include <linux/wait.h>
+
+#include <linux/in.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/ip.h>
+#include <linux/tcp.h>
+#include <linux/skbuff.h>
+#include <linux/ioq.h>
+#include <linux/vbus.h>
+#include <linux/freezer.h>
+#include <linux/kthread.h>
+#include <linux/ktime.h>
+#include <linux/macvlan.h>
+
+#include "venetdevice.h"
+
+#include <linux/in6.h>
+#include <asm/checksum.h>
+
+MODULE_AUTHOR("Patrick Mullaney");
+MODULE_LICENSE("GPL");
+
+#undef PDEBUG /* undef it, just in case */
+#ifdef VENETMACVLAN_DEBUG
+# define PDEBUG(fmt, args...) printk(KERN_DEBUG "venet-tap: " fmt, ## args)
+#else
+# define PDEBUG(fmt, args...) /* not debugging: nothing */
+#endif
+
+struct venetmacv {
+ struct macvlan_dev mdev;
+ unsigned char ll_ifname[IFNAMSIZ];
+ struct venetdev dev;
+ const struct net_device_ops *macvlan_netdev_ops;
+};
+
+static inline struct venetmacv *conn_to_macv(struct vbus_connection *conn)
+{
+ return container_of(conn, struct venetmacv, dev.vbus.conn);
+}
+
+static inline
+struct venetmacv *venetdev_to_macv(struct venetdev *vdev)
+{
+ return container_of(vdev, struct venetmacv, dev);
+}
+
+static inline
+struct venetmacv *vbusintf_to_macv(struct vbus_device_interface *intf)
+{
+ return container_of(intf, struct venetmacv, dev.vbus.intf);
+}
+
+static inline
+struct venetmacv *vbusdev_to_macv(struct vbus_device *vdev)
+{
+ return container_of(vdev, struct venetmacv, dev.vbus.dev);
+}
+
+int
+venetmacv_tx(struct sk_buff *skb, struct net_device *dev)
+{
+ struct venetmacv *priv = netdev_priv(dev);
+
+ return venetdev_xmit(skb, &priv->dev);
+}
+
+static int venetmacv_receive(struct sk_buff *skb)
+{
+ struct venetmacv *priv = netdev_priv(skb->dev);
+ int err;
+
+ if (netif_queue_stopped(skb->dev)) {
+ PDEBUG("venetmacv_receive: queue congested - dropping..\n");
+ priv->dev.netif.stats.tx_dropped++;
+ return NET_RX_DROP;
+ }
+ err = skb_linearize(skb);
+ if (unlikely(err)) {
+ printk(KERN_WARNING "venetmacv_receive: linearize failure\n");
+ kfree_skb(skb);
+ return -1;
+ }
+ skb_push(skb, ETH_HLEN);
+ return venetmacv_tx(skb, skb->dev);
+}
+
+static void
+venetmacv_vlink_release(struct vbus_connection *conn)
+{
+ struct venetmacv *macv = conn_to_macv(conn);
+ macvlan_unlink_lowerdev(macv->mdev.dev);
+ venetdev_vlink_release(conn);
+}
+
+static void
+venetmacv_vlink_up(struct venetdev *vdev)
+{
+ struct venetmacv *macv = venetdev_to_macv(vdev);
+ int ret;
+
+ if (vdev->netif.link) {
+ rtnl_lock();
+ ret = macv->macvlan_netdev_ops->ndo_open(vdev->netif.dev);
+ rtnl_unlock();
+ if (ret)
+ printk(KERN_ERR "macvlanopen failed %d!\n", ret);
+ }
+}
+
+static void
+venetmacv_vlink_down(struct venetdev *vdev)
+{
+ struct venetmacv *macv = venetdev_to_macv(vdev);
+ int ret;
+
+ if (vdev->netif.link) {
+ rtnl_lock();
+ ret = macv->macvlan_netdev_ops->ndo_stop(vdev->netif.dev);
+ rtnl_unlock();
+ if (ret)
+ printk(KERN_ERR "macvlan close failed %d!\n", ret);
+ }
+}
+
+int
+venetmacv_vlink_call(struct vbus_connection *conn,
+ unsigned long func,
+ void *data,
+ unsigned long len,
+ unsigned long flags)
+{
+ struct venetdev *priv = conn_to_priv(conn);
+ int ret;
+
+ switch (func) {
+ case VENET_FUNC_LINKUP:
+ venetmacv_vlink_up(priv);
+ break;
+ case VENET_FUNC_LINKDOWN:
+ venetmacv_vlink_down(priv);
+ break;
+ }
+ ret = venetdev_vlink_call(conn, func, data, len, flags);
+ return ret;
+}
+
+static struct vbus_connection_ops venetmacv_vbus_link_ops = {
+ .call = venetmacv_vlink_call,
+ .shm = venetdev_vlink_shm,
+ .close = venetdev_vlink_close,
+ .release = venetmacv_vlink_release,
+};
+
+/*
+ * This is called whenever a driver wants to open our device_interface
+ * for communication. The connection is represented by a
+ * vbus_connection object. It is up to the implementation to decide
+ * if it allows more than one connection at a time. This simple example
+ * does not.
+ */
+
+static int
+venetmacv_intf_connect(struct vbus_device_interface *intf,
+ struct vbus_memctx *ctx,
+ int version,
+ struct vbus_connection **conn)
+{
+ struct venetmacv *macv = vbusintf_to_macv(intf);
+ unsigned long flags;
+ int ret;
+
+ PDEBUG("connect\n");
+
+ if (version != VENET_VERSION)
+ return -EINVAL;
+
+ spin_lock_irqsave(&macv->dev.lock, flags);
+
+ /*
+ * We only allow one connection to this device
+ */
+ if (macv->dev.vbus.opened) {
+ spin_unlock_irqrestore(&macv->dev.lock, flags);
+ return -EBUSY;
+ }
+
+ kobject_get(intf->dev->kobj);
+
+ vbus_connection_init(&macv->dev.vbus.conn, &venetmacv_vbus_link_ops);
+
+ macv->dev.vbus.opened = true;
+ macv->dev.vbus.ctx = ctx;
+
+ vbus_memctx_get(ctx);
+
+ if (!macv->mdev.lowerdev)
+ return -ENXIO;
+
+ ret = macvlan_link_lowerdev(macv->mdev.dev, macv->mdev.lowerdev);
+
+ if (ret) {
+ printk(KERN_ERR "macvlan_link_lowerdev: failed\n");
+ return -ENXIO;
+ }
+
+ macvlan_transfer_operstate(macv->mdev.dev);
+
+ macv->mdev.receive = venetmacv_receive;
+
+ spin_unlock_irqrestore(&macv->dev.lock, flags);
+
+ *conn = &macv->dev.vbus.conn;
+
+ return 0;
+}
+
+static void
+venetmacv_intf_release(struct vbus_device_interface *intf)
+{
+ kobject_put(intf->dev->kobj);
+}
+
+static struct vbus_device_interface_ops venetmacv_device_interface_ops = {
+ .connect = venetmacv_intf_connect,
+ .release = venetmacv_intf_release,
+};
+
+/*
+ * This is called whenever the admin creates a symbolic link between
+ * a bus in /config/vbus/buses and our device. It represents a bus
+ * connection. Your device can chose to allow more than one bus to
+ * connect, or it can restrict it to one bus. It can also choose to
+ * register one or more device_interfaces on each bus that it
+ * successfully connects to.
+ *
+ * This example device only registers a single interface
+ */
+static int
+venetmacv_device_bus_connect(struct vbus_device *dev, struct vbus *vbus)
+{
+ struct venetdev *priv = vdev_to_priv(dev);
+ struct vbus_device_interface *intf = &priv->vbus.intf;
+
+ /* We only allow one bus to connect */
+ if (priv->vbus.connected)
+ return -EBUSY;
+
+ kobject_get(dev->kobj);
+
+ intf->name = "default";
+ intf->type = VENET_TYPE;
+ intf->ops = &venetmacv_device_interface_ops;
+
+ priv->vbus.connected = true;
+
+ /*
+ * Our example only registers one interface. If you need
+ * more, simply call interface_register() multiple times
+ */
+ return vbus_device_interface_register(dev, vbus, intf);
+}
+
+/*
+ * This is called whenever the admin removes the symbolic link between
+ * a bus in /config/vbus/buses and our device.
+ */
+static int
+venetmacv_device_bus_disconnect(struct vbus_device *dev, struct vbus *vbus)
+{
+ struct venetdev *priv = vdev_to_priv(dev);
+ struct vbus_device_interface *intf = &priv->vbus.intf;
+
+ if (!priv->vbus.connected)
+ return -EINVAL;
+
+ vbus_device_interface_unregister(intf);
+
+ priv->vbus.connected = false;
+ kobject_put(dev->kobj);
+
+ return 0;
+}
+
+static void
+venetmacv_device_release(struct vbus_device *dev)
+{
+ struct venetmacv *macv = vbusdev_to_macv(dev);
+
+ if (macv->mdev.lowerdev)
+ dev_put(macv->mdev.lowerdev);
+
+ venetdev_netdev_unregister(&macv->dev);
+ free_netdev(macv->mdev.dev);
+}
+
+
+static struct vbus_device_ops venetmacv_device_ops = {
+ .bus_connect = venetmacv_device_bus_connect,
+ .bus_disconnect = venetmacv_device_bus_disconnect,
+ .release = venetmacv_device_release,
+};
+
+#define VENETMACV_TYPE "venet-macvlan"
+static ssize_t
+ll_ifname_store(struct vbus_device *dev, struct vbus_device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct venetmacv *priv = vbusdev_to_macv(dev);
+ size_t len;
+
+ len = strlen(buf);
+
+ if (len >= IFNAMSIZ)
+ return -EINVAL;
+
+ if (priv->dev.vbus.opened)
+ return -EINVAL;
+
+ strncpy(priv->ll_ifname, buf, count-1);
+
+ if (priv->mdev.lowerdev) {
+ dev_put(priv->mdev.lowerdev);
+ priv->mdev.lowerdev = NULL;
+ }
+
+ priv->mdev.lowerdev = dev_get_by_name(dev_net(priv->mdev.dev),
+ priv->ll_ifname);
+
+ if (!priv->mdev.lowerdev)
+ return -ENXIO;
+
+ return len;
+}
+
+static ssize_t
+ll_ifname_show(struct vbus_device *dev, struct vbus_device_attribute *attr,
+ char *buf)
+{
+ struct venetmacv *priv = vbusdev_to_macv(dev);
+
+ return snprintf(buf, PAGE_SIZE, "%s\n", priv->ll_ifname);
+}
+
+static struct vbus_device_attribute attr_ll_ifname =
+__ATTR(ll_ifname, S_IRUGO | S_IWUSR, ll_ifname_show, ll_ifname_store);
+
+ssize_t
+clientmac_store(struct vbus_device *dev, struct vbus_device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct venetmacv *macv = vbusdev_to_macv(dev);
+ int ret;
+
+ ret = attr_cmac.store(dev, attr, buf, count);
+
+ if (ret == count)
+ memcpy(macv->mdev.dev->dev_addr, macv->dev.cmac, ETH_ALEN);
+
+ return ret;
+}
+
+struct vbus_device_attribute attr_clientmac =
+ __ATTR(client_mac, S_IRUGO | S_IWUSR, client_mac_show, clientmac_store);
+
+static struct attribute *attrs[] = {
+ &attr_clientmac.attr,
+ &attr_enabled.attr,
+ &attr_burstthresh.attr,
+ &attr_txmitigation.attr,
+ &attr_ifname.attr,
+ &attr_ll_ifname.attr,
+ NULL,
+};
+
+static struct attribute_group venetmacv_attr_group = {
+ .attrs = attrs,
+};
+
+static int
+venetmacv_netdev_open(struct net_device *dev)
+{
+ struct venetmacv *priv = netdev_priv(dev);
+ int ret = 0;
+
+ venetdev_open(&priv->dev);
+
+ if (priv->dev.vbus.link) {
+ rtnl_lock();
+ ret = priv->macvlan_netdev_ops->ndo_open(priv->mdev.dev);
+ rtnl_unlock();
+ }
+
+ return ret;
+}
+
+static int
+venetmacv_netdev_stop(struct net_device *dev)
+{
+ struct venetmacv *priv = netdev_priv(dev);
+ int needs_stop = false;
+ int ret = 0;
+
+ if (priv->dev.netif.link)
+ needs_stop = true;
+
+ venetdev_stop(&priv->dev);
+
+ if (priv->dev.vbus.link && needs_stop) {
+ rtnl_lock();
+ ret = priv->macvlan_netdev_ops->ndo_stop(dev);
+ rtnl_unlock();
+ }
+
+ return ret;
+}
+
+/*
+ * out routine for macvlan
+ */
+
+static int
+venetmacv_out(struct venetdev *vdev, struct sk_buff *skb)
+{
+ struct venetmacv *macv = venetdev_to_macv(vdev);
+ skb->dev = macv->mdev.lowerdev;
+ skb->protocol = eth_type_trans(skb, macv->mdev.lowerdev);
+ skb_push(skb, ETH_HLEN);
+ return macv->macvlan_netdev_ops->ndo_start_xmit(skb, macv->mdev.dev);
+}
+
+static int
+venetmacv_netdev_tx(struct sk_buff *skb, struct net_device *dev)
+{
+ struct venetmacv *priv = netdev_priv(dev);
+
+ return venetmacv_out(&priv->dev, skb);
+}
+
+static struct net_device_stats *
+venetmacv_netdev_stats(struct net_device *dev)
+{
+ struct venetmacv *priv = netdev_priv(dev);
+ return venetdev_get_stats(&priv->dev);
+}
+
+static int venetmacv_set_mac_address(struct net_device *dev, void *p)
+{
+ struct venetmacv *priv = netdev_priv(dev);
+ int ret;
+
+ ret = priv->macvlan_netdev_ops->ndo_set_mac_address(dev, p);
+
+ if (!ret)
+ memcpy(priv->dev.cmac, p, ETH_ALEN);
+
+ return ret;
+}
+
+int venetmacv_change_mtu(struct net_device *dev, int new_mtu)
+{
+ struct venetmacv *priv = netdev_priv(dev);
+
+ return priv->macvlan_netdev_ops->ndo_change_mtu(dev, new_mtu);
+}
+
+void venetmacv_change_rx_flags(struct net_device *dev, int change)
+{
+ struct venetmacv *priv = netdev_priv(dev);
+
+ priv->macvlan_netdev_ops->ndo_change_rx_flags(dev, change);
+}
+
+void venetmacv_set_multicast_list(struct net_device *dev)
+{
+ struct venetmacv *priv = netdev_priv(dev);
+
+ priv->macvlan_netdev_ops->ndo_set_multicast_list(dev);
+}
+
+static struct net_device_ops venetmacv_netdev_ops = {
+ .ndo_open = venetmacv_netdev_open,
+ .ndo_stop = venetmacv_netdev_stop,
+ .ndo_set_config = venetdev_netdev_config,
+ .ndo_change_mtu = venetmacv_change_mtu,
+ .ndo_set_mac_address = venetmacv_set_mac_address,
+ .ndo_change_rx_flags = venetmacv_change_rx_flags,
+ .ndo_set_multicast_list = venetmacv_set_multicast_list,
+ .ndo_validate_addr = eth_validate_addr,
+ .ndo_start_xmit = venetmacv_netdev_tx,
+ .ndo_do_ioctl = venetdev_netdev_ioctl,
+ .ndo_get_stats = venetmacv_netdev_stats,
+};
+
+
+/*
+ * This is called whenever the admin instantiates our devclass via
+ * "mkdir /config/vbus/devices/$(inst)/venet-tap"
+ */
+static int
+venetmacv_device_create(struct vbus_devclass *dc,
+ struct vbus_device **vdev)
+{
+ struct net_device *dev;
+ struct venetmacv *priv;
+ struct vbus_device *_vdev;
+
+ dev = alloc_netdev(sizeof(struct venetmacv), "macvenet%d",
+ macvlan_setup);
+
+ if (!dev)
+ return -ENOMEM;
+
+ priv = netdev_priv(dev);
+ memset(priv, 0, sizeof(*priv));
+
+ spin_lock_init(&priv->dev.lock);
+ random_ether_addr(priv->dev.cmac);
+ memcpy(priv->dev.hmac, priv->dev.cmac, ETH_ALEN);
+
+ /*
+ * vbus init
+ */
+ _vdev = &priv->dev.vbus.dev;
+
+ _vdev->type = VENETMACV_TYPE;
+ _vdev->ops = &venetmacv_device_ops;
+ _vdev->attrs = &venetmacv_attr_group;
+
+ venetdev_init(&priv->dev, dev);
+
+ priv->mdev.dev = dev;
+ priv->dev.netif.out = venetmacv_out;
+
+ priv->macvlan_netdev_ops = dev->netdev_ops;
+ dev->netdev_ops = &venetmacv_netdev_ops;
+
+ *vdev = _vdev;
+
+ return 0;
+}
+
+static struct vbus_devclass_ops venetmacv_devclass_ops = {
+ .create = venetmacv_device_create,
+};
+
+static struct vbus_devclass venetmacv_devclass = {
+ .name = VENETMACV_TYPE,
+ .ops = &venetmacv_devclass_ops,
+ .owner = THIS_MODULE,
+};
+
+static int __init venetmacv_init(void)
+{
+ return vbus_devclass_register(&venetmacv_devclass);
+}
+
+static void __exit venetmacv_cleanup(void)
+{
+ vbus_devclass_unregister(&venetmacv_devclass);
+}
+
+module_init(venetmacv_init);
+module_exit(venetmacv_cleanup);
+
diff --git a/kernel/vbus/devices/venet/venetdevice.h b/kernel/vbus/devices/venet/venetdevice.h
index 71c9f0f..1a74723 100644
--- a/kernel/vbus/devices/venet/venetdevice.h
+++ b/kernel/vbus/devices/venet/venetdevice.h
@@ -173,4 +173,11 @@ extern struct vbus_device_attribute attr_ifname;
extern struct vbus_device_attribute attr_txmitigation;
extern struct vbus_device_attribute attr_zcthresh;
+ssize_t cmac_store(struct vbus_device *dev,
+ struct vbus_device_attribute *attr,
+ const char *buf, size_t count);
+ssize_t client_mac_show(struct vbus_device *dev,
+ struct vbus_device_attribute *attr, char *buf);
+
+
#endif
^ permalink raw reply related
* Re: [PATCH 04/10] AOE: use rcu to find network device
From: Stephen Hemminger @ 2009-11-10 23:06 UTC (permalink / raw)
To: Ed Cashin; +Cc: davem, ecashin, harvey.harrison, bzolnier, netdev
In-Reply-To: <8ffe0a1df67d13a45a413f40d00dd80a@coraid.com>
On Tue, 10 Nov 2009 15:01:49 -0500
Ed Cashin <ecashin@coraid.com> wrote:
> On Tue Nov 10 13:07:37 EST 2009, shemminger@vyatta.com wrote:
> > This gets rid of another use of read_lock(&dev_base_lock) by using
> > RCU. Also, it only increments the reference count of the device actually
> > used rather than holding and releasing every device
> >
> > Compile tested only.
>
> This function runs once a minute when the aoe driver is loaded,
> if you'd like to test it a bit more.
>
> It looks like there's no dev_put corresponding to the dev_hold
> after the changes.
>
Hmm, looks like AOE actually is not ref counting the network device.
So my patch is incorrect.
As it stands (before my patch), it is UNSAFE. It can decide to queue
packets to a device that is removed out from underneath it causing
reference to freed memory.
Moving the rcu_read_lock up to aoecmd_cfg() would solve that but the
whole driver appears to be unsafe about device refcounting and handling
device removal properly.
It needs to:
1. Get a device ref count when it remembers a device: (ie addif)
2. Install a notifier that looks for device removal events
3. In notifier, remove interface, including flushing all pending
skb's for that device.
This obviously is beyond the scope of the RCU stuff.
^ permalink raw reply
* Re: [Bugme-new] [Bug 14546] New: Off-by-two stack buffer overflow in function rpc_uaddr2sockaddr() of net/sunrpc/addr.c
From: Andrew Morton @ 2009-11-10 23:29 UTC (permalink / raw)
To: argp-YZAGAMbGdGKGw+nKnLezzg
Cc: bugzilla-daemon-590EEB7GvNiWaY/ihj7yzEB+6BGkLq7r,
bugme-daemon-590EEB7GvNiWaY/ihj7yzEB+6BGkLq7r,
netdev-u79uwXL29TY76Z2rM5mHXA, J. Bruce Fields, Trond Myklebust,
Neil Brown, linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <bug-14546-10286-V0hAGp6uBxO456/isadD/XN4h3HLQggn@public.gmane.org/>
(switched to email. Please respond via emailed reply-to-all, not via the
bugzilla web interface).
On Thu, 5 Nov 2009 10:31:03 GMT
bugzilla-daemon-590EEB7GvNiWaY/ihj7yzEB+6BGkLq7r@public.gmane.org wrote:
> http://bugzilla.kernel.org/show_bug.cgi?id=14546
>
> Summary: Off-by-two stack buffer overflow in function
> rpc_uaddr2sockaddr() of net/sunrpc/addr.c
> Product: Networking
> Version: 2.5
> Kernel Version: 2.6.32-rc6
> Platform: All
> OS/Version: Linux
> Tree: Mainline
> Status: NEW
> Severity: normal
> Priority: P1
> Component: Other
> AssignedTo: acme-f8uhVLnGfZaxAyOMLChx1axOck334EZe@public.gmane.org
> ReportedBy: argp-YZAGAMbGdGKGw+nKnLezzg@public.gmane.org
> CC: argp-YZAGAMbGdGKGw+nKnLezzg@public.gmane.org
> Regression: No
>
>
> There is an off-by-two stack buffer overflow in function rpc_uaddr2sockaddr()
> of file net/sunrpc/addr.c in the Linux kernel SUNRPC implementation.
>
> The function rpc_uaddr2sockaddr() that is used to convert a universal address
> to a socket address takes as an argument the size_t variable uaddr_len (the
> length of the universal address string). The stack buffer buf is declared in
> line 315 to be of size RPCBIND_MAXUADDRLEN. If the passed argument uaddr_len is
> equal to RPCBIND_MAXUADDRLEN then the check at line 319 passes and then at
> lines 324 and 325 there are two out-of-bounds assignments:
>
> 319 if (uaddr_len > sizeof(buf))
> 320 return 0;
> ...
> 324 buf[uaddr_len] = '\n';
> 325 buf[uaddr_len + 1] = '\0';
>
> To fix it please see the attached patch.
>
Please don't submit patches via bugzilla.
Please prepare this patch as per Documentation/SubmittingPatches and
email it to all the recipients of this email, thanks.
--- ./net/sunrpc/addr.c.orig 2009-11-05 11:55:45.000000000 +0200
+++ ./net/sunrpc/addr.c 2009-11-05 12:09:34.000000000 +0200
@@ -316,7 +316,7 @@
unsigned long portlo, porthi;
unsigned short port;
- if (uaddr_len > sizeof(buf))
+ if (uaddr_len > sizeof(buf) - 2)
return 0;
memcpy(buf, uaddr, uaddr_len);
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [Bugme-new] [Bug 14546] New: Off-by-two stack buffer overflow in function rpc_uaddr2sockaddr() of net/sunrpc/addr.c
From: Chuck Lever @ 2009-11-10 23:38 UTC (permalink / raw)
To: argp
Cc: bugzilla-daemon, bugme-daemon, Linux Network Developers,
J. Bruce Fields, Trond Myklebust, Neil Brown, Andrew Morton,
Linux NFS Mailing list
In-Reply-To: <20091110152908.7558a471.akpm@linux-foundation.org>
On Nov 10, 2009, at 6:29 PM, Andrew Morton wrote:
>
> (switched to email. Please respond via emailed reply-to-all, not
> via the
> bugzilla web interface).
>
> On Thu, 5 Nov 2009 10:31:03 GMT
> bugzilla-daemon@bugzilla.kernel.org wrote:
>
>> http://bugzilla.kernel.org/show_bug.cgi?id=14546
>>
>> Summary: Off-by-two stack buffer overflow in function
>> rpc_uaddr2sockaddr() of net/sunrpc/addr.c
>> Product: Networking
>> Version: 2.5
>> Kernel Version: 2.6.32-rc6
>> Platform: All
>> OS/Version: Linux
>> Tree: Mainline
>> Status: NEW
>> Severity: normal
>> Priority: P1
>> Component: Other
>> AssignedTo: acme@ghostprotocols.net
>> ReportedBy: argp@census-labs.com
>> CC: argp@census-labs.com
>> Regression: No
>>
>>
>> There is an off-by-two stack buffer overflow in function
>> rpc_uaddr2sockaddr()
>> of file net/sunrpc/addr.c in the Linux kernel SUNRPC implementation.
>>
>> The function rpc_uaddr2sockaddr() that is used to convert a
>> universal address
>> to a socket address takes as an argument the size_t variable
>> uaddr_len (the
>> length of the universal address string). The stack buffer buf is
>> declared in
>> line 315 to be of size RPCBIND_MAXUADDRLEN. If the passed argument
>> uaddr_len is
>> equal to RPCBIND_MAXUADDRLEN then the check at line 319 passes and
>> then at
>> lines 324 and 325 there are two out-of-bounds assignments:
>>
>> 319 if (uaddr_len > sizeof(buf))
>> 320 return 0;
>> ...
>> 324 buf[uaddr_len] = '\n';
>> 325 buf[uaddr_len + 1] = '\0';
>>
>> To fix it please see the attached patch.
>>
>
> Please don't submit patches via bugzilla.
>
> Please prepare this patch as per Documentation/SubmittingPatches and
> email it to all the recipients of this email, thanks.
>
> --- ./net/sunrpc/addr.c.orig 2009-11-05 11:55:45.000000000 +0200
> +++ ./net/sunrpc/addr.c 2009-11-05 12:09:34.000000000 +0200
> @@ -316,7 +316,7 @@
> unsigned long portlo, porthi;
> unsigned short port;
>
> - if (uaddr_len > sizeof(buf))
> + if (uaddr_len > sizeof(buf) - 2)
> return 0;
Why wouldn't you bump the size of the buffer by two as well?
Otherwise valid universal addresses that are RPCBIND_MAXUADDRLEN bytes
long will fail here.
> memcpy(buf, uaddr, uaddr_len);
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply
* Re: [PATCH 04/10] AOE: use rcu to find network device
From: Stephen Hemminger @ 2009-11-10 23:53 UTC (permalink / raw)
To: karaluh, Ed Cashin; +Cc: roel.kluin, harvey.harrison, bzolnier, netdev
In-Reply-To: <20091110150617.0e6920f0@nehalam>
On Tue, 10 Nov 2009 15:06:17 -0800
Stephen Hemminger <shemminger@vyatta.com> wrote:
> On Tue, 10 Nov 2009 15:01:49 -0500
> Ed Cashin <ecashin@coraid.com> wrote:
>
> > On Tue Nov 10 13:07:37 EST 2009, shemminger@vyatta.com wrote:
> > > This gets rid of another use of read_lock(&dev_base_lock) by using
> > > RCU. Also, it only increments the reference count of the device actually
> > > used rather than holding and releasing every device
> > >
> > > Compile tested only.
> >
> > This function runs once a minute when the aoe driver is loaded,
> > if you'd like to test it a bit more.
> >
> > It looks like there's no dev_put corresponding to the dev_hold
> > after the changes.
> >
>
> Hmm, looks like AOE actually is not ref counting the network device.
> So my patch is incorrect.
>
> As it stands (before my patch), it is UNSAFE. It can decide to queue
> packets to a device that is removed out from underneath it causing
> reference to freed memory.
>
> Moving the rcu_read_lock up to aoecmd_cfg() would solve that but the
> whole driver appears to be unsafe about device refcounting and handling
> device removal properly.
>
> It needs to:
>
> 1. Get a device ref count when it remembers a device: (ie addif)
> 2. Install a notifier that looks for device removal events
> 3. In notifier, remove interface, including flushing all pending
> skb's for that device.
>
> This obviously is beyond the scope of the RCU stuff.
Here is a patch to get you going, it does compile but it probably
won't work because the code doesn't handle the case of the last
device going away from a target. This is yet another pre-existing
bug, since if a timeout happens: ejectif() is called to remove a device,
resend() will BUG in ifrotate() if all devices are gone.
---
drivers/block/aoe/aoe.h | 2 ++
drivers/block/aoe/aoecmd.c | 19 +++++++++++++++++++
drivers/block/aoe/aoedev.c | 14 ++++++++++++++
drivers/block/aoe/aoemain.c | 24 ++++++++++++++++++++++++
4 files changed, 59 insertions(+)
--- a/drivers/block/aoe/aoecmd.c 2009-11-10 15:13:25.673859220 -0800
+++ b/drivers/block/aoe/aoecmd.c 2009-11-10 15:49:20.009047132 -0800
@@ -413,6 +413,8 @@ addif(struct aoetgt *t, struct net_devic
p = getif(t, NULL);
if (!p)
return NULL;
+
+ dev_hold(nd);
p->nd = nd;
p->maxbcnt = DEFAULTBCNT;
p->lost = 0;
@@ -424,12 +426,29 @@ static void
ejectif(struct aoetgt *t, struct aoeif *ifp)
{
struct aoeif *e;
+ struct net_device *nd;
ulong n;
e = t->ifs + NAOEIFS - 1;
+ nd = e->nd;
n = (e - ifp) * sizeof *ifp;
memmove(ifp, ifp+1, n);
e->nd = NULL;
+ dev_put(nd);
+}
+
+void aoecmd_flushnet(struct aoedev *d, struct net_device *nd)
+{
+ struct aoetgt **tt, **te;
+ tt = d->targets;
+ te = tt + NTARGETS;
+ for (; tt < te && *tt; tt++) {
+ struct aoetgt *t = *tt;
+ struct aoeif *ifp;
+
+ while ( (ifp = getif(t, nd)) )
+ ejectif(t, ifp);
+ }
}
static int
--- a/drivers/block/aoe/aoemain.c 2009-11-10 15:13:25.696859195 -0800
+++ b/drivers/block/aoe/aoemain.c 2009-11-10 15:48:43.352047188 -0800
@@ -8,6 +8,8 @@
#include <linux/blkdev.h>
#include <linux/module.h>
#include <linux/skbuff.h>
+#include <linux/notifier.h>
+#include <linux/netdevice.h>
#include "aoe.h"
MODULE_LICENSE("GPL");
@@ -54,11 +56,28 @@ discover_timer(ulong vp)
}
}
+/* Callback on change of state of network device. */
+static int aoe_device_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct net_device *nd = ptr;
+
+ if (is_aoe_netif(nd) && event == NETDEV_UNREGISTER)
+ aoedev_ejectnet(nd);
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block aoe_notifier = {
+ .notifier_call = aoe_device_event,
+};
+
static void
aoe_exit(void)
{
discover_timer(TKILL);
+ unregister_netdevice_notifier(&aoe_notifier);
aoenet_exit();
unregister_blkdev(AOE_MAJOR, DEVICE_NAME);
aoechr_exit();
@@ -83,6 +102,9 @@ aoe_init(void)
ret = aoenet_init();
if (ret)
goto net_fail;
+ ret = register_netdevice_notifier(&aoe_notifier);
+ if (ret)
+ goto notifier_fail;
ret = register_blkdev(AOE_MAJOR, DEVICE_NAME);
if (ret < 0) {
printk(KERN_ERR "aoe: can't register major\n");
@@ -94,6 +116,8 @@ aoe_init(void)
return 0;
blkreg_fail:
+ unregister_netdevice_notifier(&aoe_notifier);
+ notifier_fail:
aoenet_exit();
net_fail:
aoeblk_exit();
--- a/drivers/block/aoe/aoe.h 2009-11-10 15:36:07.775921768 -0800
+++ b/drivers/block/aoe/aoe.h 2009-11-10 15:43:14.972984754 -0800
@@ -186,6 +186,7 @@ void aoecmd_ata_rsp(struct sk_buff *);
void aoecmd_cfg_rsp(struct sk_buff *);
void aoecmd_sleepwork(struct work_struct *);
void aoecmd_cleanslate(struct aoedev *);
+void aoecmd_flushnet(struct aoedev *, struct net_device *);
struct sk_buff *aoecmd_ata_id(struct aoedev *);
int aoedev_init(void);
@@ -194,6 +195,7 @@ struct aoedev *aoedev_by_aoeaddr(int maj
struct aoedev *aoedev_by_sysminor_m(ulong sysminor);
void aoedev_downdev(struct aoedev *d);
int aoedev_flush(const char __user *str, size_t size);
+void aoedev_ejectnet(struct net_device *);
int aoenet_init(void);
void aoenet_exit(void);
--- a/drivers/block/aoe/aoedev.c 2009-11-10 15:13:25.685859893 -0800
+++ b/drivers/block/aoe/aoedev.c 2009-11-10 15:46:19.430861404 -0800
@@ -162,6 +162,20 @@ aoedev_flush(const char __user *str, siz
return 0;
}
+void aoedev_ejectnet(struct net_device *nd)
+{
+ struct aoedev *d;
+ unsigned long flags;
+
+ spin_lock_irqsave(&devlist_lock, flags);
+ for (d = devlist; d; d = d->next) {
+ spin_lock(&d->lock);
+ aoecmd_flushnet(d, nd);
+ spin_unlock(&d->lock);
+ }
+ spin_unlock_irqrestore(&d->lock, flags);
+}
+
/* I'm not really sure that this is a realistic problem, but if the
network driver goes gonzo let's just leak memory after complaining. */
static void
^ permalink raw reply
* [PATCH 0/6] gianfar: Some fixes
From: Anton Vorontsov @ 2009-11-11 0:10 UTC (permalink / raw)
To: David Miller
Cc: Andy Fleming, Kumar Gopalpet-B05799, Lennert Buytenhek,
Stephen Hemminger, netdev, linuxppc-dev, Jon Loeliger
Hi all,
Here are some fixes for the gianfar driver, patches on the way.
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* [PATCH 1/6] skbuff: Do not allow skb recycling with disabled IRQs
From: Anton Vorontsov @ 2009-11-11 0:11 UTC (permalink / raw)
To: David Miller
Cc: Andy Fleming, Jon Loeliger, Kumar Gopalpet-B05799,
Lennert Buytenhek, Stephen Hemminger, netdev, linuxppc-dev
In-Reply-To: <20091111001036.GA28576@oksana.dev.rtsoft.ru>
NAPI drivers try to recycle SKBs in their polling routine, but we
generally don't know the context in which the polling will be called,
and the skb recycling itself may require IRQs to be enabled.
This patch adds irqs_disabled() test to the skb_recycle_check()
routine, so that we'll not let the drivers hit the skb recycling
path with IRQs disabled.
As a side effect, this patch actually disables skb recycling for some
[broken] drivers. E.g. gianfar driver grabs an irqsave spinlock during
TX ring processing, and then tries to recycle an skb, and that caused
the following badness:
nf_conntrack version 0.5.0 (1008 buckets, 4032 max)
------------[ cut here ]------------
Badness at kernel/softirq.c:143
NIP: c003e3c4 LR: c423a528 CTR: c003e344
...
NIP [c003e3c4] local_bh_enable+0x80/0xc4
LR [c423a528] destroy_conntrack+0xd4/0x13c [nf_conntrack]
Call Trace:
[c15d1b60] [c003e32c] local_bh_disable+0x1c/0x34 (unreliable)
[c15d1b70] [c423a528] destroy_conntrack+0xd4/0x13c [nf_conntrack]
[c15d1b80] [c02c6370] nf_conntrack_destroy+0x3c/0x70
--- Exception: c428168c at 0xc15d1c50
LR = 0xc15d1c40
[c15d1ba0] [c0286f3c] skb_release_head_state+0x100/0x104 (unreliable)
[c15d1bb0] [c0288340] skb_recycle_check+0x8c/0x10c
[c15d1bc0] [c01e1688] gfar_poll+0x190/0x384
[c15d1c10] [c02935ac] net_rx_action+0xec/0x22c
[c15d1c50] [c003dd8c] __do_softirq+0xe8/0x224
...
Reported-by: Jon Loeliger <jdl@jdl.com>
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
net/core/skbuff.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 80a9616..941bac9 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -493,6 +493,9 @@ int skb_recycle_check(struct sk_buff *skb, int skb_size)
{
struct skb_shared_info *shinfo;
+ if (irqs_disabled())
+ return 0;
+
if (skb_is_nonlinear(skb) || skb->fclone != SKB_FCLONE_UNAVAILABLE)
return 0;
--
1.6.3.3
^ permalink raw reply related
* [PATCH 2/6] gianfar: Remove 'Interrupt problem!' warning
From: Anton Vorontsov @ 2009-11-11 0:11 UTC (permalink / raw)
To: David Miller
Cc: Andy Fleming, Jon Loeliger, Kumar Gopalpet-B05799,
Lennert Buytenhek, Stephen Hemminger, netdev, linuxppc-dev
In-Reply-To: <20091111001036.GA28576@oksana.dev.rtsoft.ru>
It is OK to poll with disabled IRQs, so remove the warning.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/gianfar.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 197b358..79c28f5 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -2504,8 +2504,6 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
skb_put(skb, pkt_len);
dev->stats.rx_bytes += pkt_len;
- if (in_irq() || irqs_disabled())
- printk("Interrupt problem!\n");
gfar_process_frame(dev, skb, amount_pull);
} else {
--
1.6.3.3
^ permalink raw reply related
* [PATCH 3/6] gianfar: Fix build with CONFIG_PM=y
From: Anton Vorontsov @ 2009-11-11 0:11 UTC (permalink / raw)
To: David Miller
Cc: Andy Fleming, Jon Loeliger, Kumar Gopalpet-B05799,
Lennert Buytenhek, Stephen Hemminger, netdev, linuxppc-dev
In-Reply-To: <20091111001036.GA28576@oksana.dev.rtsoft.ru>
commit fba4ed030cfae7efdb6b79a57b0c5a9d72c9 ("gianfar: Add Multiple
Queue Support") introduced the following build failure:
CC gianfar.o
gianfar.c: In function 'gfar_restore':
gianfar.c:1249: error: request for member 'napi' in something not a structure or union
This patch fixes the issue.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/gianfar.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 79c28f5..a5b0038 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -1246,7 +1246,7 @@ static int gfar_restore(struct device *dev)
phy_start(priv->phydev);
netif_device_attach(ndev);
- napi_enable(&priv->gfargrp.napi);
+ enable_napi(priv);
return 0;
}
--
1.6.3.3
^ permalink raw reply related
* [PATCH 4/6] gianfar: Fix thinko in gfar_set_rx_stash_index()
From: Anton Vorontsov @ 2009-11-11 0:11 UTC (permalink / raw)
To: David Miller
Cc: Andy Fleming, Jon Loeliger, Kumar Gopalpet-B05799,
Lennert Buytenhek, Stephen Hemminger, netdev, linuxppc-dev
In-Reply-To: <20091111001036.GA28576@oksana.dev.rtsoft.ru>
We obviously want to write a modified 'temp' value back to the
register, not the saved IRQ flags.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/gianfar_sysfs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/gianfar_sysfs.c b/drivers/net/gianfar_sysfs.c
index 3724835..b31c9c8 100644
--- a/drivers/net/gianfar_sysfs.c
+++ b/drivers/net/gianfar_sysfs.c
@@ -186,7 +186,7 @@ static ssize_t gfar_set_rx_stash_index(struct device *dev,
temp = gfar_read(®s->attreli);
temp &= ~ATTRELI_EI_MASK;
temp |= ATTRELI_EI(index);
- gfar_write(®s->attreli, flags);
+ gfar_write(®s->attreli, temp);
out:
unlock_rx_qs(priv);
--
1.6.3.3
^ permalink raw reply related
* [PATCH 5/6] gianfar: Fix race between gfar_error() and gfar_start_xmit()
From: Anton Vorontsov @ 2009-11-11 0:11 UTC (permalink / raw)
To: David Miller
Cc: Andy Fleming, Jon Loeliger, Kumar Gopalpet-B05799,
Lennert Buytenhek, Stephen Hemminger, netdev, linuxppc-dev
In-Reply-To: <20091111001036.GA28576@oksana.dev.rtsoft.ru>
gfar_error() can arrive at the middle of gfar_start_xmit() processing,
and so it can trigger transfers of BDs that we don't yet expect to
be transmitted.
Fix this by locking the tx queues in gfar_error().
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/gianfar.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index a5b0038..fde430a 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -2943,14 +2943,22 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
if (events & IEVENT_CRL)
dev->stats.tx_aborted_errors++;
if (events & IEVENT_XFUN) {
+ unsigned long flags;
+
if (netif_msg_tx_err(priv))
printk(KERN_DEBUG "%s: TX FIFO underrun, "
"packet dropped.\n", dev->name);
dev->stats.tx_dropped++;
priv->extra_stats.tx_underrun++;
+ local_irq_save(flags);
+ lock_tx_qs(priv);
+
/* Reactivate the Tx Queues */
gfar_write(®s->tstat, gfargrp->tstat);
+
+ unlock_tx_qs(priv);
+ local_irq_restore(flags);
}
if (netif_msg_tx_err(priv))
printk(KERN_DEBUG "%s: Transmit Error\n", dev->name);
--
1.6.3.3
^ permalink raw reply related
* [PATCH 6/6] gianfar: Revive SKB recycling
From: Anton Vorontsov @ 2009-11-11 0:11 UTC (permalink / raw)
To: David Miller
Cc: Andy Fleming, Jon Loeliger, Kumar Gopalpet-B05799,
Lennert Buytenhek, Stephen Hemminger, netdev, linuxppc-dev
In-Reply-To: <20091111001036.GA28576@oksana.dev.rtsoft.ru>
Before calling gfar_clean_tx_ring() the driver grabs an irqsave
spinlock, and then tries to recycle skbs. But since
skb_recycle_check() returns 0 with IRQs disabled, we'll never
recycle any skbs.
It appears that gfar_clean_tx_ring() and gfar_start_xmit() are
mostly idependent and can work in parallel, except when they
modify num_txbdfree.
So we can drop the lock from most sections and thus fix the skb
recycling.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/gianfar.c | 31 +++++++++++++++++++------------
1 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index fde430a..16def13 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -1928,14 +1928,11 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
/* total number of fragments in the SKB */
nr_frags = skb_shinfo(skb)->nr_frags;
- spin_lock_irqsave(&tx_queue->txlock, flags);
-
/* check if there is space to queue this packet */
if ((nr_frags+1) > tx_queue->num_txbdfree) {
/* no space, stop the queue */
netif_tx_stop_queue(txq);
dev->stats.tx_fifo_errors++;
- spin_unlock_irqrestore(&tx_queue->txlock, flags);
return NETDEV_TX_BUSY;
}
@@ -1999,6 +1996,20 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
lstatus |= BD_LFLAG(TXBD_CRC | TXBD_READY) | skb_headlen(skb);
/*
+ * We can work in parallel with gfar_clean_tx_ring(), except
+ * when modifying num_txbdfree. Note that we didn't grab the lock
+ * when we were reading the num_txbdfree and checking for available
+ * space, that's because outside of this function it can only grow,
+ * and once we've got needed space, it cannot suddenly disappear.
+ *
+ * The lock also protects us from gfar_error(), which can modify
+ * regs->tstat and thus retrigger the transfers, which is why we
+ * also must grab the lock before setting ready bit for the first
+ * to be transmitted BD.
+ */
+ spin_lock_irqsave(&tx_queue->txlock, flags);
+
+ /*
* The powerpc-specific eieio() is used, as wmb() has too strong
* semantics (it requires synchronization between cacheable and
* uncacheable mappings, which eieio doesn't provide and which we
@@ -2225,6 +2236,8 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
skb_dirtytx = tx_queue->skb_dirtytx;
while ((skb = tx_queue->tx_skbuff[skb_dirtytx])) {
+ unsigned long flags;
+
frags = skb_shinfo(skb)->nr_frags;
lbdp = skip_txbd(bdp, frags, base, tx_ring_size);
@@ -2269,7 +2282,9 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
TX_RING_MOD_MASK(tx_ring_size);
howmany++;
+ spin_lock_irqsave(&tx_queue->txlock, flags);
tx_queue->num_txbdfree += frags + 1;
+ spin_unlock_irqrestore(&tx_queue->txlock, flags);
}
/* If we freed a buffer, we can restart transmission, if necessary */
@@ -2548,7 +2563,6 @@ static int gfar_poll(struct napi_struct *napi, int budget)
int tx_cleaned = 0, i, left_over_budget = budget;
unsigned long serviced_queues = 0;
int num_queues = 0;
- unsigned long flags;
num_queues = gfargrp->num_rx_queues;
budget_per_queue = budget/num_queues;
@@ -2568,14 +2582,7 @@ static int gfar_poll(struct napi_struct *napi, int budget)
rx_queue = priv->rx_queue[i];
tx_queue = priv->tx_queue[rx_queue->qindex];
- /* If we fail to get the lock,
- * don't bother with the TX BDs */
- if (spin_trylock_irqsave(&tx_queue->txlock, flags)) {
- tx_cleaned += gfar_clean_tx_ring(tx_queue);
- spin_unlock_irqrestore(&tx_queue->txlock,
- flags);
- }
-
+ tx_cleaned += gfar_clean_tx_ring(tx_queue);
rx_cleaned_per_queue = gfar_clean_rx_ring(rx_queue,
budget_per_queue);
rx_cleaned += rx_cleaned_per_queue;
--
1.6.3.3
^ permalink raw reply related
* Re: Possible bug: SO_TIMESTAMPING 2.6.30+
From: Marcus D. Leech @ 2009-11-11 0:13 UTC (permalink / raw)
To: Christopher Zimmermann; +Cc: netdev
In-Reply-To: <20091110230352.65d369af@pundit>
Christopher Zimmermann wrote:
>
>
> -------------------------------- net/ipv4/udp.c
> -------------------------------- index c47c989..4bd178a 100644
> @@ -596,6 +596,7 @@ int udp_sendmsg(struct kiocb *iocb, struct sock
> *sk, struct msghdr *msg, return -EOPNOTSUPP;
>
> ipc.opt = NULL;
> + ipc.shtx.flags = 0;
>
> if (up->pending) {
> /*
> @@ -643,6 +644,9 @@ int udp_sendmsg(struct kiocb *iocb, struct sock
> *sk, struct msghdr *msg, ipc.addr = inet->saddr;
>
> ipc.oif = sk->sk_bound_dev_if;
> + err = sock_tx_timestamp(msg, sk, &ipc.shtx);
> + if (err)
> + return err;
> if (msg->msg_controllen) {
> err = ip_cmsg_send(sock_net(sk), msg, &ipc);
> if (err)
>
>
That is already in 2.6.30
I took a look this afternoon at ipv6/udp.c and ipv6/ip6_output.c and
was able to glue-in a small (a few lines) piece of code
that apparently makes TX timestamping work for V6 DATAGRAM sockets--I
had to do it in ipv6/ip6_output.c, otherwise,
I'd have to modify calls to the v6 _append function to pass in an
extra parameter to indicate hardware Tx timestamping.
It was easier to simply pick up shtx->hardware in the _append
function, and then check to see if sk->sk_type == SOCK_DGRAM,
and then set the appropriate fields in the skb. In ipv4/udp.c, this
is done in udp_sendmsg(), before calling the _append function.
Doing this in AF_PACKET looked like it was going to be *very* easy, but
in my tests so far, it doesn't do anything useful. More
investigation in the lab this evening, and then I'll put together some
cdiffs against 2.6.32-rc5. Once that's done, perhaps I can get
some guidance about how to post those patches?
The same kind of stuff doesn't appear to work for AF_PACKET, so once I
have AF_PACKET working, I'll perhaps post a
patch set. I'm very new to this community, so I don't know how to go
about posting patches "officially" etc. But it seems that
getting this working for at least:
V4 SOCK_DGRAM
V6 SOCK_DGRAM
AF_PACKET SOCK_DGRAM/SOCK_RAW
Should be quite straightforward--using the existing code in ipv4/udp.c
as a guide. Hopefully, if I can get this going for all 3, I'll get
to keep my job :-) [just kidding!].
Cheers
Marcus
currently a IEEE 1588-2008 code monkey at a semiconductor company
--
Marcus Leech
Principal Investigator, Shirleys Bay Radio Astronomy Consortium
http://www.sbrac.org
^ permalink raw reply
* Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data
From: Ilpo Järvinen @ 2009-11-11 1:32 UTC (permalink / raw)
To: William Allen Simpson
Cc: Eric Dumazet, Linux Kernel Network Developers, Joe Perches
In-Reply-To: <4AF98AAE.6000307@gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 3927 bytes --]
On Tue, 10 Nov 2009, William Allen Simpson wrote:
> Eric Dumazet wrote:
> > William Allen Simpson a écrit :
> > > This is a significantly revised implementation of an earlier (year-old)
> > > patch that no longer applies cleanly, with permission of the original
> > > author (Adam Langley). That patch was previously reviewed:
> > >
> > > http://thread.gmane.org/gmane.linux.network/102586
>
> > I really tried hard to understand what was going on, and failed, because I
> > dont
> > have much time these days...
> >
> Thanks very much for your time.
>
> As I just wrote in a previous message, this is the easy part. It's going
> to get much more complicated -- as you know, since I sent you the papers
> with 30+ references....
>
>
> > Lack of documentation maybe ? Some DATA flow could help...
> >
> Where should that be? In the commit messages?
>
>
> > Please please, cook up elementatry patches to perform one action at a time,
> > even if they are not fully functionnal ?
> >
> Well, that's what I did with the (now) parts 1b, 1c, and 1d, but somebody
> complained that the callers didn't exist yet.
I guess there's misunderstanding here. Yes, if you bring in new helpers
that solely are used with your feature, then it that feature patch (and
I found DaveM commenting specifically this point I think), or right
beforehand with a clear note that it will be used in the subsequent patch.
But if there are uses in the existing code without any of your changes,
add the helpers and convert existing callers in a single go without doing
some other things (preferrably convert all callers, not just those you
intend to touch -- there is nothing extraordinary in this, other people do
the same kind of cleaning up work while they go, so please don't find that
this is something we try to force specifically for you :-)). ...Therefore
each change becomes rather self-sufficient to understand and review. There
was serious problems in your series in this as I constantly had to jump
between 3 patches to see what you said (changed) in the other. But also,
please don't make this an over-statement, I'd still occassionally have to
do that "patch hopping" even if nicely split (so this is not something to
be turned into all in a single patch statement :-)).
On positive side, there certainly has been clear progress already, as
patches 1 and 2 were quite nice already (except for some weird local
initializer order change which I could sort of "tolerate" but I didn't
find any particular reason why those had to be switched around.
And, one thing to not be afraid is making many short patches (nor long
one either). So if a self-contained change is a small one do it such and
no more. ...It is often so that you will be doing many relatively small
changes first to support the actual large one, and those small changes
might actually not end up being that small in lines if there are e.g.
multiple callsites converted but nevertheless they often are rather
simple, so that a quick browsing will already be enough to reveal that it
is ok (like in the MSS define change you made).
> > One patch to be able to send SYN + COOKIE (if we are the client)
> >
> > One patch to be able to receive this SYN + COOKIE and answer a SYNACK +
> > cookies (we are the server)
> >
> I'll split them. Remember, I started with a code base previously
> submitted, and everything was in one *single* large patch. I thought
> that's what you (Linux folks in general) wanted.
Only if the large patch is only for a single logical change. A
syntax cleanup, literal cleanup, cleanup to use helpers and a feature
patch are not logically same changes, and thus should be separated.
It's like you have already being doing for a part of the changes. Many
times the cleanup things are even such that they would be beneficial even
without any feature addition (but nobody has just done them so far).
--
i.
^ permalink raw reply
* Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data
From: Ilpo Järvinen @ 2009-11-11 1:48 UTC (permalink / raw)
To: William Allen Simpson
Cc: Ilpo Järvinen, Linux Kernel Network Developers, Eric Dumazet,
Joe Perches
In-Reply-To: <4AF999A1.80509@gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 4477 bytes --]
On Tue, 10 Nov 2009, William Allen Simpson wrote:
> Ilpo Järvinen wrote:
>
> > > This *is* actually in CodingStyle (line 679), and I'm trying to conform:
> >
> > Sure, fell free to but not in some random places like you now do in this
> > patch. If you are not touching a line because of a real change and want do a
> > syntax cleanup, please do it in another patch. Especially when your actual
> > change is as complicated as this is.
> >
> Not a single one is in a random place.
I disagree.
> > > > ...Also some comment changes which certainly are not mandatory nor even
> > > > related.
> > > >
> > > 1) The first is a hole left by the removal of the data fields some time
> > > ago, but they left the old (now incorrect) comment:
> > >
> > > -/* SACKs data */
> > > + u8 cookie_plus:6, /* bytes in authenticator/cookie option */
> > > + cookie_out_never:1,
> > > + cookie_in_always:1;
> > > u8 num_sacks; /* Number of SACK blocks */
> > > ...
> > I'm sorry but this response tells me that you don't seem to know even
> > yourself what you were submitting. Does this ring a bell:
> >
> > - if (th->doff == sizeof(struct tcphdr) >> 2) {
> > + /* In the spirit of fast parsing, compare doff directly to shifted
> > + * constant values. Because equality is used, short doff can be
> > + * ignored here, and checked later.
> > + */
> > + if (th->doff == (sizeof(*th) >> 2)) {
> >
> Ah, it's become apparent that you didn't click through to any of the
> references, internet-drafts, or mailing list discussion, so you don't
> know what this patch series is implementing.
Sure, I'll find that out from your patches instead, and it will be more
definitive than what the specs say ;-).
> One of the great difficulties in tracking down all several hundred uses
> of doff -- and functions that use doff -- is the serious deficiency of
> comments (compared to other code bases).
>
> Oh yeah -- Miller says I'm anal. This is TCP. Strict scrutiny is much
> better than the alternative.
>
>
> > Besides, I don't understand even what you're trying to say. ...I think we
> > already checked the length in tcp_v[46]_rcv against underflow. Why you add
> > such a non-sense comment here (plus the sizeof change I covered elsewhere)?
> > This is just a noise to annoy reviewer, nothing else.
> >
> You merely think? You don't know?
Let me say it out loud then. I know that among the very first things we
check is this underflow check. I quoted is below.
> (I didn't find one on the code walk
> through that got me to this point in the code, but perhaps I missed
> something.) If that's the case, there should at least be a comment that
> underflow was already checked, and *where* it was checked!
You added a comment which says neither but something else? :-D I wonder
how trustworthy such comment then would be to that poor "next coder" that
comes along ;-).
> The lack of pertinent comments and error checking annoys the next coder
> that comes down the pike.
This is something I can hardly understand... these are among the first
things we check before we even begin processing a TCP segment:
if (!pskb_may_pull(skb, sizeof(struct tcphdr)))
goto discard_it;
th = tcp_hdr(skb);
if (th->doff < sizeof(struct tcphdr) / 4)
goto bad_packet;
if (!pskb_may_pull(skb, th->doff * 4))
goto discard_it;
> > > 2) The second is a spelling error that I noticed in passing. It's within
> > > the same diff -u area (close to other insertions both before and after),
> > > so
> > > fixing it was trivial:
> > >
> > > - * to increse this, although since:
> > > + * to increase this, although since:
> > >
> > > Are we not supposed to fix spelling errors in comments?
> >
> > How many extra lines a reviewer has to read because of that? Why not do it
> > in a separate patch. git add -i / git stash would help you there. ...To be
> > honest, I'd rather have typos in comments which like this are not trouble
> > for the reader than clutter feature patches with all this random junk.
> >
> I'll have to look up the usage of git add -i / git stash, but in any case,
> I'm not likely to go around fixing comments later.
With git add -i you can change them while you go but easily split them on
before commit time to multiple patches. Rather easy unless except if you
you're also modifying the line right next or before to the comment it.
--
i.
^ permalink raw reply
* linux-next: manual merge of the net/wireless trees with the wireless-current tree
From: Stephen Rothwell @ 2009-11-11 2:01 UTC (permalink / raw)
To: David Miller, netdev
Cc: linux-next, linux-kernel, Wey-Yi Guy, John W. Linville
[-- Attachment #1: Type: text/plain, Size: 574 bytes --]
Hi all,
Today's linux-next merge of the net tree got conflicts in
drivers/net/wireless/iwlwifi/iwl-1000.c,
drivers/net/wireless/iwlwifi/iwl-6000.c and
drivers/net/wireless/iwlwifi/iwl-core.h between commits in the
wireless-current tree and commits in the net (wireless) tree.
I used the versions from the net tree except where obvious additions
had been made in the wireless-current tree. The fixes may not be
correct - someone should merge the wireless-current tree into the
net tree.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH resent] Documentation: rw_lock lessons learned
From: Stephen Hemminger @ 2009-11-11 2:06 UTC (permalink / raw)
To: William Allen Simpson, Paul E. McKenney, Linus Torvalds
Cc: Linux Kernel Developers, Linux Kernel Network Developers,
Eric Dumazet
In-Reply-To: <4AF9C540.5090403@gmail.com>
On Tue, 10 Nov 2009 14:55:44 -0500
William Allen Simpson <william.allen.simpson@gmail.com> wrote:
> In recent weeks, two different network projects erroneously
> strayed down the rw_lock path. Update the Documentation
> based upon comments in those threads.
>
> Signed-off-by: William.Allen.Simpson@gmail.com
> ---
> Documentation/spinlocks.txt | 14 ++++++++++++++
> 1 files changed, 14 insertions(+), 0 deletions(-)
>
I would rather see the text in Documentation/spinlocks give an explaination
as to why reader/writer locks are normally not desirable.
The whole document needs work to make it a developer document, rather than
a historical mail thread.. A good document says what should be done today,
and does not have old junk or ask the reader to overly new context
on old information.
--- a/Documentation/spinlocks.txt 2009-11-10 17:47:03.801984416 -0800
+++ b/Documentation/spinlocks.txt 2009-11-10 18:01:00.779749888 -0800
@@ -1,73 +1,8 @@
-SPIN_LOCK_UNLOCKED and RW_LOCK_UNLOCKED defeat lockdep state tracking and
-are hence deprecated.
+Lesson 1: Spin locks
-Please use DEFINE_SPINLOCK()/DEFINE_RWLOCK() or
-__SPIN_LOCK_UNLOCKED()/__RW_LOCK_UNLOCKED() as appropriate for static
-initialization.
-
-Most of the time, you can simply turn:
-
- static spinlock_t xxx_lock = SPIN_LOCK_UNLOCKED;
-
-into:
-
- static DEFINE_SPINLOCK(xxx_lock);
-
-Static structure member variables go from:
-
- struct foo bar {
- .lock = SPIN_LOCK_UNLOCKED;
- };
-
-to:
-
- struct foo bar {
- .lock = __SPIN_LOCK_UNLOCKED(bar.lock);
- };
-
-Declaration of static rw_locks undergo a similar transformation.
-
-Dynamic initialization, when necessary, may be performed as
-demonstrated below.
-
- spinlock_t xxx_lock;
- rwlock_t xxx_rw_lock;
-
- static int __init xxx_init(void)
- {
- spin_lock_init(&xxx_lock);
- rwlock_init(&xxx_rw_lock);
- ...
- }
-
- module_init(xxx_init);
-
-The following discussion is still valid, however, with the dynamic
-initialization of spinlocks or with DEFINE_SPINLOCK, etc., used
-instead of SPIN_LOCK_UNLOCKED.
-
------------------------
-
-On Fri, 2 Jan 1998, Doug Ledford wrote:
->
-> I'm working on making the aic7xxx driver more SMP friendly (as well as
-> importing the latest FreeBSD sequencer code to have 7895 support) and wanted
-> to get some info from you. The goal here is to make the various routines
-> SMP safe as well as UP safe during interrupts and other manipulating
-> routines. So far, I've added a spin_lock variable to things like my queue
-> structs. Now, from what I recall, there are some spin lock functions I can
-> use to lock these spin locks from other use as opposed to a (nasty)
-> save_flags(); cli(); stuff; restore_flags(); construct. Where do I find
-> these routines and go about making use of them? Do they only lock on a
-> per-processor basis or can they also lock say an interrupt routine from
-> mucking with a queue if the queue routine was manipulating it when the
-> interrupt occurred, or should I still use a cli(); based construct on that
-> one?
-
-See <asm/spinlock.h>. The basic version is:
-
- spinlock_t xxx_lock = SPIN_LOCK_UNLOCKED;
+The most basic primitive for locking is spinlock.
+static DEFINE_SPINLOCK(xxx_lock);
unsigned long flags;
@@ -141,13 +76,17 @@ Lesson 2: reader-writer spinlocks.
If your data accesses have a very natural pattern where you usually tend
to mostly read from the shared variables, the reader-writer locks
-(rw_lock) versions of the spinlocks are often nicer. They allow multiple
+(rw_lock) versions of the spinlocks are sometimes useful. They allow multiple
readers to be in the same critical region at once, but if somebody wants
-to change the variables it has to get an exclusive write lock. The
-routines look the same as above:
+to change the variables it has to get an exclusive write lock.
+
+NOTE! reader-writer locks require more atomic memory operations than
+simple spinlocks, so unless the reader critical secition is long you
+are better off just using spinlocks.
- rwlock_t xxx_lock = RW_LOCK_UNLOCKED;
+The routines look the same as above:
+static DEFINE_RWLOCK(xxx_lock);
unsigned long flags;
@@ -159,12 +98,15 @@ routines look the same as above:
.. read and write exclusive access to the info ...
write_unlock_irqrestore(&xxx_lock, flags);
-The above kind of lock is useful for complex data structures like linked
+The above kind of lock might useful for complex data structures like linked
lists etc, especially when you know that most of the work is to just
traverse the list searching for entries without changing the list itself,
for example. Then you can use the read lock for that kind of list
traversal, which allows many concurrent readers. Anything that _changes_
-the list will have to get the write lock.
+the list will have to get the write lock.
+
+NOTE! RCU is better for that most read only access, but requires
+correct operations (see Documentation/RCU/listRCU.txt)
Note: you cannot "upgrade" a read-lock to a write-lock, so if you at _any_
time need to do any changes (even if you don't do it every time), you have
@@ -233,4 +175,45 @@ indeed), while write-locks need to prote
Linus
+Reference information:
+
+* Older code used SPIN_LOCK_UNLOCKED and RW_LOCK_UNLOCKED to initialize
+ locks, but this is now deprecated because it interferes with the
+ lockdep state tracking. Please use DEFINE_SPINLOCK()/DEFINE_RWLOCK() or
+ __SPIN_LOCK_UNLOCKED()/__RW_LOCK_UNLOCKED() as appropriate for static
+ initialization.
+
+ Most of the time, you can simply turn:
+ static spinlock_t xxx_lock = SPIN_LOCK_UNLOCKED;
+ into:
+ static DEFINE_SPINLOCK(xxx_lock);
+
+ Static structure member variables go from:
+
+ struct foo bar {
+ .lock = SPIN_LOCK_UNLOCKED;
+ };
+
+ to:
+
+ struct foo bar {
+ .lock = __SPIN_LOCK_UNLOCKED(bar.lock);
+ };
+
+ Declaration of static rw_locks undergo a similar transformation.
+
+ Dynamic initialization, when necessary, may be performed as
+ demonstrated below.
+
+ spinlock_t xxx_lock;
+ rwlock_t xxx_rw_lock;
+
+ static int __init xxx_init(void)
+ {
+ spin_lock_init(&xxx_lock);
+ rwlock_init(&xxx_rw_lock);
+ ...
+ }
+
+ module_init(xxx_init);
^ permalink raw reply
* RE: [PATCH 6/6] gianfar: Revive SKB recycling
From: Kumar Gopalpet-B05799 @ 2009-11-11 4:20 UTC (permalink / raw)
To: Anton Vorontsov, David Miller
Cc: Fleming Andy-AFLEMING, Jon Loeliger, Lennert Buytenhek,
Stephen Hemminger, netdev, linuxppc-dev
In-Reply-To: <20091111001110.GF8817@oksana.dev.rtsoft.ru>
>-----Original Message-----
>From: Anton Vorontsov [mailto:avorontsov@ru.mvista.com]
>Sent: Wednesday, November 11, 2009 5:41 AM
>To: David Miller
>Cc: Fleming Andy-AFLEMING; Jon Loeliger; Kumar
>Gopalpet-B05799; Lennert Buytenhek; Stephen Hemminger;
>netdev@vger.kernel.org; linuxppc-dev@ozlabs.org
>Subject: [PATCH 6/6] gianfar: Revive SKB recycling
>
>Before calling gfar_clean_tx_ring() the driver grabs an
>irqsave spinlock, and then tries to recycle skbs. But since
>skb_recycle_check() returns 0 with IRQs disabled, we'll never
>recycle any skbs.
>
>It appears that gfar_clean_tx_ring() and gfar_start_xmit() are
>mostly idependent and can work in parallel, except when they
>modify num_txbdfree.
>
>So we can drop the lock from most sections and thus fix the
>skb recycling.
>
>Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>---
> drivers/net/gianfar.c | 31 +++++++++++++++++++------------
> 1 files changed, 19 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
>index fde430a..16def13 100644
>--- a/drivers/net/gianfar.c
>+++ b/drivers/net/gianfar.c
>@@ -1928,14 +1928,11 @@ static int gfar_start_xmit(struct
>sk_buff *skb, struct net_device *dev)
> /* total number of fragments in the SKB */
> nr_frags = skb_shinfo(skb)->nr_frags;
>
>- spin_lock_irqsave(&tx_queue->txlock, flags);
>-
> /* check if there is space to queue this packet */
> if ((nr_frags+1) > tx_queue->num_txbdfree) {
> /* no space, stop the queue */
> netif_tx_stop_queue(txq);
> dev->stats.tx_fifo_errors++;
>- spin_unlock_irqrestore(&tx_queue->txlock, flags);
> return NETDEV_TX_BUSY;
> }
>
>@@ -1999,6 +1996,20 @@ static int gfar_start_xmit(struct
>sk_buff *skb, struct net_device *dev)
> lstatus |= BD_LFLAG(TXBD_CRC | TXBD_READY) | skb_headlen(skb);
>
> /*
>+ * We can work in parallel with gfar_clean_tx_ring(), except
>+ * when modifying num_txbdfree. Note that we didn't
>grab the lock
>+ * when we were reading the num_txbdfree and checking
>for available
>+ * space, that's because outside of this function it
>can only grow,
>+ * and once we've got needed space, it cannot suddenly
>disappear.
>+ *
>+ * The lock also protects us from gfar_error(), which can modify
>+ * regs->tstat and thus retrigger the transfers, which is why we
>+ * also must grab the lock before setting ready bit for
>the first
>+ * to be transmitted BD.
>+ */
>+ spin_lock_irqsave(&tx_queue->txlock, flags);
>+
>+ /*
> * The powerpc-specific eieio() is used, as wmb() has too strong
> * semantics (it requires synchronization between cacheable and
> * uncacheable mappings, which eieio doesn't provide
>and which we @@ -2225,6 +2236,8 @@ static int
>gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
> skb_dirtytx = tx_queue->skb_dirtytx;
>
> while ((skb = tx_queue->tx_skbuff[skb_dirtytx])) {
>+ unsigned long flags;
>+
> frags = skb_shinfo(skb)->nr_frags;
> lbdp = skip_txbd(bdp, frags, base, tx_ring_size);
>
>@@ -2269,7 +2282,9 @@ static int gfar_clean_tx_ring(struct
>gfar_priv_tx_q *tx_queue)
> TX_RING_MOD_MASK(tx_ring_size);
>
> howmany++;
>+ spin_lock_irqsave(&tx_queue->txlock, flags);
> tx_queue->num_txbdfree += frags + 1;
>+ spin_unlock_irqrestore(&tx_queue->txlock, flags);
> }
>
> /* If we freed a buffer, we can restart transmission,
>if necessary */ @@ -2548,7 +2563,6 @@ static int
>gfar_poll(struct napi_struct *napi, int budget)
> int tx_cleaned = 0, i, left_over_budget = budget;
> unsigned long serviced_queues = 0;
> int num_queues = 0;
>- unsigned long flags;
>
> num_queues = gfargrp->num_rx_queues;
> budget_per_queue = budget/num_queues;
>@@ -2568,14 +2582,7 @@ static int gfar_poll(struct napi_struct
>*napi, int budget)
> rx_queue = priv->rx_queue[i];
> tx_queue = priv->tx_queue[rx_queue->qindex];
>
>- /* If we fail to get the lock,
>- * don't bother with the TX BDs */
>- if
>(spin_trylock_irqsave(&tx_queue->txlock, flags)) {
>- tx_cleaned +=
>gfar_clean_tx_ring(tx_queue);
>-
>spin_unlock_irqrestore(&tx_queue->txlock,
>- flags);
>- }
>-
>+ tx_cleaned += gfar_clean_tx_ring(tx_queue);
> rx_cleaned_per_queue =
>gfar_clean_rx_ring(rx_queue,
>
>budget_per_queue);
> rx_cleaned += rx_cleaned_per_queue;
>--
Anton, we tried some experiments too at our end, and removing the
spinlocks did help improve the performance and recycling was effective
although, I don't have exact numbers to specify.
But overall I agree with you in removing the spinlocks from the
gfar_poll context.
--
Thanks
Sandeep
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox