* Re: [PATCH 3/4] sctp: several declared/set but unused fixes
From: David Miller @ 2011-03-07 23:51 UTC (permalink / raw)
To: hagen; +Cc: netdev
In-Reply-To: <1299275106-11513-3-git-send-email-hagen@jauu.net>
From: Hagen Paul Pfeifer <hagen@jauu.net>
Date: Fri, 4 Mar 2011 22:45:05 +0100
> Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>
Applied.
^ permalink raw reply
* Re: [PATCH 2/4] af_packet: struct socket declared/assigned but unused
From: David Miller @ 2011-03-07 23:51 UTC (permalink / raw)
To: hagen; +Cc: netdev
In-Reply-To: <1299275106-11513-2-git-send-email-hagen@jauu.net>
From: Hagen Paul Pfeifer <hagen@jauu.net>
Date: Fri, 4 Mar 2011 22:45:04 +0100
> Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>
Applied.
^ permalink raw reply
* Re: [PATCH 1/4] mcast: net_device dev not used
From: David Miller @ 2011-03-07 23:51 UTC (permalink / raw)
To: hagen; +Cc: netdev
In-Reply-To: <1299275106-11513-1-git-send-email-hagen@jauu.net>
From: Hagen Paul Pfeifer <hagen@jauu.net>
Date: Fri, 4 Mar 2011 22:45:03 +0100
> ip6_mc_source(), ip6_mc_msfilter() as well as ip6_mc_msfget() declare
> and assign dev but do not use the variable afterwards.
>
> Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>
Applied.
^ permalink raw reply
* Re: [PATCH] s2io: fix uninitialized compile warning
From: David Miller @ 2011-03-07 23:49 UTC (permalink / raw)
To: shanwei
Cc: ramkrishna.vepa, sivakumar.subramani, sreenivasa.honnur,
jon.mason, netdev
In-Reply-To: <4D70CBCE.4090306@cn.fujitsu.com>
From: Shan Wei <shanwei@cn.fujitsu.com>
Date: Fri, 04 Mar 2011 19:23:58 +0800
>
> drivers/net/s2io.c:7559: warning: ‘tcp_len’ may be used uninitialized in this function
>
> Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 2/2 v2] bonding: COW before overwriting the destination MAC address
From: David Miller @ 2011-03-07 23:45 UTC (permalink / raw)
To: xiaosuo; +Cc: fubar, eric.dumazet, netdev
In-Reply-To: <1299136034-5549-1-git-send-email-xiaosuo@gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Thu, 3 Mar 2011 15:07:14 +0800
> When there is a ptype handler holding a clone of this skb, whose
> destination MAC addresse is overwritten, the owner of this handler may
> get a corrupted packet.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/2] bonding: remove the unused dummy functions when net poll controller isn't enabled
From: David Miller @ 2011-03-07 23:45 UTC (permalink / raw)
To: xiaosuo; +Cc: fubar, netdev
In-Reply-To: <1299126381-9338-1-git-send-email-xiaosuo@gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Thu, 3 Mar 2011 12:26:21 +0800
> These two functions are only used when net poll controller is enabled.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/4] r8169 driver updates
From: David Miller @ 2011-03-07 23:40 UTC (permalink / raw)
To: romieu; +Cc: netdev, hayeswang, jesse, oneukum, dwmw2, ben
In-Reply-To: <20110305093716.GA13835@electric-eye.fr.zoreil.com>
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Sat, 5 Mar 2011 10:37:16 +0100
> git://git.kernel.org/pub/scm/linux/kernel/git/romieu/netdev-2.6.git davem-next.r8169
>
> to get the changes below.
Pulled, thanks a lot.
^ permalink raw reply
* Re: net: allow handlers to be processed for orig_dev
From: David Miller @ 2011-03-07 23:37 UTC (permalink / raw)
To: nicolas.2p.debian
Cc: jpirko, netdev, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <4D6CAD33.2090706@gmail.com>
From: Nicolas de Pesloüan <nicolas.2p.debian@gmail.com>
Date: Tue, 01 Mar 2011 09:24:19 +0100
> Le 01/03/2011 07:26, Jiri Pirko a écrit :
>> This was there before, I forgot about this. Allows deliveries to
>> ptype_base handlers registered for orig_dev. I presume this is still
>> desired.
>
> Yes, it is. Sorry for missing it in my review.
>
>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>
> Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net: fix multithreaded signal handling in unix recv routines
From: David Miller @ 2011-03-07 23:31 UTC (permalink / raw)
To: rweikusat; +Cc: netdev, linux-kernel
In-Reply-To: <877hck43hs.fsf@sapphire.mobileactivedefense.com>
From: Rainer Weikusat <rweikusat@mobileactivedefense.com>
Date: Mon, 28 Feb 2011 14:50:55 +0000
> From: Rainer Weikusat <rweikusat@mobileactivedefense.com>
>
> The unix_dgram_recvmsg and unix_stream_recvmsg routines in
> net/af_unix.c utilize mutex_lock(&u->readlock) calls in order to
> serialize read operations of multiple threads on a single socket. This
> implies that, if all n threads of a process block in an AF_UNIX recv
> call trying to read data from the same socket, one of these threads
> will be sleeping in state TASK_INTERRUPTIBLE and all others in state
> TASK_UNINTERRUPTIBLE. Provided that a particular signal is supposed to
> be handled by a signal handler defined by the process and that none of
> this threads is blocking the signal, the complete_signal routine in
> kernel/signal.c will select the 'first' such thread it happens to
> encounter when deciding which thread to notify that a signal is
> supposed to be handled and if this is one of the TASK_UNINTERRUPTIBLE
> threads, the signal won't be handled until the one thread not blocking
> on the u->readlock mutex is woken up because some data to process has
> arrived (if this ever happens). The included patch fixes this by
> changing mutex_lock to mutex_lock_interruptible and handling possible
> error returns in the same way interruptions are handled by the actual
> receive-code.
>
> Signed-off-by: Rainer Weikusat <rweikusat@mobileactivedefense.com>
Looks good, applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next-2.6 0/7] be2net: Patch series for supporting Lancer
From: David Miller @ 2011-03-07 23:27 UTC (permalink / raw)
To: padmanabh.ratnakar; +Cc: netdev, sathya.perla, subbu.seetharaman
In-Reply-To: <20cba747-f885-47d5-b27d-788efc426bec@exht1.ad.emulex.com>
From: Padmanabh Ratnakar <padmanabh.ratnakar@emulex.com>
Date: Mon, 7 Mar 2011 18:35:20 +0530
> Hi David,
> Following are patches to support Lancer family of CNA.
> Please apply.
All applied to net-next-2.6, thanks.
^ permalink raw reply
* [PATCH net-next-2.6] sfc: Use write-combining to reduce TX latency
From: Ben Hutchings @ 2011-03-07 23:25 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1299540194.2522.46.camel@bwh-desktop>
Based on work by Neil Turton <nturton@solarflare.com> and
Kieran Mansley <kmansley@solarflare.com>.
The BIU has now been verified to handle 3- and 4-dword writes within a
single 128-bit register correctly. This means we can enable write-
combining and only insert write barriers between writes to distinct
registers.
This has been observed to save about 0.5 us when pushing a TX
descriptor to an empty TX queue.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/sfc/efx.c | 4 ++--
drivers/net/sfc/io.h | 13 +++++++++----
drivers/net/sfc/mcdi.c | 9 +++++----
3 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index d563049..b8bd936 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -1104,8 +1104,8 @@ static int efx_init_io(struct efx_nic *efx)
rc = -EIO;
goto fail3;
}
- efx->membase = ioremap_nocache(efx->membase_phys,
- efx->type->mem_map_size);
+ efx->membase = ioremap_wc(efx->membase_phys,
+ efx->type->mem_map_size);
if (!efx->membase) {
netif_err(efx, probe, efx->net_dev,
"could not map memory BAR at %llx+%x\n",
diff --git a/drivers/net/sfc/io.h b/drivers/net/sfc/io.h
index dc45110..d9d8c2e 100644
--- a/drivers/net/sfc/io.h
+++ b/drivers/net/sfc/io.h
@@ -48,9 +48,9 @@
* replacing the low 96 bits with zero does not affect functionality.
* - If the host writes to the last dword address of such a register
* (i.e. the high 32 bits) the underlying register will always be
- * written. If the collector does not hold values for the low 96
- * bits of the register, they will be written as zero. Writing to
- * the last qword does not have this effect and must not be done.
+ * written. If the collector and the current write together do not
+ * provide values for all 128 bits of the register, the low 96 bits
+ * will be written as zero.
* - If the host writes to the address of any other part of such a
* register while the collector already holds values for some other
* register, the write is discarded and the collector maintains its
@@ -103,6 +103,7 @@ static inline void efx_writeo(struct efx_nic *efx, efx_oword_t *value,
_efx_writed(efx, value->u32[2], reg + 8);
_efx_writed(efx, value->u32[3], reg + 12);
#endif
+ wmb();
mmiowb();
spin_unlock_irqrestore(&efx->biu_lock, flags);
}
@@ -125,6 +126,7 @@ static inline void efx_sram_writeq(struct efx_nic *efx, void __iomem *membase,
__raw_writel((__force u32)value->u32[0], membase + addr);
__raw_writel((__force u32)value->u32[1], membase + addr + 4);
#endif
+ wmb();
mmiowb();
spin_unlock_irqrestore(&efx->biu_lock, flags);
}
@@ -139,6 +141,7 @@ static inline void efx_writed(struct efx_nic *efx, efx_dword_t *value,
/* No lock required */
_efx_writed(efx, value->u32[0], reg);
+ wmb();
}
/* Read a 128-bit CSR, locking as appropriate. */
@@ -237,12 +240,14 @@ static inline void _efx_writeo_page(struct efx_nic *efx, efx_oword_t *value,
#ifdef EFX_USE_QWORD_IO
_efx_writeq(efx, value->u64[0], reg + 0);
+ _efx_writeq(efx, value->u64[1], reg + 8);
#else
_efx_writed(efx, value->u32[0], reg + 0);
_efx_writed(efx, value->u32[1], reg + 4);
-#endif
_efx_writed(efx, value->u32[2], reg + 8);
_efx_writed(efx, value->u32[3], reg + 12);
+#endif
+ wmb();
}
#define efx_writeo_page(efx, value, reg, page) \
_efx_writeo_page(efx, value, \
diff --git a/drivers/net/sfc/mcdi.c b/drivers/net/sfc/mcdi.c
index 8bba895..5e118f0 100644
--- a/drivers/net/sfc/mcdi.c
+++ b/drivers/net/sfc/mcdi.c
@@ -94,14 +94,15 @@ static void efx_mcdi_copyin(struct efx_nic *efx, unsigned cmd,
efx_writed(efx, &hdr, pdu);
- for (i = 0; i < inlen; i += 4)
+ for (i = 0; i < inlen; i += 4) {
_efx_writed(efx, *((__le32 *)(inbuf + i)), pdu + 4 + i);
-
- /* Ensure the payload is written out before the header */
- wmb();
+ /* use wmb() within loop to inhibit write combining */
+ wmb();
+ }
/* ring the doorbell with a distinctive value */
_efx_writed(efx, (__force __le32) 0x45789abc, doorbell);
+ wmb();
}
static void efx_mcdi_copyout(struct efx_nic *efx, u8 *outbuf, size_t outlen)
--
1.7.4
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 related
* pull request: sfc-next-2.6 2011-03-07
From: Ben Hutchings @ 2011-03-07 23:23 UTC (permalink / raw)
To: David Miller; +Cc: sf-linux-drivers, netdev
The following changes since commit 07df5294a753dfac2cc9f75e6159fc25fdc22149:
inet: Replace left-over references to inet->cork (2011-03-01 23:00:58 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next-2.6.git for-davem
Just one more performance optimisation.
Ben.
Ben Hutchings (1):
sfc: Use write-combining to reduce TX latency
drivers/net/sfc/efx.c | 4 ++--
drivers/net/sfc/io.h | 13 +++++++++----
drivers/net/sfc/mcdi.c | 9 +++++----
3 files changed, 16 insertions(+), 10 deletions(-)
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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
* [PATCH 2/2] ipv4: Inline fib_semantic_match into check_leaf
From: David Miller @ 2011-03-07 23:13 UTC (permalink / raw)
To: netdev
This elimiates a lot of pure overhead due to parameter
passing.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/ipv4/fib_lookup.h | 7 +++++
net/ipv4/fib_semantics.c | 68 +---------------------------------------------
net/ipv4/fib_trie.c | 51 +++++++++++++++++++++++++++++-----
3 files changed, 51 insertions(+), 75 deletions(-)
diff --git a/net/ipv4/fib_lookup.h b/net/ipv4/fib_lookup.h
index d5c40d8..84db2da 100644
--- a/net/ipv4/fib_lookup.h
+++ b/net/ipv4/fib_lookup.h
@@ -51,4 +51,11 @@ static inline void fib_result_assign(struct fib_result *res,
res->fi = fi;
}
+struct fib_prop {
+ int error;
+ u8 scope;
+};
+
+extern const struct fib_prop fib_props[RTN_MAX + 1];
+
#endif /* _FIB_LOOKUP_H */
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index c29291b..6349a21 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -90,11 +90,7 @@ static DEFINE_SPINLOCK(fib_multipath_lock);
#define endfor_nexthops(fi) }
-static const struct
-{
- int error;
- u8 scope;
-} fib_props[RTN_MAX + 1] = {
+const struct fib_prop fib_props[RTN_MAX + 1] = {
[RTN_UNSPEC] = {
.error = 0,
.scope = RT_SCOPE_NOWHERE,
@@ -902,68 +898,6 @@ failure:
return ERR_PTR(err);
}
-/* Note! fib_semantic_match intentionally uses RCU list functions. */
-int fib_semantic_match(struct fib_table *tb, struct list_head *head,
- const struct flowi *flp, struct fib_result *res,
- int prefixlen, int fib_flags)
-{
- struct fib_alias *fa;
- int nh_sel = 0;
-
- list_for_each_entry_rcu(fa, head, fa_list) {
- int err;
-
- if (fa->fa_tos &&
- fa->fa_tos != flp->fl4_tos)
- continue;
-
- if (fa->fa_scope < flp->fl4_scope)
- continue;
-
- fib_alias_accessed(fa);
-
- err = fib_props[fa->fa_type].error;
- if (err == 0) {
- struct fib_info *fi = fa->fa_info;
-
- if (fi->fib_flags & RTNH_F_DEAD)
- continue;
-
- for_nexthops(fi) {
- if (nh->nh_flags & RTNH_F_DEAD)
- continue;
- if (!flp->oif || flp->oif == nh->nh_oif)
- break;
- }
-#ifdef CONFIG_IP_ROUTE_MULTIPATH
- if (nhsel < fi->fib_nhs) {
- nh_sel = nhsel;
- goto out_fill_res;
- }
-#else
- if (nhsel < 1)
- goto out_fill_res;
-#endif
- endfor_nexthops(fi);
- continue;
- }
- return err;
- }
- return 1;
-
-out_fill_res:
- res->prefixlen = prefixlen;
- res->nh_sel = nh_sel;
- res->type = fa->fa_type;
- res->scope = fa->fa_scope;
- res->fi = fa->fa_info;
- res->table = tb;
- res->fa_head = head;
- if (!(fib_flags & FIB_LOOKUP_NOREF))
- atomic_inc(&res->fi->fib_clntref);
- return 0;
-}
-
/* Find appropriate source address to this destination */
__be32 __fib_res_prefsrc(struct fib_result *res)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index edf3b09..057062b 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1349,23 +1349,58 @@ static int check_leaf(struct fib_table *tb, struct trie *t, struct leaf *l,
struct hlist_node *node;
hlist_for_each_entry_rcu(li, node, hhead, hlist) {
- int err;
+ struct fib_alias *fa;
int plen = li->plen;
__be32 mask = inet_make_mask(plen);
if (l->key != (key & ntohl(mask)))
continue;
- err = fib_semantic_match(tb, &li->falh, flp, res, plen, fib_flags);
+ list_for_each_entry_rcu(fa, &li->falh, fa_list) {
+ struct fib_info *fi = fa->fa_info;
+ int nhsel, err;
+
+ if (fa->fa_tos && fa->fa_tos != flp->fl4_tos)
+ continue;
+ if (fa->fa_scope < flp->fl4_scope)
+ continue;
+ fib_alias_accessed(fa);
+ err = fib_props[fa->fa_type].error;
+ if (err) {
+ifdef CONFIG_IP_FIB_TRIE_STATS
+ t->stats.semantic_match_miss++;
+#endif
+ return 1;
+ }
+ if (fi->fib_flags & RTNH_F_DEAD)
+ continue;
+ for (nhsel = 0; nhsel < fi->fib_nhs; nhsel++) {
+ const struct fib_nh *nh = &fi->fib_nh[nhsel];
+
+ if (nh->nh_flags & RTNH_F_DEAD)
+ continue;
+ if (flp->oif && flp->oif != nh->nh_oif)
+ continue;
#ifdef CONFIG_IP_FIB_TRIE_STATS
- if (err <= 0)
- t->stats.semantic_match_passed++;
- else
- t->stats.semantic_match_miss++;
+ t->stats.semantic_match_passed++;
+#endif
+ res->prefixlen = plen;
+ res->nh_sel = nhsel;
+ res->type = fa->fa_type;
+ res->scope = fa->fa_scope;
+ res->fi = fi;
+ res->table = tb;
+ res->fa_head = &li->falh;
+ if (!(fib_flags & FIB_LOOKUP_NOREF))
+ atomic_inc(&res->fi->fib_clntref);
+ return 0;
+ }
+ }
+
+#ifdef CONFIG_IP_FIB_TRIE_STATS
+ t->stats.semantic_match_miss++;
#endif
- if (err <= 0)
- return err;
}
return 1;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 1/2] ipv4: Validate route entry type at insert instead of every lookup.
From: David Miller @ 2011-03-07 23:13 UTC (permalink / raw)
To: netdev
fib_semantic_match() requires that if the type doesn't signal an
automatic error, it must be of type RTN_UNICAST, RTN_LOCAL,
RTN_BROADCAST, RTN_ANYCAST, or RTN_MULTICAST.
Checking this every route lookup is pointless work.
Instead validate it during route insertion, via fib_create_info().
Also, there was nothing making sure the type value was less than
RTN_MAX, so add that missing check while we're here.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/ipv4/fib_semantics.c | 54 +++++++++++++++++++++++----------------------
1 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 562f34c..c29291b 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -707,6 +707,9 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
int nhs = 1;
struct net *net = cfg->fc_nlinfo.nl_net;
+ if (cfg->fc_type > RTN_MAX)
+ goto err_inval;
+
/* Fast check to catch the most weird cases */
if (fib_props[cfg->fc_type].scope > cfg->fc_scope)
goto err_inval;
@@ -812,6 +815,17 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
if (cfg->fc_gw || cfg->fc_oif || cfg->fc_mp)
goto err_inval;
goto link_it;
+ } else {
+ switch (cfg->fc_type) {
+ case RTN_UNICAST:
+ case RTN_LOCAL:
+ case RTN_BROADCAST:
+ case RTN_ANYCAST:
+ case RTN_MULTICAST:
+ break;
+ default:
+ goto err_inval;
+ }
}
if (cfg->fc_scope > RT_SCOPE_HOST)
@@ -915,35 +929,23 @@ int fib_semantic_match(struct fib_table *tb, struct list_head *head,
if (fi->fib_flags & RTNH_F_DEAD)
continue;
- switch (fa->fa_type) {
- case RTN_UNICAST:
- case RTN_LOCAL:
- case RTN_BROADCAST:
- case RTN_ANYCAST:
- case RTN_MULTICAST:
- for_nexthops(fi) {
- if (nh->nh_flags & RTNH_F_DEAD)
- continue;
- if (!flp->oif || flp->oif == nh->nh_oif)
- break;
- }
+ for_nexthops(fi) {
+ if (nh->nh_flags & RTNH_F_DEAD)
+ continue;
+ if (!flp->oif || flp->oif == nh->nh_oif)
+ break;
+ }
#ifdef CONFIG_IP_ROUTE_MULTIPATH
- if (nhsel < fi->fib_nhs) {
- nh_sel = nhsel;
- goto out_fill_res;
- }
+ if (nhsel < fi->fib_nhs) {
+ nh_sel = nhsel;
+ goto out_fill_res;
+ }
#else
- if (nhsel < 1)
- goto out_fill_res;
+ if (nhsel < 1)
+ goto out_fill_res;
#endif
- endfor_nexthops(fi);
- continue;
-
- default:
- pr_warning("fib_semantic_match bad type %#x\n",
- fa->fa_type);
- return -EINVAL;
- }
+ endfor_nexthops(fi);
+ continue;
}
return err;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 0/2] fib semantic matching
From: David Miller @ 2011-03-07 23:13 UTC (permalink / raw)
To: netdev
fib_semantic_match() creates unnecessary overhead on route lookups
for two reasons:
1) It does type validation at run time instead of when we insert
the route.
2) Argument passing is expensive because it does it's work as
a seperate function.
So these two patches move type validation into fib_create_info(),
and then we inline fib_semantic_match() completely into check_leaf().
This is good for 50+ cycle gain on output route lookups on Niagara2.
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [patch net-next-2.6] net: reinject arps into bonding slave instead of master
From: Nicolas de Pesloüan @ 2011-03-07 23:09 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jiri Pirko, netdev, davem, shemminger, kaber, fubar, eric.dumazet
In-Reply-To: <20110307224338.GU11864@gospo.rdu.redhat.com>
Le 07/03/2011 23:43, Andy Gospodarek a écrit :
> On Mon, Mar 07, 2011 at 01:51:00PM +0100, Jiri Pirko wrote:
>> Recent patch "bonding: move processing of recv handlers into
>> handle_frame()" caused a regression on following net scheme:
>>
>> eth0 - bond0 - bond0.5
>>
>> where arp monitoring is happening over vlan. This patch fixes it by
>> reinjecting the arp packet into bonding slave device so the bonding
>> rx_handler can pickup and process it.
>>
>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>> ---
>> net/core/dev.c | 8 ++++----
>> 1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index c71bd18..3d88458 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -3094,12 +3094,12 @@ void netdev_rx_handler_unregister(struct net_device *dev)
>> }
>> EXPORT_SYMBOL_GPL(netdev_rx_handler_unregister);
>>
>> -static void vlan_on_bond_hook(struct sk_buff *skb)
>> +static void vlan_on_bond_hook(struct sk_buff *skb, struct net_device *orig_dev)
>> {
>> /*
>> * Make sure ARP frames received on VLAN interfaces stacked on
>> * bonding interfaces still make their way to any base bonding
>> - * device that may have registered for a specific ptype.
>> + * device by reinjecting the frame into bonding slave (orig_dev)
>> */
>> if (skb->dev->priv_flags& IFF_802_1Q_VLAN&&
>> vlan_dev_real_dev(skb->dev)->priv_flags& IFF_BONDING&&
>> @@ -3108,7 +3108,7 @@ static void vlan_on_bond_hook(struct sk_buff *skb)
>>
>> if (!skb2)
>> return;
>> - skb2->dev = vlan_dev_real_dev(skb->dev);
>> + skb2->dev = orig_dev;
>> netif_rx(skb2);
>> }
>> }
>> @@ -3202,7 +3202,7 @@ ncls:
>> goto out;
>> }
>>
>> - vlan_on_bond_hook(skb);
>> + vlan_on_bond_hook(skb, orig_dev);
>>
>> /* deliver only exact match when indicated */
>> null_or_dev = deliver_exact ? skb->dev : NULL;
>
> This patch doesn't work.
>
> My setup has bond0.100 -> bond0 -> eth2 and eth3. ARP monitoring is
> enabled as is arp_valiate.
>
> The initial problem was just that just before vlan_on_bond_hook is
> called, skb->dev = bond0.100 and orig_dev = eth2. (This is after
> running goto another_route and having been called back through
> __netif_receive_skb since vlan_hwaccel_do_receive it true.)
>
> Now vlan_on_bond_hook is called and we have 2 skbs.
>
> The original skb still have skb->dev = bond0.100 and orig_dev = eth2.
> Since bond_arp_rcv is registered for traffic only to bond0, the handler
> is not hit and the frame is dropped (or processed by another handler).
After Jiri's last patch series, bond_arp_rcv() is not registered anymore as a protocol handler on
bond0, but directly called from inside bond_handle_frame(), through bond->recv_probe.
Because bond_handler_frame() is a rx_handler for the slave interfaces, bond_arp_rcv() is now called
at the slave level and not a the master level anymore.
Hence this patch and the reason I thought it should work.
Did you tested this patch with Jiri's previous patches applied before?
> The cloned skb has skb->dev = bond0 and is put back on the receive queue
> and comes back through __netif_receive_skb. This frame will match the
> ptype entry for bond_arp_rcv, but since orig_dev = bond0 in this case,
> the code in bond_arp_rcv will not handle the frame.
I definitely hate all those unnecessary reinjects from rx_handler. The another_round loop is
designed to allow for stacking inside __netif_receive_skb().
Jiri apparently has another (better) solution in mind. I hope to see it, but Jiri arguably want some
of the patchs in the queue to flow before adding more.
Does someone had a look at my proposal of a late_delivery property for packet_type, previously in
this thread, to handle the situation where a given protocol handler registered on a particular
device would like to receive the final skb instead of the one at the time it crossed that particular
device?
>
> If we truly want to track the original interface that received the
> frame, the following is a better option. With the recursive nature of
> __netif_receive_skb at this point, we should really consider setting
> orig_dev from skb_iif rather than just from skb->dev.
Or we can remove all this orig_dev stuff...
Nicolas.
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 30440e7..500fdbc 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3135,7 +3135,6 @@ static int __netif_receive_skb(struct sk_buff *skb)
>
> if (!skb->skb_iif)
> skb->skb_iif = skb->dev->ifindex;
> - orig_dev = skb->dev;
>
> skb_reset_network_header(skb);
> skb_reset_transport_header(skb);
> @@ -3145,6 +3144,7 @@ static int __netif_receive_skb(struct sk_buff *skb)
>
> rcu_read_lock();
>
> + orig_dev = dev_get_by_index_rcu(dev_net(skb->dev),skb->skb_iif);
> another_round:
>
> __this_cpu_inc(softnet_data.processed);
>
>
^ permalink raw reply
* Re: [PATCH] drivers/net: fix build warnings with CONFIG_PM_SLEEP disabled
From: Stephen Hemminger @ 2011-03-07 23:07 UTC (permalink / raw)
To: Michel Lespinasse
Cc: David S. Miller, netdev, Andrew Morton, Linus Torvalds,
linux-kernel
In-Reply-To: <20110307021450.GC31188@google.com>
On Sun, 6 Mar 2011 18:14:50 -0800
Michel Lespinasse <walken@google.com> wrote:
> This fixes a couple of build warnings when CONFIG_PM is enabled but
> CONFIG_PM_SLEEP is disabled. Applies on top of v2.6.38-rc7 - I know it's
> late, but it would be great if v2.6.38 could compile without warnings!
>
> Signed-off-by: Michel Lespinasse <walken@google.com>
This is standard throughout all the network drivers.
Why just fix 2?
--
^ permalink raw reply
* Re: [patch net-next-2.6] net: reinject arps into bonding slave instead of master
From: Andy Gospodarek @ 2011-03-07 22:43 UTC (permalink / raw)
To: Jiri Pirko
Cc: Nicolas de Pesloüan, netdev, davem, shemminger, kaber, fubar,
eric.dumazet, andy
In-Reply-To: <20110307125059.GA6053@psychotron.brq.redhat.com>
On Mon, Mar 07, 2011 at 01:51:00PM +0100, Jiri Pirko wrote:
> Recent patch "bonding: move processing of recv handlers into
> handle_frame()" caused a regression on following net scheme:
>
> eth0 - bond0 - bond0.5
>
> where arp monitoring is happening over vlan. This patch fixes it by
> reinjecting the arp packet into bonding slave device so the bonding
> rx_handler can pickup and process it.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
> net/core/dev.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c71bd18..3d88458 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3094,12 +3094,12 @@ void netdev_rx_handler_unregister(struct net_device *dev)
> }
> EXPORT_SYMBOL_GPL(netdev_rx_handler_unregister);
>
> -static void vlan_on_bond_hook(struct sk_buff *skb)
> +static void vlan_on_bond_hook(struct sk_buff *skb, struct net_device *orig_dev)
> {
> /*
> * Make sure ARP frames received on VLAN interfaces stacked on
> * bonding interfaces still make their way to any base bonding
> - * device that may have registered for a specific ptype.
> + * device by reinjecting the frame into bonding slave (orig_dev)
> */
> if (skb->dev->priv_flags & IFF_802_1Q_VLAN &&
> vlan_dev_real_dev(skb->dev)->priv_flags & IFF_BONDING &&
> @@ -3108,7 +3108,7 @@ static void vlan_on_bond_hook(struct sk_buff *skb)
>
> if (!skb2)
> return;
> - skb2->dev = vlan_dev_real_dev(skb->dev);
> + skb2->dev = orig_dev;
> netif_rx(skb2);
> }
> }
> @@ -3202,7 +3202,7 @@ ncls:
> goto out;
> }
>
> - vlan_on_bond_hook(skb);
> + vlan_on_bond_hook(skb, orig_dev);
>
> /* deliver only exact match when indicated */
> null_or_dev = deliver_exact ? skb->dev : NULL;
This patch doesn't work.
My setup has bond0.100 -> bond0 -> eth2 and eth3. ARP monitoring is
enabled as is arp_valiate.
The initial problem was just that just before vlan_on_bond_hook is
called, skb->dev = bond0.100 and orig_dev = eth2. (This is after
running goto another_route and having been called back through
__netif_receive_skb since vlan_hwaccel_do_receive it true.)
Now vlan_on_bond_hook is called and we have 2 skbs.
The original skb still have skb->dev = bond0.100 and orig_dev = eth2.
Since bond_arp_rcv is registered for traffic only to bond0, the handler
is not hit and the frame is dropped (or processed by another handler).
The cloned skb has skb->dev = bond0 and is put back on the receive queue
and comes back through __netif_receive_skb. This frame will match the
ptype entry for bond_arp_rcv, but since orig_dev = bond0 in this case,
the code in bond_arp_rcv will not handle the frame.
If we truly want to track the original interface that received the
frame, the following is a better option. With the recursive nature of
__netif_receive_skb at this point, we should really consider setting
orig_dev from skb_iif rather than just from skb->dev.
diff --git a/net/core/dev.c b/net/core/dev.c
index 30440e7..500fdbc 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3135,7 +3135,6 @@ static int __netif_receive_skb(struct sk_buff *skb)
if (!skb->skb_iif)
skb->skb_iif = skb->dev->ifindex;
- orig_dev = skb->dev;
skb_reset_network_header(skb);
skb_reset_transport_header(skb);
@@ -3145,6 +3144,7 @@ static int __netif_receive_skb(struct sk_buff *skb)
rcu_read_lock();
+ orig_dev = dev_get_by_index_rcu(dev_net(skb->dev),skb->skb_iif);
another_round:
__this_cpu_inc(softnet_data.processed);
^ permalink raw reply related
* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: David Miller @ 2011-03-07 22:09 UTC (permalink / raw)
To: eli; +Cc: roland, netdev
In-Reply-To: <20110307215803.GB19540@mtldesk30>
From: Eli Cohen <eli@dev.mellanox.co.il>
Date: Mon, 7 Mar 2011 23:58:03 +0200
> On Mon, Mar 07, 2011 at 01:49:31PM -0800, David Miller wrote:
>>
>> It's a performance optimization, if you don't get write combining you'll
>> get more strict ordering, rather than less.
>>
>> It cannot cause problem.
>
> I agree, but the function could still fail and the caller's logic
> could attempt to call ioreamp or take other action. For example, in
> the case of blue flame, it is better performance-wise to avoid using
> this feature if write combining is not available.
It could, but the less complicated the interfaces the better.
^ permalink raw reply
* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: Eli Cohen @ 2011-03-07 21:58 UTC (permalink / raw)
To: David Miller; +Cc: roland, netdev
In-Reply-To: <20110307.134931.232736253.davem@davemloft.net>
On Mon, Mar 07, 2011 at 01:49:31PM -0800, David Miller wrote:
>
> It's a performance optimization, if you don't get write combining you'll
> get more strict ordering, rather than less.
>
> It cannot cause problem.
I agree, but the function could still fail and the caller's logic
could attempt to call ioreamp or take other action. For example, in
the case of blue flame, it is better performance-wise to avoid using
this feature if write combining is not available.
^ permalink raw reply
* Re: [PATCH] bridge: control carrier based on ports online
From: Nicolas de Pesloüan @ 2011-03-07 21:51 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David S. Miller, Adam Majer, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, bridge, netdev, Andy Gospodarek, Jay Vosburgh
In-Reply-To: <20110307134450.652aea32@nehalam>
Le 07/03/2011 22:44, Stephen Hemminger a écrit :
> On Mon, 07 Mar 2011 21:48:16 +0100
> Nicolas de Pesloüan<nicolas.2p.debian@gmail.com> wrote:
>
>> Le 07/03/2011 19:34, Stephen Hemminger a écrit :
[snip]
>>> list_for_each_entry(p,&br->port_list, list) {
>>> - if (p->state != BR_STATE_DISABLED) {
>>> - if (p->port_no == br->root_port) {
>>> - p->config_pending = 0;
>>> - p->topology_change_ack = 0;
>>> - br_make_forwarding(p);
>>> - } else if (br_is_designated_port(p)) {
>>> - del_timer(&p->message_age_timer);
>>> - br_make_forwarding(p);
>>> - } else {
>>> - p->config_pending = 0;
>>> - p->topology_change_ack = 0;
>>> - br_make_blocking(p);
>>> - }
>>> + if (p->state == BR_STATE_DISABLED)
>>> + continue;
>>> +
>>> + if (p->port_no == br->root_port) {
>>> + p->config_pending = 0;
>>> + p->topology_change_ack = 0;
>>> + br_make_forwarding(p);
>>> + } else if (br_is_designated_port(p)) {
>>> + del_timer(&p->message_age_timer);
>>> + br_make_forwarding(p);
>>> + } else {
>>> + p->config_pending = 0;
>>> + p->topology_change_ack = 0;
>>> + br_make_blocking(p);
>>
>> Is the above part really related to the purpose of this patch? It looks like (good) cleanup, but
>> should be in a different patch.
>>
>> Except from this comment,
>>
>> Reviewed-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>
> The loop is going over the state of ports.
> Since the new code at the end of loop has to check for STATE_FORWARDING
> it is clearer with continue statement. When adding code it is always
> better to clarify the logic in the process rather than making it
> more complex.
Sound's good to me. Thanks for clarifying.
Nicolas.
^ permalink raw reply
* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: David Miller @ 2011-03-07 21:50 UTC (permalink / raw)
To: eli; +Cc: roland, netdev
In-Reply-To: <20110307.134931.232736253.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Mon, 07 Mar 2011 13:49:31 -0800 (PST)
> From: Eli Cohen <eli@dev.mellanox.co.il>
> Date: Mon, 7 Mar 2011 23:48:12 +0200
>
>> On Mon, Mar 07, 2011 at 01:40:01PM -0800, David Miller wrote:
>>> From: Eli Cohen <eli@dev.mellanox.co.il>
>>> Date: Mon, 7 Mar 2011 23:36:48 +0200
>>>
>>> > Specifically, some archs may not support write combining.
>>>
>>> They should just create a non-write-combining mapping if they
>>> don't support it.
>>>
>>
>> I wouldn't expect that since the caller function could be mislead to
>> believe it has a write combining capable area.
>
> It's a performance optimization, if you don't get write combining you'll
> get more strict ordering, rather than less.
>
> It cannot cause problem.
BTW, if we did as you suggest, fail if we don't support write combining,
then half the drivers in the tree would fail to probe on sparc64.
Every other driver expects it to succeed, with either write-combining
or more strict ordering semantics. Never to fail simply because
write-combining isn't supported.
It's a request, not a requirement.
^ permalink raw reply
* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: David Miller @ 2011-03-07 21:49 UTC (permalink / raw)
To: eli; +Cc: roland, netdev
In-Reply-To: <20110307214812.GA19540@mtldesk30>
From: Eli Cohen <eli@dev.mellanox.co.il>
Date: Mon, 7 Mar 2011 23:48:12 +0200
> On Mon, Mar 07, 2011 at 01:40:01PM -0800, David Miller wrote:
>> From: Eli Cohen <eli@dev.mellanox.co.il>
>> Date: Mon, 7 Mar 2011 23:36:48 +0200
>>
>> > Specifically, some archs may not support write combining.
>>
>> They should just create a non-write-combining mapping if they
>> don't support it.
>>
>
> I wouldn't expect that since the caller function could be mislead to
> believe it has a write combining capable area.
It's a performance optimization, if you don't get write combining you'll
get more strict ordering, rather than less.
It cannot cause problem.
^ permalink raw reply
* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: Eli Cohen @ 2011-03-07 21:48 UTC (permalink / raw)
To: David Miller; +Cc: roland, netdev
In-Reply-To: <20110307.134001.245386278.davem@davemloft.net>
On Mon, Mar 07, 2011 at 01:40:01PM -0800, David Miller wrote:
> From: Eli Cohen <eli@dev.mellanox.co.il>
> Date: Mon, 7 Mar 2011 23:36:48 +0200
>
> > Specifically, some archs may not support write combining.
>
> They should just create a non-write-combining mapping if they
> don't support it.
>
I wouldn't expect that since the caller function could be mislead to
believe it has a write combining capable area.
^ permalink raw reply
* Re: [PATCH] bridge: control carrier based on ports online
From: Stephen Hemminger @ 2011-03-07 21:44 UTC (permalink / raw)
To: Nicolas de Pesloüan
Cc: David S. Miller, Adam Majer, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, bridge, netdev, Andy Gospodarek, Jay Vosburgh
In-Reply-To: <4D754490.4000105@gmail.com>
On Mon, 07 Mar 2011 21:48:16 +0100
Nicolas de Pesloüan <nicolas.2p.debian@gmail.com> wrote:
> Le 07/03/2011 19:34, Stephen Hemminger a écrit :
> > This makes the bridge device behave like a physical device.
> > In earlier releases the bridge always asserted carrier. This
> > changes the behavior so that bridge device carrier is on only
> > if one or more ports are in the forwarding state. This
> > should help IPv6 autoconfiguration, DHCP, and routing daemons.
> >
> > I did brief testing with Network and Virt manager and they
> > seem fine, but since this changes behavior of bridge, it should
> > wait until net-next (2.6.39).
> >
> > Signed-off-by: Stephen Hemminger<shemminger@vyatta.com>
> >
> > ---
> > net/bridge/br_device.c | 4 ++++
> > net/bridge/br_stp.c | 35 ++++++++++++++++++++++-------------
> > net/bridge/br_stp_timer.c | 1 +
> > 3 files changed, 27 insertions(+), 13 deletions(-)
> >
> > --- a/net/bridge/br_device.c 2011-03-07 08:40:08.913599513 -0800
> > +++ b/net/bridge/br_device.c 2011-03-07 08:40:48.382377389 -0800
> > @@ -78,6 +78,8 @@ static int br_dev_open(struct net_device
> > {
> > struct net_bridge *br = netdev_priv(dev);
> >
> > + netif_carrier_off(dev);
> > +
> > br_features_recompute(br);
> > netif_start_queue(dev);
> > br_stp_enable_bridge(br);
> > @@ -94,6 +96,8 @@ static int br_dev_stop(struct net_device
> > {
> > struct net_bridge *br = netdev_priv(dev);
> >
> > + netif_carrier_off(dev);
> > +
> > br_stp_disable_bridge(br);
> > br_multicast_stop(br);
> >
> > --- a/net/bridge/br_stp.c 2011-03-07 08:41:58.619783678 -0800
> > +++ b/net/bridge/br_stp.c 2011-03-07 08:53:58.953558810 -0800
> > @@ -397,28 +397,37 @@ static void br_make_forwarding(struct ne
> > void br_port_state_selection(struct net_bridge *br)
> > {
> > struct net_bridge_port *p;
> > + unsigned int liveports = 0;
> >
> > /* Don't change port states if userspace is handling STP */
> > if (br->stp_enabled == BR_USER_STP)
> > return;
> >
> > list_for_each_entry(p,&br->port_list, list) {
> > - if (p->state != BR_STATE_DISABLED) {
> > - if (p->port_no == br->root_port) {
> > - p->config_pending = 0;
> > - p->topology_change_ack = 0;
> > - br_make_forwarding(p);
> > - } else if (br_is_designated_port(p)) {
> > - del_timer(&p->message_age_timer);
> > - br_make_forwarding(p);
> > - } else {
> > - p->config_pending = 0;
> > - p->topology_change_ack = 0;
> > - br_make_blocking(p);
> > - }
> > + if (p->state == BR_STATE_DISABLED)
> > + continue;
> > +
> > + if (p->port_no == br->root_port) {
> > + p->config_pending = 0;
> > + p->topology_change_ack = 0;
> > + br_make_forwarding(p);
> > + } else if (br_is_designated_port(p)) {
> > + del_timer(&p->message_age_timer);
> > + br_make_forwarding(p);
> > + } else {
> > + p->config_pending = 0;
> > + p->topology_change_ack = 0;
> > + br_make_blocking(p);
>
> Is the above part really related to the purpose of this patch? It looks like (good) cleanup, but
> should be in a different patch.
>
> Except from this comment,
>
> Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
The loop is going over the state of ports.
Since the new code at the end of loop has to check for STATE_FORWARDING
it is clearer with continue statement. When adding code it is always
better to clarify the logic in the process rather than making it
more complex.
--
^ 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