* Re: is non-inheritance of congestion control algorithm from the listen socket a bug or a feature?
From: Stephen Hemminger @ 2011-11-29 22:01 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, ycheng, rick.jones2, netdev
In-Reply-To: <1322603786.2596.36.camel@edumazet-laptop>
On Tue, 29 Nov 2011 22:56:26 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 29 novembre 2011 à 16:52 -0500, David Miller a écrit :
>
> > There is really no reason to keep the current behavior.
> >
> > If an application sets the congestion control algorithm on a listening
> > socket to a non-default value, what effect could possibly be intended?
> >
> > Congestion control doesn't even come into play at all on a listening
> > socket, therefore the only logical expectation is that it inherits to
> > the child.
> >
> > The only other logical behavior would be to forbid this operation on a
> > listening socket, since it has no effect, but that doesn't make any
> > sense now does it? :-)
>
> Moreover, an application can use setsockopt(TCP_CONGESTION) before
> calling listen() (while socket is still in CLOSE state)
Agreed, it was just an oversight of the initial design.
The setsockopt() on the listening socket is ignored.
^ permalink raw reply
* Re: [PATCH v2] netlabel: Fix build problems when IPv6 is not enabled
From: Randy Dunlap @ 2011-11-29 22:00 UTC (permalink / raw)
To: Paul Moore; +Cc: netdev, linux-next, linux-kernel
In-Reply-To: <20111129201054.20141.86401.stgit@sifl>
On 11/29/2011 12:10 PM, Paul Moore wrote:
> A recent fix to the the NetLabel code caused build problem with
> configurations that did not have IPv6 enabled; see below:
>
> netlabel_kapi.c: In function 'netlbl_cfg_unlbl_map_add':
> netlabel_kapi.c:165:4:
> error: implicit declaration of function 'netlbl_af6list_add'
>
> This patch fixes this problem by making the IPv6 specific code conditional
> on the IPv6 configuration flags as we done in the rest of NetLabel and the
> network stack as a whole. We have to move some variable declarations
> around as a result so things may not be quite as pretty, but at least it
> builds cleanly now.
>
> Some additional IPv6 conditionals were added to the NetLabel code as well
> for the sake of consistency.
>
> Reported-by: Randy Dunlap <rdunlap@xenotime.net>
> Signed-off-by: Paul Moore <pmoore@redhat.com>
Acked-by: Randy Dunlap <rdunlap@xenotime.net>
Thanks.
> ---
> net/netlabel/netlabel_kapi.c | 22 ++++++++++++++--------
> 1 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
> index 3735297..5952237 100644
> --- a/net/netlabel/netlabel_kapi.c
> +++ b/net/netlabel/netlabel_kapi.c
> @@ -111,8 +111,6 @@ int netlbl_cfg_unlbl_map_add(const char *domain,
> struct netlbl_domaddr_map *addrmap = NULL;
> struct netlbl_domaddr4_map *map4 = NULL;
> struct netlbl_domaddr6_map *map6 = NULL;
> - const struct in_addr *addr4, *mask4;
> - const struct in6_addr *addr6, *mask6;
>
> entry = kzalloc(sizeof(*entry), GFP_ATOMIC);
> if (entry == NULL)
> @@ -133,9 +131,9 @@ int netlbl_cfg_unlbl_map_add(const char *domain,
> INIT_LIST_HEAD(&addrmap->list6);
>
> switch (family) {
> - case AF_INET:
> - addr4 = addr;
> - mask4 = mask;
> + case AF_INET: {
> + const struct in_addr *addr4 = addr;
> + const struct in_addr *mask4 = mask;
> map4 = kzalloc(sizeof(*map4), GFP_ATOMIC);
> if (map4 == NULL)
> goto cfg_unlbl_map_add_failure;
> @@ -148,9 +146,11 @@ int netlbl_cfg_unlbl_map_add(const char *domain,
> if (ret_val != 0)
> goto cfg_unlbl_map_add_failure;
> break;
> - case AF_INET6:
> - addr6 = addr;
> - mask6 = mask;
> + }
> +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> + case AF_INET6: {
> + const struct in6_addr *addr6 = addr;
> + const struct in6_addr *mask6 = mask;
> map6 = kzalloc(sizeof(*map6), GFP_ATOMIC);
> if (map6 == NULL)
> goto cfg_unlbl_map_add_failure;
> @@ -167,6 +167,8 @@ int netlbl_cfg_unlbl_map_add(const char *domain,
> if (ret_val != 0)
> goto cfg_unlbl_map_add_failure;
> break;
> + }
> +#endif /* IPv6 */
> default:
> goto cfg_unlbl_map_add_failure;
> break;
> @@ -225,9 +227,11 @@ int netlbl_cfg_unlbl_static_add(struct net *net,
> case AF_INET:
> addr_len = sizeof(struct in_addr);
> break;
> +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> case AF_INET6:
> addr_len = sizeof(struct in6_addr);
> break;
> +#endif /* IPv6 */
> default:
> return -EPFNOSUPPORT;
> }
> @@ -266,9 +270,11 @@ int netlbl_cfg_unlbl_static_del(struct net *net,
> case AF_INET:
> addr_len = sizeof(struct in_addr);
> break;
> +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> case AF_INET6:
> addr_len = sizeof(struct in6_addr);
> break;
> +#endif /* IPv6 */
> default:
> return -EPFNOSUPPORT;
> }
>
--
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply
* Re: is non-inheritance of congestion control algorithm from the listen socket a bug or a feature?
From: Eric Dumazet @ 2011-11-29 21:56 UTC (permalink / raw)
To: David Miller; +Cc: ycheng, rick.jones2, netdev
In-Reply-To: <20111129.165205.91103035999089185.davem@davemloft.net>
Le mardi 29 novembre 2011 à 16:52 -0500, David Miller a écrit :
> There is really no reason to keep the current behavior.
>
> If an application sets the congestion control algorithm on a listening
> socket to a non-default value, what effect could possibly be intended?
>
> Congestion control doesn't even come into play at all on a listening
> socket, therefore the only logical expectation is that it inherits to
> the child.
>
> The only other logical behavior would be to forbid this operation on a
> listening socket, since it has no effect, but that doesn't make any
> sense now does it? :-)
Moreover, an application can use setsockopt(TCP_CONGESTION) before
calling listen() (while socket is still in CLOSE state)
^ permalink raw reply
* [PATCH net-next] bnx2: Support for byte queue limits
From: Eric Dumazet @ 2011-11-29 21:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Tom Herbert
Changes to bnx2 to use byte queue limits.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Tom Herbert <therbert@google.com>
---
drivers/net/ethernet/broadcom/bnx2.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
index d573169..787e175 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -2810,6 +2810,7 @@ bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
struct bnx2_tx_ring_info *txr = &bnapi->tx_ring;
u16 hw_cons, sw_cons, sw_ring_cons;
int tx_pkt = 0, index;
+ unsigned int tx_bytes = 0;
struct netdev_queue *txq;
index = (bnapi - bp->bnx2_napi);
@@ -2864,6 +2865,7 @@ bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
sw_cons = NEXT_TX_BD(sw_cons);
+ tx_bytes += skb->len;
dev_kfree_skb(skb);
tx_pkt++;
if (tx_pkt == budget)
@@ -2873,6 +2875,7 @@ bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
hw_cons = bnx2_get_hw_tx_cons(bnapi);
}
+ netdev_tx_completed_queue(txq, tx_pkt, tx_bytes);
txr->hw_tx_cons = hw_cons;
txr->tx_cons = sw_cons;
@@ -5393,6 +5396,7 @@ bnx2_free_tx_skbs(struct bnx2 *bp)
}
dev_kfree_skb(skb);
}
+ netdev_tx_reset_queue(netdev_get_tx_queue(bp->dev, i));
}
}
@@ -6546,6 +6550,8 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
}
txbd->tx_bd_vlan_tag_flags |= TX_BD_FLAGS_END;
+ netdev_tx_sent_queue(txq, skb->len);
+
prod = NEXT_TX_BD(prod);
txr->tx_prod_bseq += skb->len;
^ permalink raw reply related
* Re: is non-inheritance of congestion control algorithm from the listen socket a bug or a feature?
From: David Miller @ 2011-11-29 21:52 UTC (permalink / raw)
To: eric.dumazet; +Cc: ycheng, rick.jones2, netdev
In-Reply-To: <1322603175.2596.31.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 29 Nov 2011 22:46:15 +0100
> Le mardi 29 novembre 2011 à 13:20 -0800, Yuchung Cheng a écrit :
>> I actually think it's a feature :)
>>
>> I find it awkward to set CC on listening socket. And current document
>> defines the sysctl well
>>
>> tcp_congestion_control - STRING
>> Set the congestion control algorithm to be used for new
>> connections. The algorithm "reno" is always available, but
>> additional choices may be available based on kernel configuration.
>> Default is set as part of kernel configuration.
>
> This might be a feature, but contradicts most socket options set on
> listener and inherited by a child socket on accept()
There is really no reason to keep the current behavior.
If an application sets the congestion control algorithm on a listening
socket to a non-default value, what effect could possibly be intended?
Congestion control doesn't even come into play at all on a listening
socket, therefore the only logical expectation is that it inherits to
the child.
The only other logical behavior would be to forbid this operation on a
listening socket, since it has no effect, but that doesn't make any
sense now does it? :-)
^ permalink raw reply
* Re: [PATCH v2] netlabel: Fix build problems when IPv6 is not enabled
From: David Miller @ 2011-11-29 21:49 UTC (permalink / raw)
To: rdunlap; +Cc: pmoore, netdev, linux-next, linux-kernel
In-Reply-To: <4ED555E4.4000006@xenotime.net>
From: Randy Dunlap <rdunlap@xenotime.net>
Date: Tue, 29 Nov 2011 14:00:04 -0800
> On 11/29/2011 12:10 PM, Paul Moore wrote:
>> A recent fix to the the NetLabel code caused build problem with
>> configurations that did not have IPv6 enabled; see below:
>>
>> netlabel_kapi.c: In function 'netlbl_cfg_unlbl_map_add':
>> netlabel_kapi.c:165:4:
>> error: implicit declaration of function 'netlbl_af6list_add'
>>
>> This patch fixes this problem by making the IPv6 specific code conditional
>> on the IPv6 configuration flags as we done in the rest of NetLabel and the
>> network stack as a whole. We have to move some variable declarations
>> around as a result so things may not be quite as pretty, but at least it
>> builds cleanly now.
>>
>> Some additional IPv6 conditionals were added to the NetLabel code as well
>> for the sake of consistency.
>>
>> Reported-by: Randy Dunlap <rdunlap@xenotime.net>
>> Signed-off-by: Paul Moore <pmoore@redhat.com>
>
> Acked-by: Randy Dunlap <rdunlap@xenotime.net>
Applied, thanks everyone.
^ permalink raw reply
* Re: is non-inheritance of congestion control algorithm from the listen socket a bug or a feature?
From: Eric Dumazet @ 2011-11-29 21:46 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: Rick Jones, netdev
In-Reply-To: <CAK6E8=djWDn=ZvD0XLxex3vOfsDjCT64k+DwDSNp6+tLbPen2A@mail.gmail.com>
Le mardi 29 novembre 2011 à 13:20 -0800, Yuchung Cheng a écrit :
> I actually think it's a feature :)
>
> I find it awkward to set CC on listening socket. And current document
> defines the sysctl well
>
> tcp_congestion_control - STRING
> Set the congestion control algorithm to be used for new
> connections. The algorithm "reno" is always available, but
> additional choices may be available based on kernel configuration.
> Default is set as part of kernel configuration.
This might be a feature, but contradicts most socket options set on
listener and inherited by a child socket on accept()
tcp_congestion_control should be system wide default.
Anyway, "man 7 tcp" doesnt document TCP_CONGESTION, so we are not
supposed to play with it ;)
Oh well...
^ permalink raw reply
* Re: [RFC PATCH 00/18] netfilter: IPv6 NAT
From: Krzysztof Olędzki @ 2011-11-29 21:38 UTC (permalink / raw)
To: Jan Engelhardt
Cc: Ulrich Weber, Amos Jeffries, sclark46@earthlink.net,
kaber@trash.net, netfilter-devel@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <alpine.LNX.2.01.1111291255200.20965@frira.zrqbmnf.qr>
On 2011-11-29 13:23, Jan Engelhardt wrote:
>
> On Tuesday 2011-11-29 10:19, Ulrich Weber wrote:
>> On 28.11.2011 23:03, Amos Jeffries wrote:
>>> I'm going to dare to call FUD on those statements...
>>> * Load Balancing - what is preventing your routing rules or packet
>>> marking using the same criteria as the NAT changer? nothing. Load
>>> balancing works perfectly fine without NAT.
>
> Source address selection, having to occur on the source, would
> require that the source has to know all the parameters that a {what
> would have been your NAT GW} would need to know, which means you have
> to (a) collect and/or (b) distribute this information. Given two
> uplinks that only allow a certain source network address (different
> for each uplink), combined with the desire to balance on utilization,
> (a) a client is not in the position to easily obtain this data unless
> it is the router for all participants itself, (b) the clients needs
> to cooperate, and one cannot always trust client devices, or hope for
> their technical cooperation (firewalled themselves off).
>
> Yes, NAT is evil, but if you actually think about it, policies are
> best applied where [the policy] originates from. After all, we also
> don't do LSRR, instead, routers do the routing, because they just
> know much better.
>
>> I fully agree. NAT can not replace your firewall rules.
>>
>> However with NAT you could get some kind of anonymity.
>
> Same network prefix, some cookies, or a login form. Blam, identified,
> or at least (Almost-)Uniquely Identified Visitor tagging.
But without NAT you have pretty big chance to have the same IPv6
*suffix* everywhere, based on you MAC address. In your Home, your Work,
in a Cafe or in a hotel during your vacations in Portugal. So yes, NAT
is not a perfect solution but it really helps you privacy.
Best regards,
Krzysztof Olędzki
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: RCU'ed dst_get_neighbour()
From: Roland Dreier @ 2011-11-29 21:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Marc Aurele La France, David Miller, netdev, linux-rdma
In-Reply-To: <1322602283.2596.25.camel@edumazet-laptop>
On Tue, Nov 29, 2011 at 1:31 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Here is the result of this audit, please double check and test it, I
> only compiled this.
Thanks Eric... I'll queue this up and send it on once we get a good
report from Marc.
Thanks!
Roland
^ permalink raw reply
* Re: RCU'ed dst_get_neighbour()
From: Eric Dumazet @ 2011-11-29 21:31 UTC (permalink / raw)
To: Marc Aurele La France
Cc: Roland Dreier, David Miller, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1322601444.2596.21.camel@edumazet-laptop>
Le mardi 29 novembre 2011 à 22:17 +0100, Eric Dumazet a écrit :
> Le mardi 29 novembre 2011 à 14:00 -0700, Marc Aurele La France a écrit :
> > On Tue, 29 Nov 2011, Eric Dumazet wrote:
> >
> > > Le mardi 29 novembre 2011 à 12:47 -0800, Roland Dreier a écrit :
> > >> Thanks Eric, I'll send this to Linus shortly.
> >
> > > Oh well, I forgot one rcu_read_unlock(), I'll send a V2...
> >
> > This also doesn't address the other dst_get_neighbour() instances
> > introduced by
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=69cce1d1404968f78b177a0314f5822d5afdbbfb
> >
>
> Oh well, a complete audit is needed, and I have no choice but doing it.
>
> Thanks !
>
Here is the result of this audit, please double check and test it, I
only compiled this.
Thanks !
[PATCH V2] drivers/infiniband: fix lockdep splats
commit f2c31e32b37 (net: fix NULL dereferences in check_peer_redir())
forgot to take care of infiniband uses of dst neighbours.
Many thanks to Marc Aurele who provided a nice bug report and feedback.
Reported-by: Marc Aurele La France <tsi-yfeSBMgouQgsA/PxXw9srA@public.gmane.org>
Signed-off-by: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
CC: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
CC: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/core/addr.c | 9 +++++--
drivers/infiniband/hw/cxgb3/iwch_cm.c | 4 +++
drivers/infiniband/hw/cxgb4/cm.c | 6 +++++
drivers/infiniband/hw/nes/nes_cm.c | 6 +++--
drivers/infiniband/ulp/ipoib/ipoib_main.c | 18 +++++++++------
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 6 +++--
6 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 691276b..e9cf51b 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -216,7 +216,9 @@ static int addr4_resolve(struct sockaddr_in *src_in,
neigh = neigh_lookup(&arp_tbl, &rt->rt_gateway, rt->dst.dev);
if (!neigh || !(neigh->nud_state & NUD_VALID)) {
+ rcu_read_lock();
neigh_event_send(dst_get_neighbour(&rt->dst), NULL);
+ rcu_read_unlock();
ret = -ENODATA;
if (neigh)
goto release;
@@ -274,15 +276,16 @@ static int addr6_resolve(struct sockaddr_in6 *src_in,
goto put;
}
+ rcu_read_lock();
neigh = dst_get_neighbour(dst);
if (!neigh || !(neigh->nud_state & NUD_VALID)) {
if (neigh)
neigh_event_send(neigh, NULL);
ret = -ENODATA;
- goto put;
+ } else {
+ ret = rdma_copy_addr(addr, dst->dev, neigh->ha);
}
-
- ret = rdma_copy_addr(addr, dst->dev, neigh->ha);
+ rcu_read_unlock();
put:
dst_release(dst);
return ret;
diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c b/drivers/infiniband/hw/cxgb3/iwch_cm.c
index de6d077..c88b12b 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_cm.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c
@@ -1375,8 +1375,10 @@ static int pass_accept_req(struct t3cdev *tdev, struct sk_buff *skb, void *ctx)
goto reject;
}
dst = &rt->dst;
+ rcu_read_lock();
neigh = dst_get_neighbour(dst);
l2t = t3_l2t_get(tdev, neigh, neigh->dev);
+ rcu_read_unlock();
if (!l2t) {
printk(KERN_ERR MOD "%s - failed to allocate l2t entry!\n",
__func__);
@@ -1946,10 +1948,12 @@ int iwch_connect(struct iw_cm_id *cm_id, struct iw_cm_conn_param *conn_param)
}
ep->dst = &rt->dst;
+ rcu_read_lock();
neigh = dst_get_neighbour(ep->dst);
/* get a l2t entry */
ep->l2t = t3_l2t_get(ep->com.tdev, neigh, neigh->dev);
+ rcu_read_unlock();
if (!ep->l2t) {
printk(KERN_ERR MOD "%s - cannot alloc l2e.\n", __func__);
err = -ENOMEM;
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index b36cdac..75b57be 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -1594,6 +1594,7 @@ static int pass_accept_req(struct c4iw_dev *dev, struct sk_buff *skb)
goto reject;
}
dst = &rt->dst;
+ rcu_read_lock();
neigh = dst_get_neighbour(dst);
if (neigh->dev->flags & IFF_LOOPBACK) {
pdev = ip_dev_find(&init_net, peer_ip);
@@ -1620,6 +1621,7 @@ static int pass_accept_req(struct c4iw_dev *dev, struct sk_buff *skb)
rss_qid = dev->rdev.lldi.rxq_ids[
cxgb4_port_idx(neigh->dev) * step];
}
+ rcu_read_unlock();
if (!l2t) {
printk(KERN_ERR MOD "%s - failed to allocate l2t entry!\n",
__func__);
@@ -1820,6 +1822,7 @@ static int c4iw_reconnect(struct c4iw_ep *ep)
}
ep->dst = &rt->dst;
+ rcu_read_lock();
neigh = dst_get_neighbour(ep->dst);
/* get a l2t entry */
@@ -1856,6 +1859,7 @@ static int c4iw_reconnect(struct c4iw_ep *ep)
ep->rss_qid = ep->com.dev->rdev.lldi.rxq_ids[
cxgb4_port_idx(neigh->dev) * step];
}
+ rcu_read_unlock();
if (!ep->l2t) {
printk(KERN_ERR MOD "%s - cannot alloc l2e.\n", __func__);
err = -ENOMEM;
@@ -2301,6 +2305,7 @@ int c4iw_connect(struct iw_cm_id *cm_id, struct iw_cm_conn_param *conn_param)
}
ep->dst = &rt->dst;
+ rcu_read_lock();
neigh = dst_get_neighbour(ep->dst);
/* get a l2t entry */
@@ -2339,6 +2344,7 @@ int c4iw_connect(struct iw_cm_id *cm_id, struct iw_cm_conn_param *conn_param)
ep->retry_with_mpa_v1 = 0;
ep->tried_with_mpa_v1 = 0;
}
+ rcu_read_unlock();
if (!ep->l2t) {
printk(KERN_ERR MOD "%s - cannot alloc l2e.\n", __func__);
err = -ENOMEM;
diff --git a/drivers/infiniband/hw/nes/nes_cm.c b/drivers/infiniband/hw/nes/nes_cm.c
index dfce9ea..0a52d72 100644
--- a/drivers/infiniband/hw/nes/nes_cm.c
+++ b/drivers/infiniband/hw/nes/nes_cm.c
@@ -1377,9 +1377,11 @@ static int nes_addr_resolve_neigh(struct nes_vnic *nesvnic, u32 dst_ip, int arpi
neigh_release(neigh);
}
- if ((neigh == NULL) || (!(neigh->nud_state & NUD_VALID)))
+ if ((neigh == NULL) || (!(neigh->nud_state & NUD_VALID))) {
+ rcu_read_lock();
neigh_event_send(dst_get_neighbour(&rt->dst), NULL);
-
+ rcu_read_unlock();
+ }
ip_rt_put(rt);
return rc;
}
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 7567b60..ef38848 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -555,6 +555,7 @@ static int path_rec_start(struct net_device *dev,
return 0;
}
+/* called with rcu_read_lock */
static void neigh_add_path(struct sk_buff *skb, struct net_device *dev)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
@@ -636,6 +637,7 @@ err_drop:
spin_unlock_irqrestore(&priv->lock, flags);
}
+/* called with rcu_read_lock */
static void ipoib_path_lookup(struct sk_buff *skb, struct net_device *dev)
{
struct ipoib_dev_priv *priv = netdev_priv(skb->dev);
@@ -720,13 +722,14 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
struct neighbour *n = NULL;
unsigned long flags;
+ rcu_read_lock();
if (likely(skb_dst(skb)))
n = dst_get_neighbour(skb_dst(skb));
if (likely(n)) {
if (unlikely(!*to_ipoib_neigh(n))) {
ipoib_path_lookup(skb, dev);
- return NETDEV_TX_OK;
+ goto unlock;
}
neigh = *to_ipoib_neigh(n);
@@ -749,17 +752,17 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
ipoib_neigh_free(dev, neigh);
spin_unlock_irqrestore(&priv->lock, flags);
ipoib_path_lookup(skb, dev);
- return NETDEV_TX_OK;
+ goto unlock;
}
if (ipoib_cm_get(neigh)) {
if (ipoib_cm_up(neigh)) {
ipoib_cm_send(dev, skb, ipoib_cm_get(neigh));
- return NETDEV_TX_OK;
+ goto unlock;
}
} else if (neigh->ah) {
ipoib_send(dev, skb, neigh->ah, IPOIB_QPN(n->ha));
- return NETDEV_TX_OK;
+ goto unlock;
}
if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
@@ -793,13 +796,14 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
phdr->hwaddr + 4);
dev_kfree_skb_any(skb);
++dev->stats.tx_dropped;
- return NETDEV_TX_OK;
+ goto unlock;
}
unicast_arp_send(skb, dev, phdr);
}
}
-
+unlock:
+ rcu_read_unlock();
return NETDEV_TX_OK;
}
@@ -837,7 +841,7 @@ static int ipoib_hard_header(struct sk_buff *skb,
dst = skb_dst(skb);
n = NULL;
if (dst)
- n = dst_get_neighbour(dst);
+ n = dst_get_neighbour_raw(dst);
if ((!dst || !n) && daddr) {
struct ipoib_pseudoheader *phdr =
(struct ipoib_pseudoheader *) skb_push(skb, sizeof *phdr);
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 1b7a976..cad1894 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -266,7 +266,7 @@ static int ipoib_mcast_join_finish(struct ipoib_mcast *mcast,
skb->dev = dev;
if (dst)
- n = dst_get_neighbour(dst);
+ n = dst_get_neighbour_raw(dst);
if (!dst || !n) {
/* put pseudoheader back on for next time */
skb_push(skb, sizeof (struct ipoib_pseudoheader));
@@ -722,6 +722,8 @@ out:
if (mcast && mcast->ah) {
struct dst_entry *dst = skb_dst(skb);
struct neighbour *n = NULL;
+
+ rcu_read_lock();
if (dst)
n = dst_get_neighbour(dst);
if (n && !*to_ipoib_neigh(n)) {
@@ -734,7 +736,7 @@ out:
list_add_tail(&neigh->list, &mcast->neigh_list);
}
}
-
+ rcu_read_unlock();
spin_unlock_irqrestore(&priv->lock, flags);
ipoib_send(dev, skb, mcast->ah, IB_MULTICAST_QPN);
return;
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 related
* Re: is non-inheritance of congestion control algorithm from the listen socket a bug or a feature?
From: Yuchung Cheng @ 2011-11-29 21:20 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Rick Jones, netdev
In-Reply-To: <1322600212.2596.13.camel@edumazet-laptop>
On Tue, Nov 29, 2011 at 12:56 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 29 novembre 2011 à 12:03 -0800, Rick Jones a écrit :
>
>> Is the non-inheritance of the congestion control algorithm a bug or a
>> feature?
>
> IMHO its a bug.
>
> Can you provide a fix ?
>
> Thanks !
I actually think it's a feature :)
I find it awkward to set CC on listening socket. And current document
defines the sysctl well
tcp_congestion_control - STRING
Set the congestion control algorithm to be used for new
connections. The algorithm "reno" is always available, but
additional choices may be available based on kernel configuration.
Default is set as part of kernel configuration.
^ permalink raw reply
* Re: RCU'ed dst_get_neighbour()
From: Eric Dumazet @ 2011-11-29 21:17 UTC (permalink / raw)
To: Marc Aurele La France
Cc: Roland Dreier, David Miller, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.WNT.2.00.1111291359290.1036@TSI>
Le mardi 29 novembre 2011 à 14:00 -0700, Marc Aurele La France a écrit :
> On Tue, 29 Nov 2011, Eric Dumazet wrote:
>
> > Le mardi 29 novembre 2011 à 12:47 -0800, Roland Dreier a écrit :
> >> Thanks Eric, I'll send this to Linus shortly.
>
> > Oh well, I forgot one rcu_read_unlock(), I'll send a V2...
>
> This also doesn't address the other dst_get_neighbour() instances
> introduced by
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=69cce1d1404968f78b177a0314f5822d5afdbbfb
>
Oh well, a complete audit is needed, and I have no choice but doing it.
Thanks !
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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: is non-inheritance of congestion control algorithm from the listen socket a bug or a feature?
From: Rick Jones @ 2011-11-29 21:17 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1322600212.2596.13.camel@edumazet-laptop>
On 11/29/2011 12:56 PM, Eric Dumazet wrote:
> Le mardi 29 novembre 2011 à 12:03 -0800, Rick Jones a écrit :
>
>> Is the non-inheritance of the congestion control algorithm a bug or a
>> feature?
>
> IMHO its a bug.
>
> Can you provide a fix ?
I can try once I find my way through the maze.
rick
^ permalink raw reply
* [PATCH] rt2x00: Use kcalloc instead of kzalloc to allocate array
From: Thomas Meyer @ 2011-11-29 21:08 UTC (permalink / raw)
To: IvDoorn, gwingerde, helmut.schaa, linville, linux-wireless, users,
netdev, linux-kernel
The advantage of kcalloc is, that will prevent integer overflows which could
result from the multiplication of number of elements and size and it is also
a bit nicer to read.
The semantic patch that makes this change is available
in https://lkml.org/lkml/2011/11/25/107
Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
---
diff -u -p a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c 2011-11-28 19:36:47.770108588 +0100
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c 2011-11-28 19:54:53.495525543 +0100
@@ -831,11 +831,11 @@ static int rt2x00lib_probe_hw_modes(stru
if (spec->supported_rates & SUPPORT_RATE_OFDM)
num_rates += 8;
- channels = kzalloc(sizeof(*channels) * spec->num_channels, GFP_KERNEL);
+ channels = kcalloc(spec->num_channels, sizeof(*channels), GFP_KERNEL);
if (!channels)
return -ENOMEM;
- rates = kzalloc(sizeof(*rates) * num_rates, GFP_KERNEL);
+ rates = kcalloc(num_rates, sizeof(*rates), GFP_KERNEL);
if (!rates)
goto exit_free_channels;
^ permalink raw reply
* [PATCH] enic: Use kcalloc instead of kzalloc to allocate array
From: Thomas Meyer @ 2011-11-29 21:08 UTC (permalink / raw)
To: netdev, linux-kernel
The advantage of kcalloc is, that will prevent integer overflows which could
result from the multiplication of number of elements and size and it is also
a bit nicer to read.
The semantic patch that makes this change is available
in https://lkml.org/lkml/2011/11/25/107
Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
---
diff -u -p a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
--- a/drivers/net/ethernet/cisco/enic/enic_main.c 2011-11-13 11:07:34.306945516 +0100
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c 2011-11-28 19:53:11.983614560 +0100
@@ -2379,7 +2379,7 @@ static int __devinit enic_probe(struct p
#endif
/* Allocate structure for port profiles */
- enic->pp = kzalloc(num_pps * sizeof(*enic->pp), GFP_KERNEL);
+ enic->pp = kcalloc(num_pps, sizeof(*enic->pp), GFP_KERNEL);
if (!enic->pp) {
pr_err("port profile alloc failed, aborting\n");
err = -ENOMEM;
^ permalink raw reply
* [PATCH] bnx2x: Use kcalloc instead of kzalloc to allocate array
From: Thomas Meyer @ 2011-11-29 21:08 UTC (permalink / raw)
To: netdev, linux-kernel
The advantage of kcalloc is, that will prevent integer overflows which could
result from the multiplication of number of elements and size and it is also
a bit nicer to read.
The semantic patch that makes this change is available
in https://lkml.org/lkml/2011/11/25/107
Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
---
diff -u -p a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c 2011-11-13 11:07:33.983607086 +0100
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c 2011-11-28 19:52:50.979887113 +0100
@@ -3278,14 +3278,14 @@ int __devinit bnx2x_alloc_mem_bp(struct
msix_table_size = bp->igu_sb_cnt + 1;
/* fp array: RSS plus CNIC related L2 queues */
- fp = kzalloc((BNX2X_MAX_RSS_COUNT(bp) + NON_ETH_CONTEXT_USE) *
+ fp = kcalloc(BNX2X_MAX_RSS_COUNT(bp) + NON_ETH_CONTEXT_USE,
sizeof(*fp), GFP_KERNEL);
if (!fp)
goto alloc_err;
bp->fp = fp;
/* msix table */
- tbl = kzalloc(msix_table_size * sizeof(*tbl), GFP_KERNEL);
+ tbl = kcalloc(msix_table_size, sizeof(*tbl), GFP_KERNEL);
if (!tbl)
goto alloc_err;
bp->msix_table = tbl;
diff -u -p a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c 2011-11-28 19:36:47.716773832 +0100
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c 2011-11-28 19:52:53.063259517 +0100
@@ -3342,7 +3342,7 @@ static inline int bnx2x_mcast_refresh_re
if (!list_empty(&o->registry.exact_match.macs))
return 0;
- elem = kzalloc(sizeof(*elem)*len, GFP_ATOMIC);
+ elem = kcalloc(len, sizeof(*elem), GFP_ATOMIC);
if (!elem) {
BNX2X_ERR("Failed to allocate registry memory\n");
return -ENOMEM;
^ permalink raw reply
* Re: RCU'ed dst_get_neighbour()
From: Marc Aurele La France @ 2011-11-29 21:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Roland Dreier, David Miller, netdev, linux-rdma
In-Reply-To: <1322599991.2596.11.camel@edumazet-laptop>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1149 bytes --]
On Tue, 29 Nov 2011, Eric Dumazet wrote:
> Le mardi 29 novembre 2011 à 12:47 -0800, Roland Dreier a écrit :
>> Thanks Eric, I'll send this to Linus shortly.
> Oh well, I forgot one rcu_read_unlock(), I'll send a V2...
This also doesn't address the other dst_get_neighbour() instances
introduced by
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=69cce1d1404968f78b177a0314f5822d5afdbbfb
Marc.
+----------------------------------+----------------------------------+
| Marc Aurele La France | work: 1-780-492-9310 |
| Academic Information and | fax: 1-780-492-1729 |
| Communications Technologies | email: tsi@ualberta.ca |
| 352 General Services Building +----------------------------------+
| University of Alberta | |
| Edmonton, Alberta | Standard disclaimers apply |
| T6G 2H1 | |
| CANADA | |
+----------------------------------+----------------------------------+
^ permalink raw reply
* Re: [PATCH v4 06/10] e1000e: Support for byte queue limits
From: Jesse Brandeburg @ 2011-11-29 21:01 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <alpine.DEB.2.00.1111281819350.24413@pokey.mtv.corp.google.com>
On Mon, 28 Nov 2011 18:33:16 -0800
Tom Herbert <therbert@google.com> wrote:
> Changes to e1000e to use byte queue limits.
First: thanks Tom for looking into e1000e with this work.
> @@ -1096,6 +1097,10 @@ static bool e1000_clean_tx_irq(struct e1000_adapter *adapter)
> if (cleaned) {
> total_tx_packets += buffer_info->segs;
> total_tx_bytes += buffer_info->bytecount;
> + if (buffer_info->skb) {
> + bytes_compl += buffer_info->skb->len;
whats wrong with using total_tx_bytes or buffer_info->bytecount? it
contains the "bytes on the wire" value which will be slightly larger
than skb->len, but avoids warming the skb->len cacheline unnecessarily.
the rest of the patch to e1000e looks okay.
^ permalink raw reply
* Re: is non-inheritance of congestion control algorithm from the listen socket a bug or a feature?
From: Eric Dumazet @ 2011-11-29 20:56 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev
In-Reply-To: <4ED53AA1.9090800@hp.com>
Le mardi 29 novembre 2011 à 12:03 -0800, Rick Jones a écrit :
> Is the non-inheritance of the congestion control algorithm a bug or a
> feature?
IMHO its a bug.
Can you provide a fix ?
Thanks !
^ permalink raw reply
* Re: RCU'ed dst_get_neighbour()
From: Roland Dreier @ 2011-11-29 20:56 UTC (permalink / raw)
To: Eric Dumazet
Cc: Marc Aurele La France, David Miller,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1322599991.2596.11.camel@edumazet-laptop>
OK, haven't sent it on yet :)
On Tue, Nov 29, 2011 at 12:53 PM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Le mardi 29 novembre 2011 à 12:47 -0800, Roland Dreier a écrit :
>> Thanks Eric, I'll send this to Linus shortly.
>
> Oh well, I forgot one rcu_read_unlock(), I'll send a V2...
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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: RCU'ed dst_get_neighbour()
From: Eric Dumazet @ 2011-11-29 20:53 UTC (permalink / raw)
To: Roland Dreier
Cc: Marc Aurele La France, David Miller,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAG4TOxNJfr3X1p358LBWdNQKdkw8KOSekcKpNu6K5_phPEiR4A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Le mardi 29 novembre 2011 à 12:47 -0800, Roland Dreier a écrit :
> Thanks Eric, I'll send this to Linus shortly.
Oh well, I forgot one rcu_read_unlock(), I'll send a V2...
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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: RCU'ed dst_get_neighbour()
From: Roland Dreier @ 2011-11-29 20:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: Marc Aurele La France, David Miller,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1322599437.2596.10.camel@edumazet-laptop>
Thanks Eric, I'll send this to Linus shortly.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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: RCU'ed dst_get_neighbour()
From: Eric Dumazet @ 2011-11-29 20:43 UTC (permalink / raw)
To: Marc Aurele La France, David Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Roland Dreier,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1322589661.2596.2.camel@edumazet-laptop>
Le mardi 29 novembre 2011 à 19:01 +0100, Eric Dumazet a écrit :
> Le mardi 29 novembre 2011 à 10:44 -0700, Marc Aurele La France a écrit :
> > Hi.
> >
> > Commit (1) seems to imply that all dst_get_neighbour() references now need
> > to be wrapped with rcu_read_lock()/rcu_read_unlock() sequences. See (2)
> > for one such proposed change.
> >
> > In the case I have here (ipoib), this commit results in ...
> >
> > ===================================================
> > [ INFO: suspicious rcu_dereference_check() usage. ]
> > ---------------------------------------------------
> > include/net/dst.h:91 invoked rcu_dereference_check() without protection!
> >
> > other info that might help us debug this:
> >
> >
> > rcu_scheduler_active = 1, debug_locks = 1
> > 4 locks held by kworker/3:1/630:
> > #0: (ib_cm){.+.+.+}, at: [<ffffffff81055735>]
> > process_one_work+0x1ab/0x4f9
> > #1: ((&(&work->work)->work)){+.+.+.}, at: [<ffffffff81055735>]
> > process_one_work+0x1ab/0x4f9
> > #2: (rcu_read_lock_bh){.+....}, at: [<ffffffff81388216>]
> > dev_queue_xmit+0x0/0x5ae
> > #3: (_xmit_INFINIBAND){+.-...}, at: [<ffffffff8139eecc>]
> > sch_direct_xmit+0x4d/0x22b
> >
> > stack backtrace:
> > Pid: 630, comm: kworker/3:1 Not tainted 3.1.3-smp #1
> > Call Trace:
> > [<ffffffff8106c385>] lockdep_rcu_dereference+0x9b/0xa4
> > [<ffffffff81351cda>] ipoib_start_xmit+0xf4/0x36f
> > [<ffffffff81384215>] dev_hard_start_xmit+0x2a7/0x54f
> > [<ffffffff8139eeef>] sch_direct_xmit+0x70/0x22b
> > [<ffffffff8138851f>] dev_queue_xmit+0x309/0x5ae
> > [<ffffffff81388216>] ? napi_gro_receive+0xb3/0xb3
> > [<ffffffff813582d3>] ipoib_cm_rep_handler+0x208/0x248
> > [<ffffffff81433e16>] ? _raw_spin_unlock_irqrestore+0x3d/0x5b
> > [<ffffffff8135a912>] ipoib_cm_tx_handler+0x95/0x27f
> > [<ffffffff8106d183>] ? __trace_hardirqs_on_caller+0x41/0x65
> > [<ffffffff81327b29>] cm_process_work+0x26/0xbc
> > [<ffffffff81328d74>] cm_rep_handler+0x274/0x2ae
> > [<ffffffff81329582>] cm_work_handler+0x41/0x91
> > [<ffffffff8105582c>] process_one_work+0x2a2/0x4f9
> > [<ffffffff81055735>] ? process_one_work+0x1ab/0x4f9
> > [<ffffffff810580c6>] ? worker_thread+0x4a/0x1ca
> > [<ffffffff81329541>] ? cm_req_handler+0x355/0x355
> > [<ffffffff81058175>] worker_thread+0xf9/0x1ca
> > [<ffffffff8105807c>] ? gcwq_mayday_timeout+0x77/0x77
> > [<ffffffff8105bfa3>] kthread+0x86/0x8e
> > [<ffffffff81436b34>] kernel_thread_helper+0x4/0x10
> > [<ffffffff8143425d>] ? retint_restore_args+0xe/0xe
> > [<ffffffff8105bf1d>] ? kthread_stop+0x1cd/0x1cd
> > [<ffffffff81436b30>] ? gs_change+0xb/0xb
> >
> > ===================================================
> > [ INFO: suspicious rcu_dereference_check() usage. ]
> > ---------------------------------------------------
> > include/net/dst.h:91 invoked rcu_dereference_check() without protection!
> >
> > other info that might help us debug this:
> >
> >
> > rcu_scheduler_active = 1, debug_locks = 1
> > 2 locks held by kworker/u:2/748:
> > #0: ((name)){.+.+.+}, at: [<ffffffff81055735>]
> > process_one_work+0x1ab/0x4f9
> > #1: ((&port_priv->work)){+.+.+.}, at: [<ffffffff81055735>]
> > process_one_work+0x1ab/0x4f9
> >
> > stack backtrace:
> > Pid: 748, comm: kworker/u:2 Not tainted 3.1.3-smp #1
> > Call Trace:
> > [<ffffffff8106c385>] lockdep_rcu_dereference+0x9b/0xa4
> > [<ffffffff81354e68>] ipoib_mcast_join_finish+0x362/0x48a
> > [<ffffffff81355481>] ipoib_mcast_sendonly_join_complete+0x3b/0x174
> > [<ffffffff813246b3>] mcast_work_handler+0xba/0x182
> > [<ffffffff813248aa>] join_handler+0xe6/0xee
> > [<ffffffff81322af1>] ib_sa_mcmember_rec_callback+0x51/0x5c
> > [<ffffffff8132289c>] recv_handler+0x44/0x50
> > [<ffffffff8131efca>] ib_mad_complete_recv+0xc3/0x125
> > [<ffffffff8131debe>] ? find_mad_agent+0x13a/0x149
> > [<ffffffff8131f30a>] ib_mad_recv_done_handler+0x2de/0x326
> > [<ffffffff8131f3b0>] ib_mad_completion_handler+0x5e/0x91
> > [<ffffffff8105582c>] process_one_work+0x2a2/0x4f9
> > [<ffffffff81055735>] ? process_one_work+0x1ab/0x4f9
> > [<ffffffff810580c6>] ? worker_thread+0x4a/0x1ca
> > [<ffffffff8131f352>] ? ib_mad_recv_done_handler+0x326/0x326
> > [<ffffffff81058175>] worker_thread+0xf9/0x1ca
> > [<ffffffff8105807c>] ? gcwq_mayday_timeout+0x77/0x77
> > [<ffffffff8105bfa3>] kthread+0x86/0x8e
> > [<ffffffff81436b34>] kernel_thread_helper+0x4/0x10
> > [<ffffffff8143425d>] ? retint_restore_args+0xe/0xe
> > [<ffffffff8105bf1d>] ? kthread_stop+0x1cd/0x1cd
> > [<ffffffff81436b30>] ? gs_change+0xb/0xb
> >
> > Comments/flames?
> >
> > Thanks.
> >
> > Marc.
> >
> > PS: Please reply-to-all as I am not subscribed to netdev.
> >
> > (1) http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=f2c31e32b378a6653f8de606149d963baf11d7d3
> > (2) http://www.spinics.net/lists/netdev/msg179639.html
> >
> > +----------------------------------+----------------------------------+
> > | Marc Aurele La France | work: 1-780-492-9310 |
> > | Academic Information and | fax: 1-780-492-1729 |
> > | Communications Technologies | email: tsi-yfeSBMgouQgsA/PxXw9srA@public.gmane.org |
> > | 352 General Services Building +----------------------------------+
> > | University of Alberta | |
> > | Edmonton, Alberta | Standard disclaimers apply |
> > | T6G 2H1 | |
> > | CANADA | |
> > +----------------------------------+----------------------------------+
>
> Thanks for the report Marc, I'll take a look asap.
>
>
Hi Marc
Here is the patch I cooked based on your report, thanks a lot !
Eric
[PATCH] net: infiniband/ulp/ipoib: fix lockdep splats
commit f2c31e32b37 (net: fix NULL dereferences in check_peer_redir())
forgot to take care of infiniband uses of dst neighbours.
Many thanks to Marc Aurele who provided a nice bug report.
Reported-by: Marc Aurele La France <tsi-yfeSBMgouQgsA/PxXw9srA@public.gmane.org>
Signed-off-by: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
CC: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
CC: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/ulp/ipoib/ipoib_main.c | 17 ++++++++-------
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 6 +++--
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 7567b60..c36a2ab 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -555,6 +555,7 @@ static int path_rec_start(struct net_device *dev,
return 0;
}
+/* called with rcu_read_lock */
static void neigh_add_path(struct sk_buff *skb, struct net_device *dev)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
@@ -636,6 +637,7 @@ err_drop:
spin_unlock_irqrestore(&priv->lock, flags);
}
+/* called with rcu_read_lock */
static void ipoib_path_lookup(struct sk_buff *skb, struct net_device *dev)
{
struct ipoib_dev_priv *priv = netdev_priv(skb->dev);
@@ -720,13 +722,14 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
struct neighbour *n = NULL;
unsigned long flags;
+ rcu_read_lock();
if (likely(skb_dst(skb)))
n = dst_get_neighbour(skb_dst(skb));
if (likely(n)) {
if (unlikely(!*to_ipoib_neigh(n))) {
ipoib_path_lookup(skb, dev);
- return NETDEV_TX_OK;
+ goto unlock;
}
neigh = *to_ipoib_neigh(n);
@@ -749,17 +752,17 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
ipoib_neigh_free(dev, neigh);
spin_unlock_irqrestore(&priv->lock, flags);
ipoib_path_lookup(skb, dev);
- return NETDEV_TX_OK;
+ goto unlock;
}
if (ipoib_cm_get(neigh)) {
if (ipoib_cm_up(neigh)) {
ipoib_cm_send(dev, skb, ipoib_cm_get(neigh));
- return NETDEV_TX_OK;
+ goto unlock;
}
} else if (neigh->ah) {
ipoib_send(dev, skb, neigh->ah, IPOIB_QPN(n->ha));
- return NETDEV_TX_OK;
+ goto unlock;
}
if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
@@ -793,13 +796,13 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
phdr->hwaddr + 4);
dev_kfree_skb_any(skb);
++dev->stats.tx_dropped;
- return NETDEV_TX_OK;
+ goto unlock;
}
unicast_arp_send(skb, dev, phdr);
}
}
-
+unlock:
return NETDEV_TX_OK;
}
@@ -837,7 +840,7 @@ static int ipoib_hard_header(struct sk_buff *skb,
dst = skb_dst(skb);
n = NULL;
if (dst)
- n = dst_get_neighbour(dst);
+ n = dst_get_neighbour_raw(dst);
if ((!dst || !n) && daddr) {
struct ipoib_pseudoheader *phdr =
(struct ipoib_pseudoheader *) skb_push(skb, sizeof *phdr);
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 1b7a976..cad1894 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -266,7 +266,7 @@ static int ipoib_mcast_join_finish(struct ipoib_mcast *mcast,
skb->dev = dev;
if (dst)
- n = dst_get_neighbour(dst);
+ n = dst_get_neighbour_raw(dst);
if (!dst || !n) {
/* put pseudoheader back on for next time */
skb_push(skb, sizeof (struct ipoib_pseudoheader));
@@ -722,6 +722,8 @@ out:
if (mcast && mcast->ah) {
struct dst_entry *dst = skb_dst(skb);
struct neighbour *n = NULL;
+
+ rcu_read_lock();
if (dst)
n = dst_get_neighbour(dst);
if (n && !*to_ipoib_neigh(n)) {
@@ -734,7 +736,7 @@ out:
list_add_tail(&neigh->list, &mcast->neigh_list);
}
}
-
+ rcu_read_unlock();
spin_unlock_irqrestore(&priv->lock, flags);
ipoib_send(dev, skb, mcast->ah, IB_MULTICAST_QPN);
return;
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 related
* Re: [Bonding-devel] ethernet bonding + VLAN: additional VLAN tag in tcpdump
From: Nicolas de Pesloüan @ 2011-11-29 20:35 UTC (permalink / raw)
To: Jiri Pirko
Cc: Thomas De Schampheleire, bonding-devel, tcpdump-workers,
Ronny Meeus, netdev@vger.kernel.org
In-Reply-To: <CAAXf6LXiONe5Kb8tdQbQs3SWg8cc6SK_md7-vOV8qN4vYmL8qw@mail.gmail.com>
Le 29/11/2011 14:38, Thomas De Schampheleire a écrit :
> Hi,
>
> I'm seeing incorrect tcpdump output in the following scenario:
>
> * ethernet bonding enabled in the kernel, and a single network
> interface (eth0) added as slave
> * bonding mode was set to broadcast, but I don't think this matters
> * VLAN added to the bond0 network interface
> * ip address set on the vlan interface (bond0.1234)
> * tcpdump capturing full packets (-xx or even -x) on the eth0 interface
>
> Then, when pinging from another machine to this ip address, the ping
> reply packets shown by tcpdump incorrectly have a double VLAN tag.
> However, what really appears on the wire is correct: a single VLAN
> tag.
Copied netdev, because bonding and vlan developers are there.
Jiri, don't you think this might be related to the work you have done to make non-hw-accel rx path
similar to hw-accel?
Nicolas.
>
> Here is the output from tcpdump:
> # /tmp/tcpdump -i eth0 -xx
> tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
> listening on eth0, link-type EN10MB (Ethernet), capture size 65535 bytes
> 01:04:04.607880 IP 192.168.1.2> 192.168.1.1: ICMP echo request, id 26933, seq 4
> 16, length 64
> 0x0000: 0600 0000 0020 0600 0000 0020 8100 0ffe
> 0x0010: 0800 4500 0054 0000 4000 4001 b755 c0a8
> 0x0020: 0102 c0a8 0101 0800 98d7 6935 01a0 e528
> 0x0030: 0f2a 0000 0000 0000 0000 0000 0000 0000
> 0x0040: 0000 0000 0000 0000 0000 0000 0000 0000
> 0x0050: 0000 0000 0000 0000 0000 0000 0000 0000
> 0x0060: 0000 0000 0000
> 01:04:04.607889 IP 192.168.1.1> 192.168.1.2: ICMP echo reply, id 26933, seq 416
> , length 64
> 0x0000: 0600 0000 0020 0600 0000 0020 8100 0ffe
> 0x0010: 8100 0ffe 0800 4500 0054 cc07 0000 4001<--------
> extra VLAN header at 0x10
> 0x0020: 2b4e c0a8 0101 c0a8 0102 0000 a0d7 6935
> 0x0030: 01a0 e528 0f2a 0000 0000 0000 0000 0000
> 0x0040: 0000 0000 0000 0000 0000 0000 0000 0000
> 0x0050: 0000 0000 0000 0000 0000 0000 0000 0000
> 0x0060: 0000 0000 0000 0000 0000
>
>
> Initial debugging showed that the addition of the extra VLAN header
> takes place in function pcap_read_linux_mmap() of libpcap, in the
> following snippet:
>
> #ifdef HAVE_TPACKET2
> if (handle->md.tp_version == TPACKET_V2&& h.h2->tp_vlan_tci&&
> tp_snaplen>= 2 * ETH_ALEN) {
> struct vlan_tag *tag;
>
> bp -= VLAN_TAG_LEN;
> memmove(bp, bp + VLAN_TAG_LEN, 2 * ETH_ALEN);
>
> tag = (struct vlan_tag *)(bp + 2 * ETH_ALEN);
> tag->vlan_tpid = htons(ETH_P_8021Q);
> tag->vlan_tci = htons(h.h2->tp_vlan_tci);
>
> pcaphdr.caplen += VLAN_TAG_LEN;
> pcaphdr.len += VLAN_TAG_LEN;
> }
> #endif
>
> Upon entry of this code, the packet in bp already contains a VLAN header.
>
> It's unclear to me where the problem lies exactly. I suspect it has
> something to do with the ethernet bonding layer indicating it has
> hardware vlan tagging support, while it does already fill in the vlan
> header, and libpcap being confused by this.
>
> As mentioned previously, the packets on the wire are correct, and this
> is purely a capturing problem.
>
> Best regards,
> Thomas
>
> ------------------------------------------------------------------------------
> All the data continuously generated in your IT infrastructure
> contains a definitive record of customers, application performance,
> security threats, fraudulent activity, and more. Splunk takes this
> data and makes sense of it. IT sense. And common sense.
> http://p.sf.net/sfu/splunk-novd2d
> _______________________________________________
> Bonding-devel mailing list
> Bonding-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/bonding-devel
>
^ permalink raw reply
* Re: is non-inheritance of congestion control algorithm from the listen socket a bug or a feature?
From: Dave Taht @ 2011-11-29 20:27 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev
In-Reply-To: <4ED53AA1.9090800@hp.com>
Hey, thx for trying it!
On Tue, Nov 29, 2011 at 9:03 PM, Rick Jones <rick.jones2@hp.com> wrote:
>
> Is the non-inheritance of the congestion control algorithm a bug or a
> feature?
Hmm... It's kind of both.
Recently I submitted an RFC patch to the
rsync mailing list, and it seemed nice to be able to set it on a per
socket basis based on various configuration options in the
rsync conf file and what was explicitly allowed.
That patch is here:
https://lists.samba.org/archive/rsync/2011-November/027111.html
It was also way cool to see tcp-lp in action vs other sorts of tcp.
That said we have a range of allowable congestion algorithms
ranging from more intense than cubic to low bandwidth to wireless,
to highly specialized (data center tcp), to some that require
both sides to be running the same alg, others that are server
only...
Your question sort of involves what should a user be able
to set.
On the one hand, in the general case an app should
not be allowed to choose a more aggressive tcp.
On the other hand, an app on a wireless box,
requesting/getting, say, westwood over a default of lp or cubic sort
of makes sense.
On the gripping hand I thiink pushing this level of decison
making out to the app is just fine by me, so inherit
or not, is the app's call to setsockopt...
And in netperf's case, that would be all the algs made
available via allowable_congestion_control.
I had another idea regarding pushing alternate tcp algos out
to more apps without requiring source code changes,
which would be to create a cgroup controller, which
would be both trendy and would provide 'inherit-ability'...
But first up I think would be for more folk to see how
cool different algos can be.
>
> rick jones
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
FR Tel: 0638645374
http://www.bufferbloat.net
^ 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