* Re: [net-next-2.6 PATCH] virtio_net: remove forgotten assignment
From: David Miller @ 2010-03-02 11:57 UTC (permalink / raw)
To: jpirko; +Cc: netdev
In-Reply-To: <20100301112254.GB2799@psychotron.lab.eng.brq.redhat.com>
From: Jiri Pirko <jpirko@redhat.com>
Date: Mon, 1 Mar 2010 12:22:55 +0100
> This is no longer needed. I missed to remove this in
> 567ec874d15b478c8eda7e9a5d2dcb05f13f1fb5
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Applied, thanks.
^ permalink raw reply
* Re: [net-next-2.6 PATCH] infiniband: convert to use netdev_for_each_mc_addr
From: Jiri Pirko @ 2010-03-02 12:01 UTC (permalink / raw)
To: Or Gerlitz
Cc: Jason Gunthorpe, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, Moni Shoua
In-Reply-To: <4B8B7139.9050707-hKgKHo2Ms0FWk0Htik3J/w@public.gmane.org>
Mon, Mar 01, 2010 at 08:48:09AM CET, ogerlitz-smomgflXvOZWk0Htik3J/w@public.gmane.org wrote:
>Jason Gunthorpe wrote:
>> Jiri Pirko wrote:
>>> when bonding changes it's type, flush mc addresses and start over.
>
>> There was a patch posted that tried to do something like what you are describing
>
>Indeed, Jason, commit 75c785 "bonding: remap muticast addresses without using dev_close() and dev_open()" from Moni Shoua <monis-smomgflXvOZWk0Htik3J/w@public.gmane.org> does exactly that, isn't it?
This affects different mc_list.
>
>Or.
--
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: [GIT]: Networking
From: David Miller @ 2010-03-02 12:03 UTC (permalink / raw)
To: torvalds; +Cc: akpm, netdev, linux-kernel
In-Reply-To: <20100228.230535.123985162.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Sun, 28 Feb 2010 23:05:35 -0800 (PST)
> Please pull, thanks a lot!
>
> The following changes since commit 30ff056c42c665b9ea535d8515890857ae382540:
> Linus Torvalds (1):
> Merge branch 'x86-uv-for-linus' of git://git.kernel.org/.../tip/linux-2.6-tip
>
> are available in the git repository at:
>
> master.kernel.org:/pub/scm/linux/kernel/git/davem/net-next-2.6.git master
Linus, if you haven't pulled in the networking tree yet, please pull
again as I just pushed the following bug fixes into there tonight.
Thanks!
The following changes since commit 47871889c601d8199c51a4086f77eebd77c29b0b:
David S. Miller (1):
Merge branch 'master' of /home/davem/src/GIT/linux-2.6/
are available in the git repository at:
master.kernel.org:/pub/scm/linux/kernel/git/davem/net-next-2.6.git master
Eric Dumazet (1):
net: fix protocol sk_buff field
Herton Ronaldo Krzesinski (1):
sis190: fix cable detect via link status poll
Jiri Pirko (1):
virtio_net: remove forgotten assignment
Sathya Perla (1):
be2net: fix tx completion polling
Sridhar Samudrala (1):
bridge: Fix build error when IGMP_SNOOPING is not enabled
drivers/net/benet/be_cmds.c | 26 +++++++++++-----------
drivers/net/benet/be_cmds.h | 2 +-
drivers/net/benet/be_main.c | 47 +++++++++++++++++++++----------------------
drivers/net/sis190.c | 23 ++++++++++++++++----
drivers/net/virtio_net.c | 1 -
include/linux/skbuff.h | 2 +-
net/bridge/br_private.h | 14 ++++++++----
7 files changed, 65 insertions(+), 50 deletions(-)
^ permalink raw reply
* KS8695: possible NAPI issue
From: Yegor Yefremov @ 2010-03-02 12:04 UTC (permalink / raw)
To: netdev; +Cc: figo1802, zealcook
I'm using 2.6.33 kernel and I noticed such a strange behavior:
after system start I transfer one file via netcat from my development
host, after this transfer the network is not functioning i.e. no pings
possible etc.
To narrow down the problem I checked out this commit
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=451f14439847db302e5104c44458b2dbb4b1829d.
Till here the network driver is functioning as intended, but after
NAPI introduction I have this issue. With latest git-pull of "Linus'
kernel tree" I can't even ping right after the systems start.
Any Ideas? What am I missing?
Regards,
Yegor
^ permalink raw reply
* Re: [RFC PATCH]xfrm: fix perpetual bundles
From: jamal @ 2010-03-02 12:11 UTC (permalink / raw)
To: Herbert Xu
Cc: davem, kaber, yoshfuji, nakam, eric.dumazet, netdev,
Steffen Klassert
In-Reply-To: <20100302112754.GA1513@gondor.apana.org.au>
On Tue, 2010-03-02 at 19:27 +0800, Herbert Xu wrote:
> On Wed, Feb 24, 2010 at 08:19:24AM -0500, jamal wrote:
> > 1)In the connect() stage, in the slow path a route cache is
> > created with the rth->fl.fl4_src of 0.0.0.0...
> > ==> policy->bundles is empty, so we do a lookup, fail, create
> > one.. (remember rth->fl.fl4_src of 0.0.0.0 at this stage and
> > thats what we end storing in the bundle/xdst for later comparison
> > instead of the skb's fl)
>
> So this is root number 1. When this stuff was first written this
> case simply wasn't possible. So I think the question we need to
> ask here is can we get a valid address there at the connect stage?
fl->fl4_src is valid non-zero. But in xfrm4_fill_dst()
we do wholesale copy of xdst->u.rt.fl = rt->fl; and rt->fl.fl4_src is
0.0.0.0.
> After all, for non-IPsec connect(2)s, you do get a valid IP address.
> So I don't see why the IPsec case should be different.
>
> Creating a bundle with a zero source address is just a hack to
> make connect(2) succeed immediately. AFAICS getting a valid IP
> address can also be done without waiting for the whole IPsec state
> to be created.
>
I did try to "fix it" above via:
+ if (!xdst->u.rt.fl.fl4_src) {
+ xdst->u.rt.fl.fl4_src = fl->fl4_src;
+ }
But this breaks again later in sendmsg bundle lookup because of
XFRM_SUB_POLICY. If i turned off config XFRM_SUB_POLICY, then
all works. I didnt look closely, but SUB_POLICY does do a memcpy
or two off the dst passed in connect() - which has the wrong src.
So i would have to "fix" a few more spots for it to work. This is
where i gave up concluding that i was just plugging with band-aids.
> Of course if anybody is still interested we could also revisit
> the neighbouresque queueing idea.
not plugged into that discussion..
> > 2)ping sends a packet (does a sendmsg)
> > ==> xfrm_find_bundle() ends up comparing skb's->fl (non-zero
> > fl->fl4_src) with what we stored from #1b. Fails.
> > ==> we create a new bundle at attach the old one at the end of it.
> > ...and now policy->bundles has two xdst entries
>
> This is the way it's supposed to work.
> > 3) Repeat #2, and now we have 3 xdsts in policy bundles
>
> This is what I don't understand. The code is supposed to look
> at every bundle attached to the policy. So why doesn't it find
> the one we created at step #2?
The issue is that the comparison is between xdst->u.rt.fl.fl4_src and
fl->fl4_src. fl->fl4_src is always non-zero. stored
xdst->u.rt.fl.fl4_src is always zero
> In conclusion, I think we have two problems, with the second
> one being the most immediate cause of your symptoms.
Remember the route cache (refer to dst copying above) is created at
connect time;->
So Steffen (on CC) tried to "fix it" by fixing at route cache creation
time. His approach:
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2778,15 +2778,26 @@ int ip_route_output_flow(struct net *net, struct
rtable **rp, struct flowi *flp,
struct sock *sk, int flags)
{
int err;
+ int update_route = 0;
if ((err = __ip_route_output_key(net, rp, flp)) != 0)
return err;
if (flp->proto) {
- if (!flp->fl4_src)
+ if (!flp->fl4_src) {
flp->fl4_src = (*rp)->rt_src;
- if (!flp->fl4_dst)
+ update_route = 1;
+ }
+ if (!flp->fl4_dst) {
flp->fl4_dst = (*rp)->rt_dst;
+ update_route = 1;
+ }
+ if (update_route) {
+ dst_release(&(*rp)->u.dst);
+ if ((err = __ip_route_output_key(net, rp,
flp)) != 0)
+ return err;
+ }
+
err = __xfrm_lookup(net, (struct dst_entry **)rp, flp,
sk,
flags ? XFRM_LOOKUP_WAIT : 0);
if (err == -EREMOTE)
--
I was worried about the impact of this on something else that expects
the behavior.
cheers,
jamal
^ permalink raw reply
* RE: [PATCH 1/1] bnx2x: Tx barriers and locks
From: Vladislav Zolotarov @ 2010-03-02 12:50 UTC (permalink / raw)
To: Stanislaw Gruszka, Michael Chan
Cc: netdev@vger.kernel.org, davem@davemloft.net, Eilon Greenstein,
Matthew Carlson
In-Reply-To: <20100302113032.GA2362@dhcp-lab-161.englab.brq.redhat.com>
Stanislaw barrier() is not a memory barrier - it's a compiler barrier. I don't think removing it from bnx2x_tx_avail() will improve anything. If u think I'm wrong, could u, pls., provide a specific example.
Regarding the patch u've proposed - I don't see any obvious benefit in the code change u've suggested except for replacing the smp_wmb() to smp_mb() in the bnx2x_tx_int() and would prefer to wait for Dave's reply on that matter.
Let's recall that the bxn2x_start_xmit() is called under tx_lock(), so no additional memory barriers needed to ensure that bnx2x_tx_avail() has fresh values of BD consumer and producer at the beginning of bnx2x_start_xmit(). That's why we don't need the more complicated logic putting the queue asleep at the beginning of bnx2x_start_xmit() as we need at the end. That's why I think your "goto" change in the beginning of bnx2x_start_xmit() is suboptimal.
Thanks,
vlad
> -----Original Message-----
> From: Stanislaw Gruszka [mailto:sgruszka@redhat.com]
> Sent: Tuesday, March 02, 2010 1:31 PM
> To: Michael Chan
> Cc: Vladislav Zolotarov; netdev@vger.kernel.org;
> davem@davemloft.net; Eilon Greenstein; Matthew Carlson
> Subject: Re: [PATCH 1/1] bnx2x: Tx barriers and locks
>
> On Mon, Mar 01, 2010 at 09:59:07AM -0800, Michael Chan wrote:
> > > There is still difference between what we have in bnx2x
> and bnx2/tg3
> > > regarding memory barriers in tx_poll/start_xmit code.
> Mainly we have
> > > smp_mb() in bnx2/tg3_tx_avail(), and in bnx2/tg3_tx_int()
> is smp_mb()
> > > not smp_wmb(). I do not see that bnx2x is wrong, but
> would like to know
> > > why there is a difference, maybe bnx2/tg3 should be changed?
> > >
> >
> > The memory barrier in tx_int() is to make the tx index update happen
> > before the netif_tx_queue_stopped() check. The barrier is
> to prevent a
> > situation like this:
> >
> > CPU0 CPU1
> > start_xmit()
> > if (tx_ring_full) {
> > tx_int()
> >
> if (!netif_tx_queue_stopped)
> > netif_tx_stop_queue()
> > if (!tx_ring_full)
> >
> update_tx_index
> > netif_tx_wake_queue()
> > }
> >
> >
> > The update_tx_index code is before the if statement in
> program order,
> > but the CPU and/or compiler can reorder it as shown above.
> smp_mb() will
> > prevent that. Will smp_wmb() prevent that as well?
>
> No. smp_wmb() affect only write orders on CPU1 performing tx_int(), so
> that should be fixed in bnx2x.
>
> Regarding memory barrier in tx_avail(), I don't think it its
> needed for
> anything, except maybe usage at the beginning of
> start_xmit(), but we can
> just remove that like in the patch below. I going to post "official"
> patches for tg3, bnx2 and bnx2x, if no nobody has nothing against.
>
> diff --git a/drivers/net/bnx2x_main.c b/drivers/net/bnx2x_main.c
> index ed785a3..0f406b8 100644
> --- a/drivers/net/bnx2x_main.c
> +++ b/drivers/net/bnx2x_main.c
> @@ -893,7 +893,6 @@ static inline u16 bnx2x_tx_avail(struct
> bnx2x_fastpath *fp)
> u16 prod;
> u16 cons;
>
> - barrier(); /* Tell compiler that prod and cons can change */
> prod = fp->tx_bd_prod;
> cons = fp->tx_bd_cons;
>
> @@ -963,9 +962,8 @@ static int bnx2x_tx_int(struct bnx2x_fastpath *fp)
> * start_xmit() will miss it and cause the queue to be stopped
> * forever.
> */
> - smp_wmb();
> + smp_mb();
>
> - /* TBD need a thresh? */
> if (unlikely(netif_tx_queue_stopped(txq))) {
> /* Taking tx_lock() is needed to prevent
> reenabling the queue
> * while it's empty. This could have happen if
> rx_action() gets
> @@ -11177,10 +11175,9 @@ static netdev_tx_t
> bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
> struct eth_tx_bd *tx_data_bd, *total_pkt_bd = NULL;
> struct eth_tx_parse_bd *pbd = NULL;
> u16 pkt_prod, bd_prod;
> - int nbd, fp_index;
> + int nbd, fp_index, i, ret;
> dma_addr_t mapping;
> u32 xmit_type = bnx2x_xmit_type(bp, skb);
> - int i;
> u8 hlen = 0;
> __le16 pkt_size = 0;
>
> @@ -11195,10 +11192,9 @@ static netdev_tx_t
> bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
> fp = &bp->fp[fp_index];
>
> if (unlikely(bnx2x_tx_avail(fp) <
> (skb_shinfo(skb)->nr_frags + 3))) {
> - fp->eth_q_stats.driver_xoff++;
> - netif_tx_stop_queue(txq);
> BNX2X_ERR("BUG! Tx ring full when queue awake!\n");
> - return NETDEV_TX_BUSY;
> + ret = NETDEV_TX_BUSY;
>
> + goto stop_queue;
> }
>
> DP(NETIF_MSG_TX_QUEUED, "SKB: summed %x protocol %x
> protocol(%x,%x)"
> @@ -11426,19 +11422,24 @@ static netdev_tx_t
> bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
> mmiowb();
>
> fp->tx_bd_prod += nbd;
> -
> - if (unlikely(bnx2x_tx_avail(fp) < MAX_SKB_FRAGS + 3)) {
> - netif_tx_stop_queue(txq);
> - /* We want bnx2x_tx_int to "see" the updated tx_bd_prod
> - if we put Tx into XOFF state. */
> - smp_mb();
> - fp->eth_q_stats.driver_xoff++;
> - if (bnx2x_tx_avail(fp) >= MAX_SKB_FRAGS + 3)
> - netif_tx_wake_queue(txq);
> - }
> fp->tx_pkt++;
> +
> + ret = NETDEV_TX_OK;
> + if (unlikely(bnx2x_tx_avail(fp) < MAX_SKB_FRAGS + 3))
> + goto stop_queue;
> +
> + return ret;
>
> - return NETDEV_TX_OK;
> +stop_queue:
> + netif_tx_stop_queue(txq);
> + /* paired barrier is in bnx2x_tx_int(), update of tx_bd_cons
> + * have to be visable here, after we XOFF bit setting */
> + smp_mb();
> + fp->eth_q_stats.driver_xoff++;
> + if (bnx2x_tx_avail(fp) >= MAX_SKB_FRAGS + 3)
> + netif_tx_wake_queue(txq);
> +
> + return ret;
> }
>
> /* called with rtnl_lock */
>
>
^ permalink raw reply
* Re: [RFC PATCH]xfrm: fix perpetual bundles
From: Herbert Xu @ 2010-03-02 12:51 UTC (permalink / raw)
To: jamal; +Cc: davem, kaber, yoshfuji, nakam, eric.dumazet, netdev,
Steffen Klassert
In-Reply-To: <1267531905.21749.21.camel@bigi>
On Tue, Mar 02, 2010 at 07:11:45AM -0500, jamal wrote:
>
> fl->fl4_src is valid non-zero. But in xfrm4_fill_dst()
I see, so problem #1 doesn't exist.
> we do wholesale copy of xdst->u.rt.fl = rt->fl; and rt->fl.fl4_src is
> 0.0.0.0.
Heh, you've just discovered a bug that I carefully planted back
in 2007, while merging the v4/v6 policy code :)
It is a clear merging error, where *fl became rt->fl which is
totally different. So please try this patch:
ipsec: Fix bogus bundle flowi
When I merged the bundle creation code, I introduced a bogus
flowi value in the bundle. Instead of getting from the caller,
it was instead set to the flow in the route object, which is
totally different.
The end result is that the bundles we created never match, and
we instead end up with an ever growing bundle list.
Thanks to Jamal for find this problem.
Reported-by: Jamal Hadi Salim <hadi@cyberus.ca>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 60c2770..1e355d8 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -274,7 +274,8 @@ struct xfrm_policy_afinfo {
struct dst_entry *dst,
int nfheader_len);
int (*fill_dst)(struct xfrm_dst *xdst,
- struct net_device *dev);
+ struct net_device *dev,
+ struct flowi *fl);
};
extern int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo);
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 67107d6..e4a1483 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -91,11 +91,12 @@ static int xfrm4_init_path(struct xfrm_dst *path, struct dst_entry *dst,
return 0;
}
-static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev)
+static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
+ struct flowi *fl)
{
struct rtable *rt = (struct rtable *)xdst->route;
- xdst->u.rt.fl = rt->fl;
+ xdst->u.rt.fl = *fl;
xdst->u.dst.dev = dev;
dev_hold(dev);
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index dbdc696..ae18165 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -116,7 +116,8 @@ static int xfrm6_init_path(struct xfrm_dst *path, struct dst_entry *dst,
return 0;
}
-static int xfrm6_fill_dst(struct xfrm_dst *xdst, struct net_device *dev)
+static int xfrm6_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
+ struct flowi *fl)
{
struct rt6_info *rt = (struct rt6_info*)xdst->route;
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 0ecb16a..f12dd3d 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1354,7 +1354,8 @@ static inline int xfrm_init_path(struct xfrm_dst *path, struct dst_entry *dst,
return err;
}
-static inline int xfrm_fill_dst(struct xfrm_dst *xdst, struct net_device *dev)
+static inline int xfrm_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
+ struct flowi *fl)
{
struct xfrm_policy_afinfo *afinfo =
xfrm_policy_get_afinfo(xdst->u.dst.ops->family);
@@ -1363,7 +1364,7 @@ static inline int xfrm_fill_dst(struct xfrm_dst *xdst, struct net_device *dev)
if (!afinfo)
return -EINVAL;
- err = afinfo->fill_dst(xdst, dev);
+ err = afinfo->fill_dst(xdst, dev, fl);
xfrm_policy_put_afinfo(afinfo);
@@ -1468,7 +1469,7 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
for (dst_prev = dst0; dst_prev != dst; dst_prev = dst_prev->child) {
struct xfrm_dst *xdst = (struct xfrm_dst *)dst_prev;
- err = xfrm_fill_dst(xdst, dev);
+ err = xfrm_fill_dst(xdst, dev, fl);
if (err)
goto free_dst;
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Cedric Le Goater @ 2010-03-02 13:10 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Daniel Lezcano, Pavel Emelyanov, Linux Netdev List, containers,
Netfilter Development Mailinglist, Ben Greear
In-Reply-To: <m1ljebwwgd.fsf@fess.ebiederm.org>
On 03/01/2010 10:42 PM, Eric W. Biederman wrote:
> I am of two mind about my patches. Right now they are a brilliant
> proof of concept that we can name namespaces without needing a
> namespace for the names of namespaces, and start to be a practical
> solution to the join problem. At the same time, I'm not certain
> I like a solution that requires yet more syscalls so I ask myself
> is there not yet a simpler way.
thinking aloud,
what if you made the nsproxy a vfs_inode ? we could then mount the nsfs
to do all sorts of fs operations on the object, like notifying easily
its deletion. we would need to find a meaningful name, probably the inode
number.
one syscall (nsfd) would be required to get the nsproxy of a task (pid).
you can't guess that from an inode number.
C.
^ permalink raw reply
* Re: [RFC PATCH]xfrm: fix perpetual bundles
From: jamal @ 2010-03-02 13:10 UTC (permalink / raw)
To: Herbert Xu
Cc: davem, kaber, yoshfuji, nakam, eric.dumazet, netdev,
Steffen Klassert
In-Reply-To: <20100302125156.GA2420@gondor.apana.org.au>
On Tue, 2010-03-02 at 20:51 +0800, Herbert Xu wrote:
> Heh, you've just discovered a bug that I carefully planted back
> in 2007, while merging the v4/v6 policy code :)
;-> I am suprised it hasnt been noticed sooner given it accumulates
memory on a per-packet basis.
> It is a clear merging error, where *fl became rt->fl which is
> totally different. So please try this patch:
>
Looks like it would work.
I dont have time right now - but will by either tonight or tomorrow
evening. Steffen, if you have time - please go ahead and try it out
as well.
cheers,
jamal
^ permalink raw reply
* Re: Panic at tcp_xmit_retransmit_queue
From: sbs @ 2010-03-02 13:16 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: Netdev, LKML
In-Reply-To: <alpine.DEB.2.00.1002151520250.7063@wel-95.cs.helsinki.fi>
thank you very much, have stable running server for a week and it
seems that it works like a charm now, i havent detected any panics
since i apply the patch. although seems that the problem stops
ocurring cause i dont see any debug information through netconsole
On Mon, Feb 15, 2010 at 4:21 PM, Ilpo Järvinen
<ilpo.jarvinen@helsinki.fi> wrote:
> On Wed, 3 Feb 2010, Ilpo Järvinen wrote:
>
>> On Mon, 1 Feb 2010, sbs wrote:
>>
>> > actually removing netconsole from kernel didnt help.
>> > i found many guys with the same problem but with different hardware
>> > configurations here:
>> >
>> > freez in TCP stack :
>> > http://bugzilla.kernel.org/show_bug.cgi?id=14470
>> >
>> > is there someone who can investigate it?
>> >
>> >
>> > On Tue, Jan 19, 2010 at 7:13 PM, sbs <gexlie@gmail.com> wrote:
>> > > We are hiting kernel panics on servers with nVidia MCP55 NICs once a day;
>> > > it appears usualy under a high network trafic ( around 10000Mbit/s) but
>> > > it is not a rule, it has happened even on low trafic.
>> > >
>> > > Servers are used as nginx+static content
>> > > On 2 equal servers this panic happens aprox 2 times a day depending on
>> > > network load. Machine completly freezes till the netconsole reboots.
>> > >
>> > > Kernel: 2.6.32.3
>> > >
>> > > what can it be? whats wrong with tcp_xmit_retransmit_queue() function ?
>> > > can anyone explain or fix?
>>
>> You might want to try with to debug patch below. It might even make the
>> box to survive the event (if I got it coded right).
>
> Here should be a better version of the debug patch, hopefully the infinite
> looping is now gone.
>
> --
> i.
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 383ce23..4672a30 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2186,6 +2186,42 @@ static int tcp_can_forward_retransmit(struct sock *sk)
> return 1;
> }
>
> +static void print_queue(struct sock *sk, struct sk_buff *old, struct sk_buff *hole)
> +{
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct sk_buff *skb, *prev;
> +
> + skb = tcp_write_queue_head(sk);
> + prev = (struct sk_buff *)(&sk->sk_write_queue);
> +
> + if (skb == NULL) {
> + printk("NULL head, pkts %u\n", tp->packets_out);
> + return;
> + }
> + printk("head %p tail %p sendhead %p oldhint %p now %p hole %p high %u\n",
> + tcp_write_queue_head(sk), tcp_write_queue_tail(sk),
> + tcp_send_head(sk), old, tp->retransmit_skb_hint, hole,
> + tp->retransmit_high);
> +
> + while (skb) {
> + printk("skb %p (%u-%u) next %p prev %p sacked %u\n",
> + skb, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq,
> + skb->next, skb->prev, TCP_SKB_CB(skb)->sacked);
> + if (prev != skb->prev)
> + printk("Inconsistent prev\n");
> +
> + if (skb == tcp_write_queue_tail(sk)) {
> + if (skb->next != (struct sk_buff *)(&sk->sk_write_queue))
> + printk("Improper next at tail\n");
> + return;
> + }
> +
> + prev = skb;
> + skb = skb->next;
> + }
> + printk("Encountered unexpected NULL\n");
> +}
> +
> /* This gets called after a retransmit timeout, and the initially
> * retransmitted data is acknowledged. It tries to continue
> * resending the rest of the retransmit queue, until either
> @@ -2194,12 +2230,15 @@ static int tcp_can_forward_retransmit(struct sock *sk)
> * based retransmit packet might feed us FACK information again.
> * If so, we use it to avoid unnecessarily retransmissions.
> */
> +static int caught_it = 0;
> +
> void tcp_xmit_retransmit_queue(struct sock *sk)
> {
> const struct inet_connection_sock *icsk = inet_csk(sk);
> struct tcp_sock *tp = tcp_sk(sk);
> struct sk_buff *skb;
> struct sk_buff *hole = NULL;
> + struct sk_buff *old = tp->retransmit_skb_hint;
> u32 last_lost;
> int mib_idx;
> int fwd_rexmitting = 0;
> @@ -2217,6 +2256,16 @@ void tcp_xmit_retransmit_queue(struct sock *sk)
> last_lost = tp->snd_una;
> }
>
> +checknull:
> + if (skb == NULL) {
> + if (!caught_it)
> + print_queue(sk, old, hole);
> + caught_it++;
> + if (net_ratelimit())
> + printk("Errors caught so far %u\n", caught_it);
> + return;
> + }
> +
> tcp_for_write_queue_from(skb, sk) {
> __u8 sacked = TCP_SKB_CB(skb)->sacked;
>
> @@ -2257,7 +2306,7 @@ begin_fwd:
> } else if (!(sacked & TCPCB_LOST)) {
> if (hole == NULL && !(sacked & (TCPCB_SACKED_RETRANS|TCPCB_SACKED_ACKED)))
> hole = skb;
> - continue;
> + goto cont;
>
> } else {
> last_lost = TCP_SKB_CB(skb)->end_seq;
> @@ -2268,7 +2317,7 @@ begin_fwd:
> }
>
> if (sacked & (TCPCB_SACKED_ACKED|TCPCB_SACKED_RETRANS))
> - continue;
> + goto cont;
>
> if (tcp_retransmit_skb(sk, skb))
> return;
> @@ -2278,6 +2327,9 @@ begin_fwd:
> inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
> inet_csk(sk)->icsk_rto,
> TCP_RTO_MAX);
> +cont:
> + skb = skb->next;
> + goto checknull;
> }
> }
>
^ permalink raw reply
* Re: [RFC PATCH]xfrm: fix perpetual bundles
From: Steffen Klassert @ 2010-03-02 13:46 UTC (permalink / raw)
To: jamal; +Cc: Herbert Xu, davem, kaber, yoshfuji, nakam, eric.dumazet, netdev
In-Reply-To: <1267535426.21749.24.camel@bigi>
On Tue, Mar 02, 2010 at 08:10:26AM -0500, jamal wrote:
> On Tue, 2010-03-02 at 20:51 +0800, Herbert Xu wrote:
>
> > Heh, you've just discovered a bug that I carefully planted back
> > in 2007, while merging the v4/v6 policy code :)
>
> ;-> I am suprised it hasnt been noticed sooner given it accumulates
> memory on a per-packet basis.
The problem was spotted by
commit a33bc5c15154c835aae26f16e6a3a7d9ad4acb45
xfrm: select sane defaults for xfrm[4|6] gc_thresh
Before this commit, the xfrm garbage collector started to remove
stale bundle entries as soon as we reached an amount of 1024
bundle entries. Now the default value for the gc_thresh is
based on the main route table hash size, so we can have much more
bundle entries.
>
> > It is a clear merging error, where *fl became rt->fl which is
> > totally different. So please try this patch:
> >
>
> Looks like it would work.
> I dont have time right now - but will by either tonight or tomorrow
> evening. Steffen, if you have time - please go ahead and try it out
> as well.
>
I tried it, works for me too.
Thanks,
Steffen
^ permalink raw reply
* Re: [RFC PATCH]xfrm: fix perpetual bundles
From: jamal @ 2010-03-02 13:54 UTC (permalink / raw)
To: Steffen Klassert
Cc: Herbert Xu, davem, kaber, yoshfuji, nakam, eric.dumazet, netdev
In-Reply-To: <20100302134656.GC20508@secunet.com>
On Tue, 2010-03-02 at 14:46 +0100, Steffen Klassert wrote:
> The problem was spotted by
> commit a33bc5c15154c835aae26f16e6a3a7d9ad4acb45
> xfrm: select sane defaults for xfrm[4|6] gc_thresh
>
> Before this commit, the xfrm garbage collector started to remove
> stale bundle entries as soon as we reached an amount of 1024
> bundle entries. Now the default value for the gc_thresh is
> based on the main route table hash size, so we can have much more
> bundle entries.
yikes. Ok. Seems this fix needs to go -stable as well then.
> I tried it, works for me too.
Did you try with CONFIG_XFRM_SUB_POLICY=y. Thats the only reason
i said "looks like it might work". If you tried with that, then
I dont need to test and I can add an ACKed-by;->
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 1/1] bnx2x: Tx barriers and locks
From: Stanislaw Gruszka @ 2010-03-02 13:55 UTC (permalink / raw)
To: Vladislav Zolotarov
Cc: Michael Chan, netdev@vger.kernel.org, davem@davemloft.net,
Eilon Greenstein, Matthew Carlson
In-Reply-To: <8628FE4E7912BF47A96AE7DD7BAC0AADCB46A2B40C@SJEXCHCCR02.corp.ad.broadcom.com>
On Tue, Mar 02, 2010 at 04:50:59AM -0800, Vladislav Zolotarov wrote:
> Stanislaw barrier() is not a memory barrier - it's a compiler barrier. I don't think removing it from bnx2x_tx_avail() will improve anything. If u think I'm wrong, could u, pls., provide a specific example.
Only improvement is removing confusing code, And comment like
"Tell compiler that prod and cons can change" is even more
confusing. If you think I'm wrong, just tell as why that
barrier is needed :)
For bnx2 and tg3 that would be removal of smp_mb().
> Let's recall that the bxn2x_start_xmit() is called under tx_lock(), so no additional memory barriers needed to ensure that bnx2x_tx_avail() has fresh values of BD consumer and producer at the beginning of bnx2x_start_xmit().
Memory barriers are not about refreshing anything, they are
about ordering.
> That's why we don't need the more complicated logic putting the queue asleep at the beginning of bnx2x_start_xmit() as we need at the end. That's why I think your "goto" change in the beginning of bnx2x_start_xmit() is suboptimal.
You may have right, about that smb_mb() is not needed at the beginning
of start_xmit(), but I'm not sure for that, since we are changing tx_bd_cons
without netif_tx_lock and setting XOFF bit there. If we can assume that
at the beginning of start_xmit() there is always enough space it hw queue,
we can just remove tx_avail() check. Otherwise IMHO is better to put queue
asleep in safe way.
About performance, adding smp_mb() on bug/unused code path does not hurt
much :)
Cheers
Stanislaw
> > -----Original Message-----
> > From: Stanislaw Gruszka [mailto:sgruszka@redhat.com]
> > Sent: Tuesday, March 02, 2010 1:31 PM
> > To: Michael Chan
> > Cc: Vladislav Zolotarov; netdev@vger.kernel.org;
> > davem@davemloft.net; Eilon Greenstein; Matthew Carlson
> > Subject: Re: [PATCH 1/1] bnx2x: Tx barriers and locks
> >
> > On Mon, Mar 01, 2010 at 09:59:07AM -0800, Michael Chan wrote:
> > > > There is still difference between what we have in bnx2x
> > and bnx2/tg3
> > > > regarding memory barriers in tx_poll/start_xmit code.
> > Mainly we have
> > > > smp_mb() in bnx2/tg3_tx_avail(), and in bnx2/tg3_tx_int()
> > is smp_mb()
> > > > not smp_wmb(). I do not see that bnx2x is wrong, but
> > would like to know
> > > > why there is a difference, maybe bnx2/tg3 should be changed?
> > > >
> > >
> > > The memory barrier in tx_int() is to make the tx index update happen
> > > before the netif_tx_queue_stopped() check. The barrier is
> > to prevent a
> > > situation like this:
> > >
> > > CPU0 CPU1
> > > start_xmit()
> > > if (tx_ring_full) {
> > > tx_int()
> > >
> > if (!netif_tx_queue_stopped)
> > > netif_tx_stop_queue()
> > > if (!tx_ring_full)
> > >
> > update_tx_index
> > > netif_tx_wake_queue()
> > > }
> > >
> > >
> > > The update_tx_index code is before the if statement in
> > program order,
> > > but the CPU and/or compiler can reorder it as shown above.
> > smp_mb() will
> > > prevent that. Will smp_wmb() prevent that as well?
> >
> > No. smp_wmb() affect only write orders on CPU1 performing tx_int(), so
> > that should be fixed in bnx2x.
> >
> > Regarding memory barrier in tx_avail(), I don't think it its
> > needed for
> > anything, except maybe usage at the beginning of
> > start_xmit(), but we can
> > just remove that like in the patch below. I going to post "official"
> > patches for tg3, bnx2 and bnx2x, if no nobody has nothing against.
> >
> > diff --git a/drivers/net/bnx2x_main.c b/drivers/net/bnx2x_main.c
> > index ed785a3..0f406b8 100644
> > --- a/drivers/net/bnx2x_main.c
> > +++ b/drivers/net/bnx2x_main.c
> > @@ -893,7 +893,6 @@ static inline u16 bnx2x_tx_avail(struct
> > bnx2x_fastpath *fp)
> > u16 prod;
> > u16 cons;
> >
> > - barrier(); /* Tell compiler that prod and cons can change */
> > prod = fp->tx_bd_prod;
> > cons = fp->tx_bd_cons;
> >
> > @@ -963,9 +962,8 @@ static int bnx2x_tx_int(struct bnx2x_fastpath *fp)
> > * start_xmit() will miss it and cause the queue to be stopped
> > * forever.
> > */
> > - smp_wmb();
> > + smp_mb();
> >
> > - /* TBD need a thresh? */
> > if (unlikely(netif_tx_queue_stopped(txq))) {
> > /* Taking tx_lock() is needed to prevent
> > reenabling the queue
> > * while it's empty. This could have happen if
> > rx_action() gets
> > @@ -11177,10 +11175,9 @@ static netdev_tx_t
> > bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
> > struct eth_tx_bd *tx_data_bd, *total_pkt_bd = NULL;
> > struct eth_tx_parse_bd *pbd = NULL;
> > u16 pkt_prod, bd_prod;
> > - int nbd, fp_index;
> > + int nbd, fp_index, i, ret;
> > dma_addr_t mapping;
> > u32 xmit_type = bnx2x_xmit_type(bp, skb);
> > - int i;
> > u8 hlen = 0;
> > __le16 pkt_size = 0;
> >
> > @@ -11195,10 +11192,9 @@ static netdev_tx_t
> > bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
> > fp = &bp->fp[fp_index];
> >
> > if (unlikely(bnx2x_tx_avail(fp) <
> > (skb_shinfo(skb)->nr_frags + 3))) {
> > - fp->eth_q_stats.driver_xoff++;
> > - netif_tx_stop_queue(txq);
> > BNX2X_ERR("BUG! Tx ring full when queue awake!\n");
> > - return NETDEV_TX_BUSY;
> > + ret = NETDEV_TX_BUSY;
> >
> > + goto stop_queue;
> > }
> >
> > DP(NETIF_MSG_TX_QUEUED, "SKB: summed %x protocol %x
> > protocol(%x,%x)"
> > @@ -11426,19 +11422,24 @@ static netdev_tx_t
> > bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
> > mmiowb();
> >
> > fp->tx_bd_prod += nbd;
> > -
> > - if (unlikely(bnx2x_tx_avail(fp) < MAX_SKB_FRAGS + 3)) {
> > - netif_tx_stop_queue(txq);
> > - /* We want bnx2x_tx_int to "see" the updated tx_bd_prod
> > - if we put Tx into XOFF state. */
> > - smp_mb();
> > - fp->eth_q_stats.driver_xoff++;
> > - if (bnx2x_tx_avail(fp) >= MAX_SKB_FRAGS + 3)
> > - netif_tx_wake_queue(txq);
> > - }
> > fp->tx_pkt++;
> > +
> > + ret = NETDEV_TX_OK;
> > + if (unlikely(bnx2x_tx_avail(fp) < MAX_SKB_FRAGS + 3))
> > + goto stop_queue;
> > +
> > + return ret;
> >
> > - return NETDEV_TX_OK;
> > +stop_queue:
> > + netif_tx_stop_queue(txq);
> > + /* paired barrier is in bnx2x_tx_int(), update of tx_bd_cons
> > + * have to be visable here, after we XOFF bit setting */
> > + smp_mb();
> > + fp->eth_q_stats.driver_xoff++;
> > + if (bnx2x_tx_avail(fp) >= MAX_SKB_FRAGS + 3)
> > + netif_tx_wake_queue(txq);
> > +
> > + return ret;
> > }
> >
> > /* called with rtnl_lock */
> >
> >
^ permalink raw reply
* Re: Gianfar driver failing on MPC8641D based board
From: Anton Vorontsov @ 2010-03-02 14:02 UTC (permalink / raw)
To: Kumar Gopalpet-B05799
Cc: Paul Gortmaker, Martyn Welch, netdev, linux-kernel,
linuxppc-dev list, davem
In-Reply-To: <9F4C7D19E8361D4C94921B95BE08B81BC93EC3@zin33exm22.fsl.freescale.net>
Hi!
On Sat, Feb 27, 2010 at 11:05:32AM +0530, Kumar Gopalpet-B05799 wrote:
[...]
> Understood, and thanks for the explanation. Am I correct in saying that
> this is
> due to the out-of-order execution capability on powerpc ?
Nope, that was just a logic issue in the driver.
Though, with the patch, the eieio() is needed so that compiler (or CPU)
won't reorder lstatus and skbuff writes.
> I have one more question, why don't we use use atomic_t for num_txbdfree
> and
> completely do away with spin_locks in gfar_clean_tx_ring() and
> gfar_start_xmit().
> In an non-SMP, scenario I would feel there is absolutely no requirement
> of spin_locks
> and in case of SMP atomic operation would be much more safer on powerpc
> rather than spin_locks.
>
> What is your suggestion ?
I think that's a good idea.
However, in start_xmit() we'll have to keep the spinlock anyway
since it also protects from gfar_error(), which can modify
regs->tstat.
Thanks!
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [RFC PATCH]xfrm: fix perpetual bundles
From: David Miller @ 2010-03-02 14:06 UTC (permalink / raw)
To: hadi
Cc: steffen.klassert, herbert, kaber, yoshfuji, nakam, eric.dumazet,
netdev
In-Reply-To: <1267538070.21749.27.camel@bigi>
From: jamal <hadi@cyberus.ca>
Date: Tue, 02 Mar 2010 08:54:30 -0500
> On Tue, 2010-03-02 at 14:46 +0100, Steffen Klassert wrote:
>
>> The problem was spotted by
>> commit a33bc5c15154c835aae26f16e6a3a7d9ad4acb45
>> xfrm: select sane defaults for xfrm[4|6] gc_thresh
>>
>> Before this commit, the xfrm garbage collector started to remove
>> stale bundle entries as soon as we reached an amount of 1024
>> bundle entries. Now the default value for the gc_thresh is
>> based on the main route table hash size, so we can have much more
>> bundle entries.
>
> yikes. Ok. Seems this fix needs to go -stable as well then.
Jamal please do some testing then I'll push Herbert's fix around.
^ permalink raw reply
* Re: [RFC PATCH]xfrm: fix perpetual bundles
From: Steffen Klassert @ 2010-03-02 14:06 UTC (permalink / raw)
To: jamal; +Cc: Herbert Xu, davem, kaber, yoshfuji, nakam, eric.dumazet, netdev
In-Reply-To: <1267538070.21749.27.camel@bigi>
On Tue, Mar 02, 2010 at 08:54:30AM -0500, jamal wrote:
>
> yikes. Ok. Seems this fix needs to go -stable as well then.
Indeed, it should.
>
> > I tried it, works for me too.
>
> Did you try with CONFIG_XFRM_SUB_POLICY=y. Thats the only reason
> i said "looks like it might work". If you tried with that, then
> I dont need to test and I can add an ACKed-by;->
>
Yes, I tested with CONFIG_XFRM_SUB_POLICY=y.
So for me I can add an
Acked-by: Steffen Klassert <steffen.klassert@secunet.com>
^ permalink raw reply
* Re: KS8695: possible NAPI issue
From: Dick Hollenbeck @ 2010-03-02 14:09 UTC (permalink / raw)
To: Yegor Yefremov; +Cc: netdev, figo1802, zealcook
In-Reply-To: <f69abfc31003020404y7af77fe5icefeee4644c7d08e@mail.gmail.com>
Yegor Yefremov wrote:
> I'm using 2.6.33 kernel and I noticed such a strange behavior:
>
> after system start I transfer one file via netcat from my development
> host, after this transfer the network is not functioning i.e. no pings
> possible etc.
>
> To narrow down the problem I checked out this commit
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=451f14439847db302e5104c44458b2dbb4b1829d.
> Till here the network driver is functioning as intended, but after
> NAPI introduction I have this issue. With latest git-pull of "Linus'
> kernel tree" I can't even ping right after the systems start.
>
> Any Ideas? What am I missing?
>
No idea, although I am using the same ARM chip, my kernel is at
2.6.30.5, and except for this occasional loss of connection I get, the
ethernet driver seems to work better than what you are reporting.
I would get your printk() working to the internal ring buffer, and then
simply instrument the key pieces of code with printk()s until the
situation becomes clear. You can also watch
# cat /proc/interrupts to see which interrupt handlers are firing,
before and after an activity. Reception of a frame all starts with an
interrupt handler usually.
printk() should be able to print even from within an interrupt handler,
so long as you are only going to that internal ring buffer.
Dick
> Regards,
> Yegor
>
>
^ permalink raw reply
* Re: [RFC PATCH]xfrm: fix perpetual bundles
From: jamal @ 2010-03-02 14:16 UTC (permalink / raw)
To: David Miller
Cc: steffen.klassert, herbert, kaber, yoshfuji, nakam, eric.dumazet,
netdev
In-Reply-To: <20100302.060606.73203093.davem@davemloft.net>
On Tue, 2010-03-02 at 06:06 -0800, David Miller wrote:
> Jamal please do some testing then I'll push Herbert's fix around.
I will get to it tonight.
cheers,
jamal
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Pavel Emelyanov @ 2010-03-02 15:03 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Eric W. Biederman, hadi, Patrick McHardy, Linux Netdev List,
containers, Netfilter Development Mailinglist, Ben Greear,
Serge Hallyn, Matt Helsley
In-Reply-To: <4B8AE8C1.1030305@free.fr>
> I agree with all the points you and Pavel you talked about but I don't
> feel comfortable to have the current process to switch the pid namespace
> because of the process tree hierarchy (what will be the parent of the
> process when you enter the pid namespace for example).
The answer is - the one, that used to be. I see no problems with it.
Do you?
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Jan Engelhardt @ 2010-03-02 15:14 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: Daniel Lezcano, Eric W. Biederman, hadi, Patrick McHardy,
Linux Netdev List, containers, Netfilter Development Mailinglist,
Ben Greear, Serge Hallyn, Matt Helsley
In-Reply-To: <4B8D28CF.8060304@parallels.com>
On Tuesday 2010-03-02 16:03, Pavel Emelyanov wrote:
>> I agree with all the points you and Pavel you talked about but I don't
>> feel comfortable to have the current process to switch the pid namespace
>> because of the process tree hierarchy (what will be the parent of the
>> process when you enter the pid namespace for example).
>
>The answer is - the one, that used to be. I see no problems with it.
>Do you?
But perhaps it could be named "namespacefd" instead of nsfd, to reduce
potential clashes (because glibc will usually just use the same name
when making the syscall available as a C function).
^ permalink raw reply
* Re: Notifier chains bug ?
From: Américo Wang @ 2010-03-02 15:42 UTC (permalink / raw)
To: Oleg Kutkov; +Cc: Américo Wang, linux-kernel, netdev
In-Reply-To: <5e9821061003012150y15e543cdkf6f23c3a7a47e4a3@mail.gmail.com>
On Tue, Mar 02, 2010 at 07:50:01AM +0200, Oleg Kutkov wrote:
>2010/3/2 Américo Wang <xiyou.wangcong@gmail.com>:
>> On Tue, Mar 2, 2010 at 8:08 AM, Oleg Kutkov <elenbert@gmail.com> wrote:
>>> Hello.
>>> I try to used notifier chains for monitoring network devices events.
>>> All works perfectly when just i'm connecting/disconnecting network cable or
>>> up/down interface via ifconfig.
>>> But when i try to change interface address - nothing happens. Notifier is
>>> don't send any events :(
>>
>> I think you mean IP address? No, NETDEV_CHANGEADDR is for hardware
>> address, not for IP address.
>>
>> If you were changing mac address, you will receive NETDEV_CHANGEADDR.
>>
>
>Thank for quick answer.
>Yes, i mean IP address. And what about NETDEV_CHANGE ?
It is for rtnetlink state transition.
>Is there possible for monitoring IP address/netmask changing ?
>
AFAIK, no.
Cc'ing netdev experts...
^ permalink raw reply
* Re: Notifier chains bug ?
From: Atis Elsts @ 2010-03-02 15:57 UTC (permalink / raw)
To: Oleg Kutkov; +Cc: netdev
In-Reply-To: <20100302153651.GC4014@hack>
I'm no expert, but IMHO for handling IP address changes you should use
function register_inetaddr_notifier() to register you callback and
then wait for NETDEV_UP and NETDEV_DOWN events.
On Tue, Mar 2, 2010 at 5:42 PM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Mar 02, 2010 at 07:50:01AM +0200, Oleg Kutkov wrote:
>>2010/3/2 Américo Wang <xiyou.wangcong@gmail.com>:
>>> On Tue, Mar 2, 2010 at 8:08 AM, Oleg Kutkov <elenbert@gmail.com> wrote:
>>>> Hello.
>>>> I try to used notifier chains for monitoring network devices events.
>>>> All works perfectly when just i'm connecting/disconnecting network cable or
>>>> up/down interface via ifconfig.
>>>> But when i try to change interface address - nothing happens. Notifier is
>>>> don't send any events :(
>>>
>>> I think you mean IP address? No, NETDEV_CHANGEADDR is for hardware
>>> address, not for IP address.
>>>
>>> If you were changing mac address, you will receive NETDEV_CHANGEADDR.
>>>
>>
>>Thank for quick answer.
>>Yes, i mean IP address. And what about NETDEV_CHANGE ?
>
> It is for rtnetlink state transition.
>
>>Is there possible for monitoring IP address/netmask changing ?
>>
>
> AFAIK, no.
>
> Cc'ing netdev experts...
> --
> 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
>
--
~Atis
^ permalink raw reply
* Re: [PATCH 1/1] bnx2x: Tx barriers and locks
From: Michael Chan @ 2010-03-02 16:18 UTC (permalink / raw)
To: 'Stanislaw Gruszka', Vladislav Zolotarov
Cc: netdev@vger.kernel.org, davem@davemloft.net, Eilon Greenstein,
Matthew Carlson
In-Reply-To: <20100302135555.GC2362@dhcp-lab-161.englab.brq.redhat.com>
Stanislaw Gruszka wrote:
> On Tue, Mar 02, 2010 at 04:50:59AM -0800, Vladislav Zolotarov wrote:
> > Stanislaw barrier() is not a memory barrier - it's a
> compiler barrier. I don't think removing it from
> bnx2x_tx_avail() will improve anything. If u think I'm wrong,
> could u, pls., provide a specific example.
>
> Only improvement is removing confusing code, And comment like
> "Tell compiler that prod and cons can change" is even more
> confusing. If you think I'm wrong, just tell as why that
> barrier is needed :)
The barrier (compiler barrier at least) is required in
bnx2x_tx_avail(). The status block index can be updated by DMA and
the compiler doesn't know it (because it is considered wrong to
declare the status block as volatile). Near the end of
bnx2x_start_xmit() where we call bnx2x_tx_avail() twice. It is
possible that the compiler will optimize it and not look at the
status block in memory the second time.
^ permalink raw reply
* RE: [PATCH 1/1] bnx2x: Tx barriers and locks
From: Vladislav Zolotarov @ 2010-03-02 16:21 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Michael Chan, netdev@vger.kernel.org, davem@davemloft.net,
Eilon Greenstein, Matthew Carlson
In-Reply-To: <20100302135555.GC2362@dhcp-lab-161.englab.brq.redhat.com>
> -----Original Message-----
> From: Stanislaw Gruszka [mailto:sgruszka@redhat.com]
> Sent: Tuesday, March 02, 2010 3:56 PM
> To: Vladislav Zolotarov
> Cc: Michael Chan; netdev@vger.kernel.org;
> davem@davemloft.net; Eilon Greenstein; Matthew Carlson
> Subject: Re: [PATCH 1/1] bnx2x: Tx barriers and locks
>
> On Tue, Mar 02, 2010 at 04:50:59AM -0800, Vladislav Zolotarov wrote:
> > Stanislaw barrier() is not a memory barrier - it's a
> compiler barrier. I don't think removing it from
> bnx2x_tx_avail() will improve anything. If u think I'm wrong,
> could u, pls., provide a specific example.
>
> Only improvement is removing confusing code, And comment like
> "Tell compiler that prod and cons can change" is even more
> confusing. If you think I'm wrong, just tell as why that
> barrier is needed :)
Both the coment and the barrier() ensures that compiler doesn't optimize the reading of the consumer and producers outside the inline function. This is what was meant when the function has been written so I don't think that either the remark or a barrier() itself are confusing.
>
> For bnx2 and tg3 that would be removal of smp_mb().
>
> > Let's recall that the bxn2x_start_xmit() is called under
> tx_lock(), so no additional memory barriers needed to ensure
> that bnx2x_tx_avail() has fresh values of BD consumer and
> producer at the beginning of bnx2x_start_xmit().
>
> Memory barriers are not about refreshing anything, they are
> about ordering.
>
> > That's why we don't need the more complicated logic putting
> the queue asleep at the beginning of bnx2x_start_xmit() as we
> need at the end. That's why I think your "goto" change in the
> beginning of bnx2x_start_xmit() is suboptimal.
>
> You may have right, about that smb_mb() is not needed at the beginning
> of start_xmit(), but I'm not sure for that, since we are
> changing tx_bd_cons
> without netif_tx_lock and setting XOFF bit there. If we can
> assume that
> at the beginning of start_xmit() there is always enough space
> it hw queue,
> we can just remove tx_avail() check. Otherwise IMHO is better
> to put queue
> asleep in safe way.
The driver (bnx2x, Michael and Matthuew may elaborate on bnx2 and tg3 better) is written the way that is meant to ensure that there is always place on the ring at the beginning of bnx2x_start_xmit(). The check at the beginning ensures that nothing is broken. And if it's broken - it's a bug that should be fixed and we may not remove that check because if we do we will not even know that there was something wrong. But as long as there is nothing wrong - the current implementation of the tx_avail() check at the beginning of bnx2x_start_xmit() is absolutely safe as it is now.
>
> About performance, adding smp_mb() on bug/unused code path
> does not hurt
> much :)
It doesn't help either.... ;)
>
> Cheers
> Stanislaw
>
> > > -----Original Message-----
> > > From: Stanislaw Gruszka [mailto:sgruszka@redhat.com]
> > > Sent: Tuesday, March 02, 2010 1:31 PM
> > > To: Michael Chan
> > > Cc: Vladislav Zolotarov; netdev@vger.kernel.org;
> > > davem@davemloft.net; Eilon Greenstein; Matthew Carlson
> > > Subject: Re: [PATCH 1/1] bnx2x: Tx barriers and locks
> > >
> > > On Mon, Mar 01, 2010 at 09:59:07AM -0800, Michael Chan wrote:
> > > > > There is still difference between what we have in bnx2x
> > > and bnx2/tg3
> > > > > regarding memory barriers in tx_poll/start_xmit code.
> > > Mainly we have
> > > > > smp_mb() in bnx2/tg3_tx_avail(), and in bnx2/tg3_tx_int()
> > > is smp_mb()
> > > > > not smp_wmb(). I do not see that bnx2x is wrong, but
> > > would like to know
> > > > > why there is a difference, maybe bnx2/tg3 should be changed?
> > > > >
> > > >
> > > > The memory barrier in tx_int() is to make the tx index
> update happen
> > > > before the netif_tx_queue_stopped() check. The barrier is
> > > to prevent a
> > > > situation like this:
> > > >
> > > > CPU0 CPU1
> > > > start_xmit()
> > > > if (tx_ring_full) {
> > > > tx_int()
> > > >
> > > if (!netif_tx_queue_stopped)
> > > > netif_tx_stop_queue()
> > > > if (!tx_ring_full)
> > > >
> > > update_tx_index
> > > > netif_tx_wake_queue()
> > > > }
> > > >
> > > >
> > > > The update_tx_index code is before the if statement in
> > > program order,
> > > > but the CPU and/or compiler can reorder it as shown above.
> > > smp_mb() will
> > > > prevent that. Will smp_wmb() prevent that as well?
> > >
> > > No. smp_wmb() affect only write orders on CPU1 performing
> tx_int(), so
> > > that should be fixed in bnx2x.
> > >
> > > Regarding memory barrier in tx_avail(), I don't think it its
> > > needed for
> > > anything, except maybe usage at the beginning of
> > > start_xmit(), but we can
> > > just remove that like in the patch below. I going to post
> "official"
> > > patches for tg3, bnx2 and bnx2x, if no nobody has nothing against.
> > >
> > > diff --git a/drivers/net/bnx2x_main.c b/drivers/net/bnx2x_main.c
> > > index ed785a3..0f406b8 100644
> > > --- a/drivers/net/bnx2x_main.c
> > > +++ b/drivers/net/bnx2x_main.c
> > > @@ -893,7 +893,6 @@ static inline u16 bnx2x_tx_avail(struct
> > > bnx2x_fastpath *fp)
> > > u16 prod;
> > > u16 cons;
> > >
> > > - barrier(); /* Tell compiler that prod and cons can change */
> > > prod = fp->tx_bd_prod;
> > > cons = fp->tx_bd_cons;
> > >
> > > @@ -963,9 +962,8 @@ static int bnx2x_tx_int(struct
> bnx2x_fastpath *fp)
> > > * start_xmit() will miss it and cause the queue to be stopped
> > > * forever.
> > > */
> > > - smp_wmb();
> > > + smp_mb();
> > >
> > > - /* TBD need a thresh? */
> > > if (unlikely(netif_tx_queue_stopped(txq))) {
> > > /* Taking tx_lock() is needed to prevent
> > > reenabling the queue
> > > * while it's empty. This could have happen if
> > > rx_action() gets
> > > @@ -11177,10 +11175,9 @@ static netdev_tx_t
> > > bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
> > > struct eth_tx_bd *tx_data_bd, *total_pkt_bd = NULL;
> > > struct eth_tx_parse_bd *pbd = NULL;
> > > u16 pkt_prod, bd_prod;
> > > - int nbd, fp_index;
> > > + int nbd, fp_index, i, ret;
> > > dma_addr_t mapping;
> > > u32 xmit_type = bnx2x_xmit_type(bp, skb);
> > > - int i;
> > > u8 hlen = 0;
> > > __le16 pkt_size = 0;
> > >
> > > @@ -11195,10 +11192,9 @@ static netdev_tx_t
> > > bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
> > > fp = &bp->fp[fp_index];
> > >
> > > if (unlikely(bnx2x_tx_avail(fp) <
> > > (skb_shinfo(skb)->nr_frags + 3))) {
> > > - fp->eth_q_stats.driver_xoff++;
> > > - netif_tx_stop_queue(txq);
> > > BNX2X_ERR("BUG! Tx ring full when queue awake!\n");
> > > - return NETDEV_TX_BUSY;
> > > + ret = NETDEV_TX_BUSY;
> > >
> > > + goto stop_queue;
> > > }
> > >
> > > DP(NETIF_MSG_TX_QUEUED, "SKB: summed %x protocol %x
> > > protocol(%x,%x)"
> > > @@ -11426,19 +11422,24 @@ static netdev_tx_t
> > > bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
> > > mmiowb();
> > >
> > > fp->tx_bd_prod += nbd;
> > > -
> > > - if (unlikely(bnx2x_tx_avail(fp) < MAX_SKB_FRAGS + 3)) {
> > > - netif_tx_stop_queue(txq);
> > > - /* We want bnx2x_tx_int to "see" the updated tx_bd_prod
> > > - if we put Tx into XOFF state. */
> > > - smp_mb();
> > > - fp->eth_q_stats.driver_xoff++;
> > > - if (bnx2x_tx_avail(fp) >= MAX_SKB_FRAGS + 3)
> > > - netif_tx_wake_queue(txq);
> > > - }
> > > fp->tx_pkt++;
> > > +
> > > + ret = NETDEV_TX_OK;
> > > + if (unlikely(bnx2x_tx_avail(fp) < MAX_SKB_FRAGS + 3))
> > > + goto stop_queue;
> > > +
> > > + return ret;
> > >
> > > - return NETDEV_TX_OK;
> > > +stop_queue:
> > > + netif_tx_stop_queue(txq);
> > > + /* paired barrier is in bnx2x_tx_int(), update of tx_bd_cons
> > > + * have to be visable here, after we XOFF bit setting */
> > > + smp_mb();
> > > + fp->eth_q_stats.driver_xoff++;
> > > + if (bnx2x_tx_avail(fp) >= MAX_SKB_FRAGS + 3)
> > > + netif_tx_wake_queue(txq);
> > > +
> > > + return ret;
> > > }
> > >
> > > /* called with rtnl_lock */
> > >
> > >
>
>
^ permalink raw reply
* Re: Yahoo/Msn Ticket Number 3XTA932CL9
From: Yahoo/Msn© @ 2010-03-02 16:26 UTC (permalink / raw)
To: info
£1,000,000.00 has been awarded to you in the Yahoo/Msn Grant Award,send us your Names,Tel,Country:
^ 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