* Re: [PATCH] tipc: set sk_err correctly when connection fails
From: Paul Gortmaker @ 2013-08-27 18:12 UTC (permalink / raw)
To: Erik Hugne; +Cc: netdev, jon.maloy, ying.xue, tipc-discussion, nhan.tt.vo
In-Reply-To: <20130827151841.GE1939@eerihug-hybrid.rnd.ki.sw.ericsson.se>
On 13-08-27 11:18 AM, Erik Hugne wrote:
> On Tue, Aug 27, 2013 at 09:20:23AM -0400, Paul Gortmaker wrote:
>> What was the high level user visible symptom in this case?
>> Stalled connections or ... ?
>
> Should the connect fail, if the publication/server is unavailable or some other
> error. The connect() call returns the error code directly (as a positive value).
Please send a v2 with the end-user visible symptom clearly described;
as this information is what people use in order to triage whether
commits belong in stable, or net vs. net-next etc. For example:
Should the connect fail, say if the publication/server is unavailable or
some other error, then the code returns a positive return value. Since
most code only checks for a negative return on connect(), it tries to
continue, but will ultimately fail on the 1st sendto() as the strace
snippet below shows.
I've said "most code" since I simply don't know what it was that you were
tracing below. It would help if we knew if this part of a common application
or similar.
Thanks,
Paul.
--
>
> [...]
> socket(0x1e /* PF_??? */, SOCK_SEQPACKET, 0) = 3
> setsockopt(3, 0x10f /* SOL_?? */, 129, [0], 4) = 0
> setsockopt(3, 0x10f /* SOL_?? */, 127, [0], 4) = 0
> connect(3, {sa_family=0x1e /* AF_??? */, sa_data="\2\1\322\4\0\0\322\4\0\0\0\0\0\0"}, 16) = 111
> sendto(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 66000, 0, NULL, 0) = -1 EPIPE (Broken pipe)
>
> In the strace above, error checking was done as:
> if (connect(fd,.....) < 0)
> perror("connect");
>
> //E
>
^ permalink raw reply
* RE: [net-next RFC 3/7] i40evf: core ethtool functionality
From: Williams, Mitch A @ 2013-08-27 18:23 UTC (permalink / raw)
To: Ben Hutchings, Kirsher, Jeffrey T
Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
sassmann@redhat.com
In-Reply-To: <1377624472.13272.43.camel@bwh-desktop.uk.level5networks.com>
> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Tuesday, August 27, 2013 10:28 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Williams, Mitch A; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com
> Subject: Re: [net-next RFC 3/7] i40evf: core ethtool functionality
>
> On Thu, 2013-08-22 at 21:53 -0700, Jeff Kirsher wrote:
> [...]
> > --- /dev/null
> > +++ b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
> [...]
> > +/**
> > + * i40evf_get_settings - Get Link Speed and Duplex settings
> > + * @netdev: network interface device structure
> > + * @ecmd: ethtool command
> > + *
> > + * Reports speed/duplex settings. Because this is a VF, we don't know
> what
> > + * kind of link we really have, so we fake it.
> > + **/
> > +static int i40evf_get_settings(struct net_device *netdev,
> > + struct ethtool_cmd *ecmd)
> > +{
> > + ecmd->supported = SUPPORTED_10000baseT_Full;
>
> This is silly.
>
> > + ecmd->autoneg = AUTONEG_DISABLE;
> > + ecmd->transceiver = XCVR_DUMMY1;
> > + ecmd->port = -1;
>
> PORT_NONE, I think.
>
> > + return 0;
> > +}
> > +
> > +/**
> > + * i40evf_get_sset_count - Get length of string set
> > + * @netdev: network interface device structure
> > + * @sset: id of string set
> > + *
> > + * Reports size (in bytes) of string set.
>
> Not in bytes.
>
> > This driver only supports
> > + * strings for statistics.
> > + **/
> [...]
> > +/**
> > + * i40evf_get_strings - Get string set
> > + * @netdev: network interface device structure
> > + * @sset: id of string set
> > + * @data: buffer for string data
> > + *
> > + * Strings are concatenated into the data buffer, separated by nulls.
>
> This looks like a description of a Windows-style double-null-terminated
> string list, not an ethtool string set.
>
> > + **/
> > +static void i40evf_get_strings(struct net_device *netdev, u32 sset, u8
> *data)
> > +{
> > + struct i40evf_adapter *adapter = netdev_priv(netdev);
> > + u8 *p = data;
> > + int i;
> > +
> > + if (sset == ETH_SS_STATS) {
> > + for (i = 0; i < I40EVF_GLOBAL_STATS_LEN; i++) {
> > + memcpy(p, i40evf_gstrings_stats[i].stat_string,
> > + ETH_GSTRING_LEN);
> > + p += ETH_GSTRING_LEN;
> > + }
> > + for (i = 0; i < adapter->vsi_res->num_queue_pairs; i++) {
> > + snprintf(p, ETH_GSTRING_LEN, "%s.tx_queue_%u.packets",
> > + netdev->name, i);
> > + p += ETH_GSTRING_LEN;
> > + snprintf(p, ETH_GSTRING_LEN, "%s.tx_queue_%u.bytes",
> > + netdev->name, i);
> > + p += ETH_GSTRING_LEN;
> > + }
> > + for (i = 0; i < adapter->vsi_res->num_queue_pairs; i++) {
> > + snprintf(p, ETH_GSTRING_LEN, "%s.rx_queue_%u.packets",
> > + netdev->name, i);
> > + p += ETH_GSTRING_LEN;
> > + snprintf(p, ETH_GSTRING_LEN, "%s.rx_queue_%u.bytes",
> > + netdev->name, i);
> > + p += ETH_GSTRING_LEN;
>
> It is completely redundant to put the device name into statistic names,
> and is liable to cause the names to be truncated. There was also a
> discussion about how to name per-queue statistics a while back, which
> seemed to end in agreement:
>
> http://thread.gmane.org/gmane.linux.network/273014/focus=12205
>
> So I think the name formats should be "tx-%u.packets" etc.
>
> [...]
> > +/**
> > + * i40evf_get_drvinto - Get driver info
> > + * @netdev: network interface device structure
> > + * @drvinfo: ethool driver info structure
> > + *
> > + * Returns information about the driver and device for display to the
> user.
> > + **/
> > +static void i40evf_get_drvinfo(struct net_device *netdev,
> > + struct ethtool_drvinfo *drvinfo)
> > +{
> > + struct i40evf_adapter *adapter = netdev_priv(netdev);
> > +
> > + strncpy(drvinfo->driver, i40evf_driver_name, 32);
> > + strncpy(drvinfo->version, i40evf_driver_version, 32);
> > +
> > + strncpy(drvinfo->fw_version, "N/A", 4);
>
> If there is no firmware version then don't set fw_version at all.
>
> > + strncpy(drvinfo->bus_info, pci_name(adapter->pdev), 32);
>
> Use strlcpy() not strncpy() for all of the other strings.
>
> > + drvinfo->n_stats = I40EVF_STATS_LEN;
> > + drvinfo->regdump_len = 0;
>
> Don't set these at all, as they are initialised by the ethtool core
> based on other driver operations.
>
> [...]
> > +static void i40evf_get_ringparam(struct net_device *netdev,
> > + struct ethtool_ringparam *ring)
> > +{
> > + struct i40evf_adapter *adapter = netdev_priv(netdev);
> > + struct i40e_ring *tx_ring = &adapter->tx_rings[0];
> > + struct i40e_ring *rx_ring = &adapter->rx_rings[0];
> > +
> > + ring->rx_max_pending = I40EVF_MAX_RXD;
> > + ring->tx_max_pending = I40EVF_MAX_TXD;
> > + ring->rx_mini_max_pending = 0;
> > + ring->rx_jumbo_max_pending = 0;
> > + ring->rx_pending = rx_ring->count;
> > + ring->tx_pending = tx_ring->count;
> > + ring->rx_mini_pending = 0;
> > + ring->rx_jumbo_pending = 0;
> > +}
>
> No need to set the unsupported fields to 0.
>
> [...]
> > +static int i40evf_get_coalesce(struct net_device *netdev,
> > + struct ethtool_coalesce *ec)
> > +{
> > + struct i40evf_adapter *adapter = netdev_priv(netdev);
> > + struct i40e_vsi *vsi = &adapter->vsi;
> > +
> > + ec->tx_max_coalesced_frames_irq = vsi->work_limit;
> > + ec->rx_max_coalesced_frames_irq = vsi->work_limit;
>
> Use the fields without the '_irq' suffix. The '_irq' suffixed fields
> are there for some NICs that change coalescing behaviour depending on
> whether the previous IRQ is still being handled.
>
> [...]
> > +static int i40evf_set_coalesce(struct net_device *netdev,
> > + struct ethtool_coalesce *ec)
> > +{
> > + struct i40evf_adapter *adapter = netdev_priv(netdev);
> > + struct i40e_hw *hw = &adapter->hw;
> > + struct i40e_vsi *vsi = &adapter->vsi;
> > + struct i40e_q_vector *q_vector;
> > + int i;
> > +
> > + if (ec->tx_max_coalesced_frames_irq || ec->rx_max_coalesced_frames_irq)
> > + vsi->work_limit = ec->tx_max_coalesced_frames_irq;
> [...]
>
> Again, use the fields without the '_irq' suffix.
>
> Ben.
>
Thanks for the review, Ben. I'll fix all of these before we send this out again.
-Mitch
> --
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH RFC net-next] ipv6: avoid high order memory allocations for /proc/net/ipv6_route
From: Hannes Frederic Sowa @ 2013-08-27 18:24 UTC (permalink / raw)
To: netdev, yoshfuji; +Cc: greearb, kaber, eric.dumazet, gaofeng, kuznet, davem
In-Reply-To: <20130822031602.GA19269@order.stressinduktion.org>
Hello Ben and Patrick!
I have a question regarding this patch:
commit 2bec5a369ee79576a3eea2c23863325089785a2c
Author: Patrick McHardy <kaber@trash.net>
Date: Mon Feb 8 05:19:03 2010 +0000
ipv6: fib: fix crash when changing large fib while dumping it
When the fib size exceeds what can be dumped in a single skb, the
dump is suspended and resumed once the last skb has been received
by userspace. When the fib is changed while the dump is suspended,
the walker might contain stale pointers, causing a crash when the
dump is resumed.
Below is a patch to switch /proc/net/ipv6_route to an iterating interface
as well, thus avoiding large order allocations in the procfs code.
Looking at the netlink dump code I think I might also need the
serialnumber check and restart logic but I could not reproduce any NULL-ptr
deref with my code or with your patch reverted. Maybe you remember some
special cases how to reproduce this? There was already some doubt the
serialnumber check would be needed in this thread:
https://lkml.org/lkml/2012/6/12/386
I also tried to check the walker logic and have not found any spots where it
should produce such a BUG.
It would also help to know the exact kernel version on which this bug was
produced.
Thanks a lot!
On Thu, Aug 22, 2013 at 05:16:02AM +0200, Hannes Frederic Sowa wrote:
> On Wed, Aug 21, 2013 at 07:50:24PM +0200, Hannes Frederic Sowa wrote:
> > I created this patch in a hurry while trying to get the rt6i_flags of
> > the routing entries on a system with lots of routes (I later cleaned it
> > up a bit).
> >
> > The current approach for dumping routes over netlink is to redump a tree
> > as soon as the serialnumber on the nodes changes. Is this behavior more
> > preferable as to just ignore the routes for that moment? I am in favour
> > of not restarting the dump on /proc/net/ipv6_route.
>
> Ok, I hit the same panics as Patrick once did
> while some more stress testing this change
> (http://permalink.gmane.org/gmane.linux.network/151391).
>
> As I read this up, this was also the reason the serial checks where
> introduced. It also tries to skip visited notes, so please void my
> question above.
>
> The reason is a disconnected fib6_node in FWS_U state trying to walk
> up. I'll investigate if we can fix this up in the walker post-cleanup on
> fib6_repair_tree/fib6_del_route (then we could also remove the serial
> number checks on inet6_dump_fib) or I will make the appropriate serial
> changes to this patch, too.
This was actually an error in my debugging code. I could not trigger it
afterwards. It was fully my fault.
Current patch looks like this:
[PATCH RFC] ipv6: avoid high order memory allocations for /proc/net/ipv6_route
Dumping routes on a system with lots rt6_infos in the fibs causes up to
11-order allocations in seq_file (which fail). While we could switch
there to vmalloc we could just implement the streaming interface for
/proc/net/ipv6_route. This patch switches /proc/net/ipv6_route from
single_open_net to seq_open_net.
loff_t *pos tracks dst entries.
Also kill never used struct rt6_proc_arg and now unused function
fib6_clean_all_ro.
Cc: Ben Greear <greearb@candelatech.com>
Cc: Patrick McHardy <kaber@trash.net>
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/net/ip6_fib.h | 7 +-
net/ipv6/ip6_fib.c | 191 +++++++++++++++++++++++++++++++++++++++++++++-----
net/ipv6/route.c | 46 +-----------
3 files changed, 176 insertions(+), 68 deletions(-)
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 48ec25a..9bd5de8 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -281,10 +281,6 @@ struct fib6_node *fib6_locate(struct fib6_node *root,
const struct in6_addr *daddr, int dst_len,
const struct in6_addr *saddr, int src_len);
-extern void fib6_clean_all_ro(struct net *net,
- int (*func)(struct rt6_info *, void *arg),
- int prune, void *arg);
-
extern void fib6_clean_all(struct net *net,
int (*func)(struct rt6_info *, void *arg),
int prune, void *arg);
@@ -306,6 +302,9 @@ extern void fib6_gc_cleanup(void);
extern int fib6_init(void);
+extern int ipv6_route_open(struct inode *inode,
+ struct file *file);
+
#ifdef CONFIG_IPV6_MULTIPLE_TABLES
extern int fib6_rules_init(void);
extern void fib6_rules_cleanup(void);
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 73db48e..04192af 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -1529,25 +1529,6 @@ static void fib6_clean_tree(struct net *net, struct fib6_node *root,
fib6_walk(&c.w);
}
-void fib6_clean_all_ro(struct net *net, int (*func)(struct rt6_info *, void *arg),
- int prune, void *arg)
-{
- struct fib6_table *table;
- struct hlist_head *head;
- unsigned int h;
-
- rcu_read_lock();
- for (h = 0; h < FIB6_TABLE_HASHSZ; h++) {
- head = &net->ipv6.fib_table_hash[h];
- hlist_for_each_entry_rcu(table, head, tb6_hlist) {
- read_lock_bh(&table->tb6_lock);
- fib6_clean_tree(net, &table->tb6_root,
- func, prune, arg);
- read_unlock_bh(&table->tb6_lock);
- }
- }
- rcu_read_unlock();
-}
void fib6_clean_all(struct net *net, int (*func)(struct rt6_info *, void *arg),
int prune, void *arg)
{
@@ -1782,3 +1763,175 @@ void fib6_gc_cleanup(void)
unregister_pernet_subsys(&fib6_net_ops);
kmem_cache_destroy(fib6_node_kmem);
}
+
+#ifdef CONFIG_PROC_FS
+
+struct ipv6_route_iter {
+ struct seq_net_private p;
+ struct fib6_walker_t w;
+ loff_t skip;
+ struct fib6_table *tbl;
+};
+
+static int ipv6_route_seq_show(struct seq_file *seq, void *v)
+{
+ struct rt6_info *rt = v;
+ struct ipv6_route_iter *iter = seq->private;
+
+ seq_printf(seq, "%pi6 %02x ", &rt->rt6i_dst.addr, rt->rt6i_dst.plen);
+
+#ifdef CONFIG_IPV6_SUBTREES
+ seq_printf(seq, "%pi6 %02x ", &rt->rt6i_src.addr, rt->rt6i_src.plen);
+#else
+ seq_puts(seq, "00000000000000000000000000000000 00 ");
+#endif
+ if (rt->rt6i_flags & RTF_GATEWAY)
+ seq_printf(seq, "%pi6", &rt->rt6i_gateway);
+ else
+ seq_puts(seq, "00000000000000000000000000000000");
+
+ seq_printf(seq, " %08x %08x %08x %08x %8s\n",
+ rt->rt6i_metric, atomic_read(&rt->dst.__refcnt),
+ rt->dst.__use, rt->rt6i_flags,
+ rt->dst.dev ? rt->dst.dev->name : "");
+ iter->w.leaf = NULL;
+ return 0;
+}
+
+static int ipv6_route_yield(struct fib6_walker_t *w)
+{
+ struct ipv6_route_iter *iter = w->args;
+
+ if (!iter->skip)
+ return 1;
+
+ do {
+ iter->w.leaf = iter->w.leaf->dst.rt6_next;
+ iter->skip--;
+ if (!iter->skip && iter->w.leaf)
+ return 1;
+ } while (iter->w.leaf);
+
+ return 0;
+}
+
+static void ipv6_route_seq_setup_walk(struct ipv6_route_iter *iter)
+{
+ memset(&iter->w, 0, sizeof(iter->w));
+ iter->w.func = ipv6_route_yield;
+ iter->w.root = &iter->tbl->tb6_root;
+ iter->w.state = FWS_INIT;
+ iter->w.node = iter->w.root;
+ iter->w.args = iter;
+ INIT_LIST_HEAD(&iter->w.lh);
+ fib6_walker_link(&iter->w);
+}
+
+static struct fib6_table *ipv6_route_seq_next_table(struct fib6_table *tbl,
+ struct net *net)
+{
+ unsigned int h;
+ struct hlist_node *node;
+
+ if (tbl) {
+ h = (tbl->tb6_id & (FIB6_TABLE_HASHSZ - 1)) + 1;
+ node = rcu_dereference_bh(hlist_next_rcu(&tbl->tb6_hlist));
+ } else {
+ h = 0;
+ node = NULL;
+ }
+
+ while (!node && h < FIB6_TABLE_HASHSZ) {
+ node = rcu_dereference_bh(
+ hlist_first_rcu(&net->ipv6.fib_table_hash[h++]));
+ }
+ return hlist_entry_safe(node, struct fib6_table, tb6_hlist);
+}
+
+static void *ipv6_route_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+{
+ int r;
+ struct rt6_info *n;
+ struct net *net = seq_file_net(seq);
+ struct ipv6_route_iter *iter = seq->private;
+
+ if (!v)
+ goto iter_table;
+
+ n = ((struct rt6_info *)v)->dst.rt6_next;
+ if (n) {
+ ++*pos;
+ return n;
+ }
+
+iter_table:
+ read_lock(&iter->tbl->tb6_lock);
+ r = fib6_walk_continue(&iter->w);
+ read_unlock(&iter->tbl->tb6_lock);
+ if (r > 0) {
+ if (v)
+ ++*pos;
+ return iter->w.leaf;
+ } else if (r < 0) {
+ fib6_walker_unlink(&iter->w);
+ return NULL;
+ }
+ fib6_walker_unlink(&iter->w);
+
+ iter->tbl = ipv6_route_seq_next_table(iter->tbl, net);
+ if (!iter->tbl)
+ return NULL;
+
+ ipv6_route_seq_setup_walk(iter);
+ goto iter_table;
+}
+
+static void *ipv6_route_seq_start(struct seq_file *seq, loff_t *pos)
+ __acquires(RCU_BH)
+{
+ struct net *net = seq_file_net(seq);
+ struct ipv6_route_iter *iter = seq->private;
+
+ rcu_read_lock_bh();
+ iter->tbl = ipv6_route_seq_next_table(NULL, net);
+ iter->skip = *pos;
+
+ if (iter->tbl) {
+ ipv6_route_seq_setup_walk(iter);
+ return ipv6_route_seq_next(seq, NULL, pos);
+ } else {
+ return NULL;
+ }
+}
+
+static bool ipv6_route_iter_active(struct ipv6_route_iter *iter)
+{
+ struct fib6_walker_t *w = &iter->w;
+ return w->node && !(w->state == FWS_U && w->node == w->root);
+}
+
+static void ipv6_route_seq_stop(struct seq_file *seq, void *v)
+ __releases(RCU_BH)
+{
+ struct ipv6_route_iter *iter = seq->private;
+
+ if (ipv6_route_iter_active(iter))
+ fib6_walker_unlink(&iter->w);
+
+ rcu_read_unlock_bh();
+}
+
+static const struct seq_operations ipv6_route_seq_ops = {
+ .start = ipv6_route_seq_start,
+ .next = ipv6_route_seq_next,
+ .stop = ipv6_route_seq_stop,
+ .show = ipv6_route_seq_show
+};
+
+int ipv6_route_open(struct inode *inode, struct file *file)
+{
+ return seq_open_net(inode, file, &ipv6_route_seq_ops,
+ sizeof(struct ipv6_route_iter));
+}
+
+#endif /* CONFIG_PROC_FS */
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index e22c4db..42cb227 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2729,56 +2729,12 @@ static int ip6_route_dev_notify(struct notifier_block *this,
#ifdef CONFIG_PROC_FS
-struct rt6_proc_arg
-{
- char *buffer;
- int offset;
- int length;
- int skip;
- int len;
-};
-
-static int rt6_info_route(struct rt6_info *rt, void *p_arg)
-{
- struct seq_file *m = p_arg;
-
- seq_printf(m, "%pi6 %02x ", &rt->rt6i_dst.addr, rt->rt6i_dst.plen);
-
-#ifdef CONFIG_IPV6_SUBTREES
- seq_printf(m, "%pi6 %02x ", &rt->rt6i_src.addr, rt->rt6i_src.plen);
-#else
- seq_puts(m, "00000000000000000000000000000000 00 ");
-#endif
- if (rt->rt6i_flags & RTF_GATEWAY) {
- seq_printf(m, "%pi6", &rt->rt6i_gateway);
- } else {
- seq_puts(m, "00000000000000000000000000000000");
- }
- seq_printf(m, " %08x %08x %08x %08x %8s\n",
- rt->rt6i_metric, atomic_read(&rt->dst.__refcnt),
- rt->dst.__use, rt->rt6i_flags,
- rt->dst.dev ? rt->dst.dev->name : "");
- return 0;
-}
-
-static int ipv6_route_show(struct seq_file *m, void *v)
-{
- struct net *net = (struct net *)m->private;
- fib6_clean_all_ro(net, rt6_info_route, 0, m);
- return 0;
-}
-
-static int ipv6_route_open(struct inode *inode, struct file *file)
-{
- return single_open_net(inode, file, ipv6_route_show);
-}
-
static const struct file_operations ipv6_route_proc_fops = {
.owner = THIS_MODULE,
.open = ipv6_route_open,
.read = seq_read,
.llseek = seq_lseek,
- .release = single_release_net,
+ .release = seq_release_net,
};
static int rt6_stats_seq_show(struct seq_file *seq, void *v)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 06/11] net: calxedaxgmac: fix race with tx queue stop/wake
From: Ben Hutchings @ 2013-08-27 18:31 UTC (permalink / raw)
To: Rob Herring
Cc: netdev, linux-kernel, Andreas Herrmann, Lennert Buytenhek,
Rob Herring
In-Reply-To: <1377557126-10716-6-git-send-email-robherring2@gmail.com>
On Mon, 2013-08-26 at 17:45 -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> Since the xgmac transmit start and completion work locklessly, it is
> possible for xgmac_xmit to stop the tx queue after the xgmac_tx_complete
> has run resulting in the tx queue never being woken up. Fix this by
> ensuring that ring buffer index updates are visible and serialize the
> queue wake with netif_tx_lock.
>
> The implementation used here was copied from
> drivers/net/ethernet/broadcom/tg3.c.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
> drivers/net/ethernet/calxeda/xgmac.c | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
> index f630855..cd5872c 100644
> --- a/drivers/net/ethernet/calxeda/xgmac.c
> +++ b/drivers/net/ethernet/calxeda/xgmac.c
> @@ -410,6 +410,9 @@ struct xgmac_priv {
> #define dma_ring_space(h, t, s) CIRC_SPACE(h, t, s)
> #define dma_ring_cnt(h, t, s) CIRC_CNT(h, t, s)
>
> +#define tx_dma_ring_space(p) \
> + dma_ring_space((p)->tx_head, (p)->tx_tail, DMA_TX_RING_SZ)
> +
> /* XGMAC Descriptor Access Helpers */
> static inline void desc_set_buf_len(struct xgmac_dma_desc *p, u32 buf_sz)
> {
> @@ -886,9 +889,14 @@ static void xgmac_tx_complete(struct xgmac_priv *priv)
> priv->tx_tail = dma_ring_incr(entry, DMA_TX_RING_SZ);
> }
>
> - if (dma_ring_space(priv->tx_head, priv->tx_tail, DMA_TX_RING_SZ) >
> - MAX_SKB_FRAGS)
> - netif_wake_queue(priv->dev);
> + /* Ensure tx_tail is visible to xgmac_xmit */
> + smp_mb();
> + if (unlikely(netif_queue_stopped(priv->dev))) {
> + netif_tx_lock(priv->dev);
> + if (tx_dma_ring_space(priv) > MAX_SKB_FRAGS)
> + netif_wake_queue(priv->dev);
> + netif_tx_unlock(priv->dev);
> + }
> }
You don't need to take the TX lock for this. The memory barriers
provide sufficient synchronisation.
> static void xgmac_tx_timeout_work(struct work_struct *work)
> @@ -1125,10 +1133,15 @@ static netdev_tx_t xgmac_xmit(struct sk_buff *skb, struct net_device *dev)
>
> priv->tx_head = dma_ring_incr(entry, DMA_TX_RING_SZ);
>
> - if (dma_ring_space(priv->tx_head, priv->tx_tail, DMA_TX_RING_SZ) <
> - MAX_SKB_FRAGS)
> + /* Ensure tx_head update is visible to tx completion */
> + smp_mb();
> + if (unlikely(tx_dma_ring_space(priv) < MAX_SKB_FRAGS)) {
> netif_stop_queue(dev);
> -
> + /* Ensure netif_stop_queue is visible to tx completion */
> + smp_mb();
> + if (tx_dma_ring_space(priv) > MAX_SKB_FRAGS)
> + netif_wake_queue(dev);
You should use netif_start_queue() rather than netif_wake_queue(), since
you know the TX scheduler is already active.
Ben.
> + }
> return NETDEV_TX_OK;
> }
>
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH net-next 01/10] net: etherdevice: add address inherit helper
From: David Miller @ 2013-08-27 18:35 UTC (permalink / raw)
To: bjorn; +Cc: stephen, netdev
In-Reply-To: <87ppt4yrbs.fsf@nemi.mork.no>
From: Bjørn Mork <bjorn@mork.no>
Date: Sat, 24 Aug 2013 00:28:55 +0200
> Stephen Hemminger <stephen@networkplumber.org> writes:
>> On Fri, 23 Aug 2013 11:35:04 +0200
>> Bjørn Mork <bjorn@mork.no> wrote:
>>
>>> /**
>>> + * eth_hw_addr_inherit - Copy dev_addr from another net_device
>>> + * @dst: pointer to net_device to copy dev_addr to
>>> + * @src: pointer to net_device to copy dev_addr from
>>> + *
>>> + * Copy the Ethernet address from one net_device to another along with
>>> + * the addr_assign_type.
>>> + */
>>> +static inline int eth_hw_addr_inherit(struct net_device *dst,
>>> + struct net_device *src)
>>> +{
>>> + if (dst->addr_len != src->addr_len)
>>> + return -EINVAL;
>>> +
>>> + dst->addr_assign_type = src->addr_assign_type;
>>> + memcpy(dst->dev_addr, src->dev_addr, dst->addr_len);
>>> + return 0;
>>> +}
>>> +
>>
>> Since all the other code in this file assumes addr_len == ETH_ALEN
>> why does this code need to handle variable addresses. Trivial but
>> then the memcpy is fixed size and can be optimized.
>
> Didn't know that. I'll make that change in the next version. Not that
> optimization matters much here, but consistency is always good.
You can bug check that addr_len ETH_ALEN if you want.
I realize that some drivers don't initialize it by this call point but
that is arguably a bug.
^ permalink raw reply
* Re: [net-next RFC 6/7] i40evf: init code and hardware support
From: David Miller @ 2013-08-27 18:44 UTC (permalink / raw)
To: john.ronciak
Cc: bhutchings, jeffrey.t.kirsher, mitch.a.williams, netdev, gospo,
sassmann
In-Reply-To: <CALWPMBHKfzvLpi9J+tqeFtmOzPANXw5ozdiXhSku_sQtVGg5yg@mail.gmail.com>
From: John Ronciak <john.ronciak@gmail.com>
Date: Tue, 27 Aug 2013 11:39:03 -0700
>> I wonder why this is thought to be necessary. I've never heard of
>> copyright assignment conditions being accepted in the Linux kernel.
>
>> Some Linux drivers are dual-licenced under a permissive licence and GPL,
>> and all contributions to them presumably also have to be dual-licenced.
>> It seems like that would allow you to retain the ability to reuse the
>> core hardware code on other operating systems and under proprietary
>> licences, even with copyrightable contributions from others.
>
> The problem here is that it's a legal issue. The Intel lawyers (and I've
> heard form other companies on this as well) say that just because we
> release code under a dual license does not mean that any changes (patches)
> that come back are also under a dual license. That cannot be assumed. So
> all changes would require us to ask that the patches in question are being
> supplied under the same dual license. This can get unmanageable fairly
> quickly. There is also long term tracking of those requests/responses. So
> using a dual license isn't the same thing unfortunately.
I think it's pretty anti-social to not accept changes made outside of
your group.
I don't want Intel ethernet drivers to go the way of ACPI where nobody
outside of you guys can make changes to the code.
I guarentee I will want to make changes to your drivers if an interface
is changed and I have to touch your code to keep the tree building when
a function signature changes.
To say that I am upset about this would be an understatement, it blocks
normal smooth development of networking infrastructure.
^ permalink raw reply
* Re: [patch] net/fec: "u32" is more explicit than "unsigned long"
From: David Miller @ 2013-08-27 18:51 UTC (permalink / raw)
To: bhutchings
Cc: dan.carpenter, grant.likely, rob.herring, fabio.estevam, Frank.Li,
jim_baxter, B38611, netdev, devicetree, kernel-janitors
In-Reply-To: <1377261869.3364.3.camel@bwh-desktop.uk.level5networks.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Fri, 23 Aug 2013 13:44:29 +0100
> On Fri, 2013-08-23 at 12:49 +0300, Dan Carpenter wrote:
>> tmpaddr[] is a six byte array. We want to set the first four bytes on
>> the first line and the remaining two on the next line. The code assumes
>> that "unsigned long" is 32 bits and obviously that's not true on 64 bit
>> arches. It's better to just use u32 instead.
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>> This is a static checker thing and I can't compile this file.
>>
>> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
>> index fdf9307..422b125 100644
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -1100,9 +1100,9 @@ static void fec_get_mac(struct net_device *ndev)
>> * 4) FEC mac registers set by bootloader
>> */
>> if (!is_valid_ether_addr(iap)) {
>> - *((unsigned long *) &tmpaddr[0]) =
>> + *((u32 *) &tmpaddr[0]) =
>> be32_to_cpu(readl(fep->hwp + FEC_ADDR_LOW));
>> - *((unsigned short *) &tmpaddr[4]) =
>> + *((u16 *) &tmpaddr[4]) =
>> be16_to_cpu(readl(fep->hwp + FEC_ADDR_HIGH) >> 16);
>> iap = &tmpaddr[0];
>> }
>
> This code also seems to have CPU vs big-endian byte order the wrong way
> round. readl() returns bytes in native order whereas we always store
> MAC addresses in network (big-endian) order. So I think it should be
> doing:
>
> *((__be32 *) &tmpaddr[0]) =
> cpu_to_be32(readl(fep->hwp + FEC_ADDR_LOW));
> *((__be16 *) &tmpaddr[4]) =
> cpu_to_be16(readl(fep->hwp + FEC_ADDR_HIGH) >> 16);
Dan please resubmit with Ben's suggested changes, thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] ipv4: IP_TOS and IP_TTL can be specified as ancillary data
From: David Miller @ 2013-08-27 18:56 UTC (permalink / raw)
To: ffusco; +Cc: netdev
In-Reply-To: <aaa7e0af2fe535ea5216e438b5f65db88dcef8b2.1377257704.git.ffusco@redhat.com>
From: Francesco Fusco <ffusco@redhat.com>
Date: Fri, 23 Aug 2013 14:19:32 +0200
> - changed the icmp_cookie ttl field from __s16 to __u8.
> A value of 0 means that the TTL has not been specified
Sorry, I have to ask you to change the ttl field type back to __s16
and use "-1" to mean not-specified.
Zero is a valid TTL setting and it means to not allow the
packet to leave this host.
Please make this change and resubmit, thanks.
^ permalink raw reply
* Re: [patch] net/fec: "u32" is more explicit than "unsigned long"
From: Dan Carpenter @ 2013-08-27 18:59 UTC (permalink / raw)
To: David Miller
Cc: bhutchings, grant.likely, rob.herring, fabio.estevam, Frank.Li,
jim_baxter, B38611, netdev, devicetree, kernel-janitors
In-Reply-To: <20130827.145144.478588680902912563.davem@davemloft.net>
On Tue, Aug 27, 2013 at 02:51:44PM -0400, David Miller wrote:
>
> Dan please resubmit with Ben's suggested changes, thanks.
Yes. Of course. Sorry for that, I'll resend tomorrow.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v2 1/4] jme: lower NAPI weight
From: David Miller @ 2013-08-27 19:00 UTC (permalink / raw)
To: mschmidt; +Cc: sergei.shtylyov, netdev, edumazet, cooldavid
In-Reply-To: <52176665.2030507@redhat.com>
From: Michal Schmidt <mschmidt@redhat.com>
Date: Fri, 23 Aug 2013 15:40:53 +0200
> Since commit 82dc3c63 ("net: introduce NAPI_POLL_WEIGHT")
> netif_napi_add() produces an error message if a NAPI poll weight
> greater than 64 is requested.
>
> jme requests a quarter of the rx ring size as the NAPI weight.
> jme's rx ring size is 1 << 9 = 512.
>
> Use the standard NAPI weight.
>
> v2: proper reference to the related commit
>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2 2/4] netxen: lower NAPI weight
From: David Miller @ 2013-08-27 19:00 UTC (permalink / raw)
To: mschmidt
Cc: sergei.shtylyov, netdev, edumazet, manish.chopra, sony.chacko,
rajesh.borundia
In-Reply-To: <52176675.3060202@redhat.com>
From: Michal Schmidt <mschmidt@redhat.com>
Date: Fri, 23 Aug 2013 15:41:09 +0200
> Since commit 82dc3c63 ("net: introduce NAPI_POLL_WEIGHT")
> netif_napi_add() produces an error message if a NAPI poll weight
> greater than 64 is requested.
>
> Use the standard NAPI weight.
>
> v2: proper reference to the related commit
>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2 3/4] ps3_gelic: lower NAPI weight
From: David Miller @ 2013-08-27 19:00 UTC (permalink / raw)
To: mschmidt; +Cc: geoff, netdev, edumazet, sergei.shtylyov
In-Reply-To: <5217667F.5010207@redhat.com>
From: Michal Schmidt <mschmidt@redhat.com>
Date: Fri, 23 Aug 2013 15:41:19 +0200
> Since commit 82dc3c63 ("net: introduce NAPI_POLL_WEIGHT")
> netif_napi_add() produces an error message if a NAPI poll weight
> greater than 64 is requested.
>
> GELIC_NET_NAPI_WEIGHT is defined to GELIC_NET_RX_DESCRIPTORS,
> which is 128.
>
> Use the standard NAPI weight.
>
> v2: proper reference to the related commit
>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> Acked-by: Geoff Levand <geoff@infradead.org>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] drivers:net: Convert dma_alloc_coherent(...__GFP_ZERO) to dma_zalloc_coherent
From: Jesse Brandeburg @ 2013-08-27 19:34 UTC (permalink / raw)
To: Joe Perches
Cc: netdev, linux-kernel, e1000-devel, linux-wireless, b43-dev, users,
jesse.brandeburg
In-Reply-To: <1377582323.2658.10.camel@joe-AO722>
On Mon, 26 Aug 2013 22:45:23 -0700
Joe Perches <joe@perches.com> wrote:
> __GFP_ZERO is an uncommon flag and perhaps is better
> not used. static inline dma_zalloc_coherent exists
> so convert the uses of dma_alloc_coherent with __GFP_ZERO
> to the more common kernel style with zalloc.
>
> Remove memset from the static inline dma_zalloc_coherent
> and add just one use of __GFP_ZERO instead.
>
> Trivially reduces the size of the existing uses of
> dma_zalloc_coherent.
>
> Realign arguments as appropriate.
>
> Signed-off-by: Joe Perches <joe@perches.com>
e1000 and ixgb bits:
Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
^ permalink raw reply
* Re: [PATCH net-next 0/5] qlcnic: DCB support
From: David Miller @ 2013-08-27 19:47 UTC (permalink / raw)
To: sucheta.chakraborty; +Cc: netdev, Dept-HSGLinuxNICDev
In-Reply-To: <1377279509-18869-1-git-send-email-sucheta.chakraborty@qlogic.com>
From: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Date: Fri, 23 Aug 2013 13:38:24 -0400
> This patch series will enable Data Center Bridging (DCB) feature in
> qlcnic driver. Currently the support is limited to CEE version of DCBX.
> Only GET operations are supported through CEE rtnetlink interface.
>
> Please apply to net-next.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] net: tcp_probe: allow more advanced ingress filtering by mark
From: David Miller @ 2013-08-27 19:54 UTC (permalink / raw)
To: dborkman; +Cc: netdev
In-Reply-To: <1377267393-24604-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann <dborkman@redhat.com>
Date: Fri, 23 Aug 2013 16:16:33 +0200
> Currently, the tcp_probe snooper can either filter packets by a given
> port (handed to the module via module parameter e.g. port=80) or lets
> all TCP traffic pass (port=0, default). When a port is specified, the
> port number is tested against the sk's source/destination port. Thus,
> if one of them matches, the information will be further processed for
> the log.
>
> As this is quite limited, allow for more advanced filtering possibilities
> which can facilitate debugging/analysis with the help of the tcp_probe
> snooper. Therefore, similarly as added to BPF machine in commit 7e75f93e
> ("pkt_sched: ingress socket filter by mark"), add the possibility to
> use skb->mark as a filter.
>
> If the mark is not being used otherwise, this allows ingress filtering
> by flow (e.g. in order to track updates from only a single flow, or a
> subset of all flows for a given port) and other things such as dynamic
> logging and reconfiguration without removing/re-inserting the tcp_probe
> module, etc. Simple example:
>
> insmod net/ipv4/tcp_probe.ko fwmark=8888 full=1
> ...
> iptables -A INPUT -i eth4 -t mangle -p tcp --dport 22 \
> --sport 60952 -j MARK --set-mark 8888
> [... sampling interval ...]
> iptables -D INPUT -i eth4 -t mangle -p tcp --dport 22 \
> --sport 60952 -j MARK --set-mark 8888
>
> The current option to filter by a given port is still being preserved. A
> similar approach could be done for the sctp_probe module as a follow-up.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
I'm fine with this, applied.
^ permalink raw reply
* Re: pull request: wireless 2013-08-23
From: David Miller @ 2013-08-27 19:55 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20130823162044.GD15114@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Fri, 23 Aug 2013 12:20:44 -0400
> This is one more set of fixes intended for the 3.11 stream...
>
> For the mac80211 bits, Johannes says:
>
> "I have three more patches for the 3.11 stream: Felix's fix for the
> fairly visible brcmsmac crash, a fix from Simon for an IBSS join bug I
> found and a fix for a channel context bug in IBSS I'd introduced."
>
> Along with those...
>
> Sujith Manoharan makes a minor change to not use a PLL hang workaroun
> for AR9550. This one-liner fixes a couple of bugs reported in the Red Hat
> bugzilla.
>
> Helmut Schaa addresses an ath9k_htc bug that mangles frame headers
> during Tx. This fix is small, tested by the bug reported and isolated
> to ath9k_htc.
>
> Stanislaw Gruszka reverts a recent iwl4965 change that broke rfkill
> notification to user space.
>
> Please let me know if there are problems!
Pulled, thanks John.
^ permalink raw reply
* Re: [PATCH next 0/6] be2net patch set
From: David Miller @ 2013-08-27 19:57 UTC (permalink / raw)
To: sathya.perla; +Cc: netdev
In-Reply-To: <1377602855-13920-1-git-send-email-sathya.perla@emulex.com>
From: Sathya Perla <sathya.perla@emulex.com>
Date: Tue, 27 Aug 2013 16:57:29 +0530
> This series includes code refactoring patches needed to cleanly implement
> ethtool set-channels features. Pls apply. Thanks.
>
> Sathya Perla (5):
> be2net: use EQ_CREATEv2 for SH-R
> be2net: refactor be_get_resources() code
> be2net: Fix be_cmd_if_create() to use MBOX if MCCQ is not created
> be2net: refactor be_setup() to consolidate queue creation routines
> be2net: implement ethtool set/get_channel hooks
>
> Vasundhara Volam (1):
> be2net: Fixup profile management routines
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH] VMXNET3: Add support for virtual IOMMU
From: David Miller @ 2013-08-27 20:02 UTC (permalink / raw)
To: acking; +Cc: pv-drivers, netdev, gregkh, linux-kernel, virtualization
In-Reply-To: <1377275629-14587-1-git-send-email-acking@vmware.com>
From: Andy King <acking@vmware.com>
Date: Fri, 23 Aug 2013 09:33:49 -0700
> This patch adds support for virtual IOMMU to the vmxnet3 module. We
> switch to DMA consistent mappings for anything we pass to the device.
> There were a few places where we already did this, but using pci_blah();
> these have been fixed to use dma_blah(), along with all new occurrences
> where we've replaced kmalloc() and friends.
>
> Also fix two small bugs:
> 1) use after free of rq->buf_info in vmxnet3_rq_destroy()
> 2) a cpu_to_le32() that should have been a cpu_to_le64()
>
> Acked-by: George Zhang <georgezhang@vmware.com>
> Acked-by: Aditya Sarwade <asarwade@vmware.com>
> Signed-off-by: Andy King <acking@vmware.com>
Applied, thanks.
^ permalink raw reply
* Re: [net-next 1/1] bna: firmware update to 3.2.1.1
From: David Miller @ 2013-08-27 20:03 UTC (permalink / raw)
To: rmody; +Cc: netdev, adapter_linux_open_src_team
In-Reply-To: <1377293490-7583-1-git-send-email-rmody@brocade.com>
From: Rasesh Mody <rmody@brocade.com>
Date: Fri, 23 Aug 2013 14:31:30 -0700
> This patch updates the firmware to address the thermal notification issue
>
> Signed-off-by: Rasesh Mody <rmody@brocade.com>
Applied.
^ permalink raw reply
* Re: [net-next v2] Documentation/networking/: Update Intel wired LAN driver documentation
From: David Miller @ 2013-08-27 20:05 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann
In-Reply-To: <1377303563-12136-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 23 Aug 2013 17:19:23 -0700
> Updates the documentation to the Intel wired LAN drivers.
>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Applied.
^ permalink raw reply
* Re: [net-next] e1000e: balance semaphore put/get for 82573
From: David Miller @ 2013-08-27 20:05 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: sla, netdev, gospo, sassmann
In-Reply-To: <1377303577-12180-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 23 Aug 2013 17:19:37 -0700
> From: Steven La <sla@riverbed.com>
>
> Steven (cc-ed) noticed an imbalance in semaphore put/get for
> 82573-based NICs. Don't we need something like the following
> (untested) patch?
>
> Signed-off-by: Steven La <sla@riverbed.com>
> Acked-by: Arthur Kepner <akepner@riverbed.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* [PATCH net-next 01/11] openvswitch: Use RCU lock for flow dump operation.
From: Jesse Gross @ 2013-08-27 20:20 UTC (permalink / raw)
To: David Miller; +Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1377634848-34327-1-git-send-email-jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
From: Pravin B Shelar <pshelar-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
Flow dump operation is read-only operation. There is no need to
take ovs-lock. Following patch use rcu-lock for dumping flows.
Signed-off-by: Pravin B Shelar <pshelar-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
---
net/openvswitch/datapath.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index f2ed760..e6fb866 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1104,7 +1104,6 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow, struct datapath *dp,
u32 seq, u32 flags, u8 cmd)
{
const int skb_orig_len = skb->len;
- const struct sw_flow_actions *sf_acts;
struct nlattr *start;
struct ovs_flow_stats stats;
struct ovs_header *ovs_header;
@@ -1113,8 +1112,6 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow, struct datapath *dp,
u8 tcp_flags;
int err;
- sf_acts = ovsl_dereference(flow->sf_acts);
-
ovs_header = genlmsg_put(skb, portid, seq, &dp_flow_genl_family, flags, cmd);
if (!ovs_header)
return -EMSGSIZE;
@@ -1161,6 +1158,11 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow, struct datapath *dp,
*/
start = nla_nest_start(skb, OVS_FLOW_ATTR_ACTIONS);
if (start) {
+ const struct sw_flow_actions *sf_acts;
+
+ sf_acts = rcu_dereference_check(flow->sf_acts,
+ lockdep_ovsl_is_held());
+
err = actions_to_attr(sf_acts->actions, sf_acts->actions_len, skb);
if (!err)
nla_nest_end(skb, start);
@@ -1440,15 +1442,14 @@ static int ovs_flow_cmd_dump(struct sk_buff *skb, struct netlink_callback *cb)
struct datapath *dp;
struct flow_table *table;
- ovs_lock();
+ rcu_read_lock();
dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
if (!dp) {
- ovs_unlock();
+ rcu_read_unlock();
return -ENODEV;
}
- table = ovsl_dereference(dp->table);
-
+ table = rcu_dereference(dp->table);
for (;;) {
struct sw_flow *flow;
u32 bucket, obj;
@@ -1468,7 +1469,7 @@ static int ovs_flow_cmd_dump(struct sk_buff *skb, struct netlink_callback *cb)
cb->args[0] = bucket;
cb->args[1] = obj;
}
- ovs_unlock();
+ rcu_read_unlock();
return skb->len;
}
--
1.8.1.2
^ permalink raw reply related
* [PATCH net-next 05/11] openvswitch: Fix argument descriptions in vport.c.
From: Jesse Gross @ 2013-08-27 20:20 UTC (permalink / raw)
To: David Miller; +Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1377634848-34327-1-git-send-email-jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
From: Justin Pettit <jpettit-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Justin Pettit <jpettit-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
---
net/openvswitch/vport.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index d69e0c0..6f65dbe 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -203,7 +203,7 @@ out:
* ovs_vport_set_options - modify existing vport device (for kernel callers)
*
* @vport: vport to modify.
- * @port: New configuration.
+ * @options: New configuration.
*
* Modifies an existing device with the specified configuration (which is
* dependent on device type). ovs_mutex must be held.
@@ -328,6 +328,7 @@ int ovs_vport_get_options(const struct vport *vport, struct sk_buff *skb)
*
* @vport: vport that received the packet
* @skb: skb that was received
+ * @tun_key: tunnel (if any) that carried packet
*
* Must be called with rcu_read_lock. The packet cannot be shared and
* skb->data should point to the Ethernet header.
--
1.8.1.2
^ permalink raw reply related
* [GIT net-next] Open vSwitch
From: Jesse Gross @ 2013-08-27 20:20 UTC (permalink / raw)
To: David Miller; +Cc: netdev, dev
A number of significant new features and optimizations for net-next/3.12.
Highlights are:
* "Megaflows", an optimization that allows userspace to specify which
flow fields were used to compute the results of the flow lookup.
This allows for a major reduction in flow setups (the major
performance bottleneck in Open vSwitch) without reducing flexibility.
* Converting netlink dump operations to use RCU, allowing for
additional parallelism in userspace.
* Matching and modifying SCTP protocol fields.
The following changes since commit 2771399ac9986c75437a83b1c723493cfcdfa439:
fs_enet: cleanup clock API use (2013-08-22 22:13:54 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jesse/openvswitch.git master
for you to fetch changes up to 5828cd9a68873df1340b420371c02c47647878fb:
openvswitch: optimize flow compare and mask functions (2013-08-27 13:13:09 -0700)
----------------------------------------------------------------
Andy Zhou (3):
openvswitch: Mega flow implementation
openvswitch: Rename key_len to key_end
openvswitch: optimize flow compare and mask functions
Cong Wang (1):
openvswitch: check CONFIG_OPENVSWITCH_GRE in makefile
Jiri Pirko (1):
openvswitch:: link upper device for port devices
Joe Stringer (2):
net: Add NEXTHDR_SCTP to ipv6.h
openvswitch: Add SCTP support
Justin Pettit (1):
openvswitch: Fix argument descriptions in vport.c.
Pravin B Shelar (3):
openvswitch: Use RCU lock for flow dump operation.
openvswitch: Use RCU lock for dp dump operation.
openvswitch: Use non rcu hlist_del() flow table entry.
Documentation/networking/openvswitch.txt | 40 +
include/net/ipv6.h | 1 +
include/uapi/linux/openvswitch.h | 15 +-
net/openvswitch/Kconfig | 1 +
net/openvswitch/Makefile | 5 +-
net/openvswitch/actions.c | 45 +-
net/openvswitch/datapath.c | 176 ++--
net/openvswitch/datapath.h | 6 +
net/openvswitch/flow.c | 1485 +++++++++++++++++++++---------
net/openvswitch/flow.h | 89 +-
net/openvswitch/vport-gre.c | 3 -
net/openvswitch/vport-netdev.c | 20 +-
net/openvswitch/vport.c | 3 +-
13 files changed, 1346 insertions(+), 543 deletions(-)
^ permalink raw reply
* [PATCH net-next 02/11] openvswitch: Use RCU lock for dp dump operation.
From: Jesse Gross @ 2013-08-27 20:20 UTC (permalink / raw)
To: David Miller; +Cc: netdev, dev, Pravin B Shelar, Jesse Gross
In-Reply-To: <1377634848-34327-1-git-send-email-jesse@nicira.com>
From: Pravin B Shelar <pshelar@nicira.com>
RCUfy dp-dump operation which is already read-only. This
makes all ovs dump operations lockless.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
net/openvswitch/datapath.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index e6fb866..9d97ef3 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -951,9 +951,10 @@ static struct genl_ops dp_packet_genl_ops[] = {
static void get_dp_stats(struct datapath *dp, struct ovs_dp_stats *stats)
{
+ struct flow_table *table;
int i;
- struct flow_table *table = ovsl_dereference(dp->table);
+ table = rcu_dereference_check(dp->table, lockdep_ovsl_is_held());
stats->n_flows = ovs_flow_tbl_count(table);
stats->n_hit = stats->n_missed = stats->n_lost = 0;
@@ -1665,7 +1666,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
goto err_destroy_local_port;
ovs_net = net_generic(ovs_dp_get_net(dp), ovs_net_id);
- list_add_tail(&dp->list_node, &ovs_net->dps);
+ list_add_tail_rcu(&dp->list_node, &ovs_net->dps);
ovs_unlock();
@@ -1703,7 +1704,7 @@ static void __dp_destroy(struct datapath *dp)
ovs_dp_detach_port(vport);
}
- list_del(&dp->list_node);
+ list_del_rcu(&dp->list_node);
/* OVSP_LOCAL is datapath internal port. We need to make sure that
* all port in datapath are destroyed first before freeing datapath.
@@ -1808,8 +1809,8 @@ static int ovs_dp_cmd_dump(struct sk_buff *skb, struct netlink_callback *cb)
int skip = cb->args[0];
int i = 0;
- ovs_lock();
- list_for_each_entry(dp, &ovs_net->dps, list_node) {
+ rcu_read_lock();
+ list_for_each_entry_rcu(dp, &ovs_net->dps, list_node) {
if (i >= skip &&
ovs_dp_cmd_fill_info(dp, skb, NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, NLM_F_MULTI,
@@ -1817,7 +1818,7 @@ static int ovs_dp_cmd_dump(struct sk_buff *skb, struct netlink_callback *cb)
break;
i++;
}
- ovs_unlock();
+ rcu_read_unlock();
cb->args[0] = i;
--
1.8.1.2
^ permalink raw reply related
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