* [PATCH 2/6] netfilter: nft_meta: fix typo "CONFIG_NET_CLS_ROUTE"
From: Pablo Neira Ayuso @ 2014-02-19 11:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1392810102-13877-1-git-send-email-pablo@netfilter.org>
From: Paul Bolle <pebolle@tiscali.nl>
There are two checks for CONFIG_NET_CLS_ROUTE, but the corresponding
Kconfig symbol was dropped in v2.6.39. Since the code guards access to
dst_entry.tclassid it seems CONFIG_IP_ROUTE_CLASSID should be used
instead.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nft_meta.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index e8254ad..425cf39 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -116,7 +116,7 @@ static void nft_meta_get_eval(const struct nft_expr *expr,
skb->sk->sk_socket->file->f_cred->fsgid);
read_unlock_bh(&skb->sk->sk_callback_lock);
break;
-#ifdef CONFIG_NET_CLS_ROUTE
+#ifdef CONFIG_IP_ROUTE_CLASSID
case NFT_META_RTCLASSID: {
const struct dst_entry *dst = skb_dst(skb);
@@ -199,7 +199,7 @@ static int nft_meta_init_validate_get(uint32_t key)
case NFT_META_OIFTYPE:
case NFT_META_SKUID:
case NFT_META_SKGID:
-#ifdef CONFIG_NET_CLS_ROUTE
+#ifdef CONFIG_IP_ROUTE_CLASSID
case NFT_META_RTCLASSID:
#endif
#ifdef CONFIG_NETWORK_SECMARK
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/6] netfilter: nf_nat_snmp_basic: fix duplicates in if/else branches
From: Pablo Neira Ayuso @ 2014-02-19 11:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1392810102-13877-1-git-send-email-pablo@netfilter.org>
From: FX Le Bail <fx.lebail@yahoo.com>
The solution was found by Patrick in 2.4 kernel sources.
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
Acked-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/ipv4/netfilter/nf_nat_snmp_basic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/netfilter/nf_nat_snmp_basic.c b/net/ipv4/netfilter/nf_nat_snmp_basic.c
index d551e31..7c67667 100644
--- a/net/ipv4/netfilter/nf_nat_snmp_basic.c
+++ b/net/ipv4/netfilter/nf_nat_snmp_basic.c
@@ -1198,8 +1198,8 @@ static int snmp_translate(struct nf_conn *ct,
map.to = NOCT1(&ct->tuplehash[!dir].tuple.dst.u3.ip);
} else {
/* DNAT replies */
- map.from = NOCT1(&ct->tuplehash[dir].tuple.src.u3.ip);
- map.to = NOCT1(&ct->tuplehash[!dir].tuple.dst.u3.ip);
+ map.from = NOCT1(&ct->tuplehash[!dir].tuple.src.u3.ip);
+ map.to = NOCT1(&ct->tuplehash[dir].tuple.dst.u3.ip);
}
if (map.from == map.to)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/6] netfilter: nft_reject_inet: fix unintended fall-through in switch-statatement
From: Pablo Neira Ayuso @ 2014-02-19 11:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1392810102-13877-1-git-send-email-pablo@netfilter.org>
From: Patrick McHardy <kaber@trash.net>
For IPv4 packets, we call both IPv4 and IPv6 reject.
Reported-by: Dave Jones <davej@redhat.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nft_reject_inet.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nft_reject_inet.c b/net/netfilter/nft_reject_inet.c
index 8a310f2..b718a52 100644
--- a/net/netfilter/nft_reject_inet.c
+++ b/net/netfilter/nft_reject_inet.c
@@ -21,9 +21,9 @@ static void nft_reject_inet_eval(const struct nft_expr *expr,
{
switch (pkt->ops->pf) {
case NFPROTO_IPV4:
- nft_reject_ipv4_eval(expr, data, pkt);
+ return nft_reject_ipv4_eval(expr, data, pkt);
case NFPROTO_IPV6:
- nft_reject_ipv6_eval(expr, data, pkt);
+ return nft_reject_ipv6_eval(expr, data, pkt);
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/6] Netfilter fixes for net
From: Pablo Neira Ayuso @ 2014-02-19 11:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
Hi David,
The following patchset contains Netfilter fixes for your net tree,
they are:
* Fix nf_trace in nftables if XT_TRACE=n, from Florian Westphal.
* Don't use the fast payload operation in nf_tables if the length is
not power of 2 or it is not aligned, from Nikolay Aleksandrov.
* Fix missing break statement the inet flavour of nft_reject, which
results in evaluating IPv4 packets with the IPv6 evaluation routine,
from Patrick McHardy.
* Fix wrong kconfig symbol in nft_meta to match the routing realm,
from Paul Bolle.
* Allocate the NAT null binding when creating new conntracks via
ctnetlink to avoid that several packets race at initializing the
the conntrack NAT extension, original patch from Florian Westphal,
revisited version from me.
* Fix DNAT handling in the snmp NAT helper, the same handling was being
done for SNAT and DNAT and 2.4 already contains that fix, from
Francois-Xavier Le Bail.
You can pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
Thanks!
----------------------------------------------------------------
The following changes since commit 20e7c4e80dcd01dad5e6c8b32455228b8fe9c619:
6lowpan: fix lockdep splats (2014-02-10 17:51:29 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
for you to fetch changes up to 0eba801b64cc8284d9024c7ece30415a2b981a72:
netfilter: ctnetlink: force null nat binding on insert (2014-02-18 00:13:51 +0100)
----------------------------------------------------------------
FX Le Bail (1):
netfilter: nf_nat_snmp_basic: fix duplicates in if/else branches
Florian Westphal (1):
netfilter: nf_tables: fix nf_trace always-on with XT_TRACE=n
Nikolay Aleksandrov (1):
netfilter: nf_tables: check if payload length is a power of 2
Pablo Neira Ayuso (1):
netfilter: ctnetlink: force null nat binding on insert
Patrick McHardy (1):
netfilter: nft_reject_inet: fix unintended fall-through in switch-statatement
Paul Bolle (1):
netfilter: nft_meta: fix typo "CONFIG_NET_CLS_ROUTE"
include/linux/skbuff.h | 5 ++-
net/core/skbuff.c | 3 --
net/ipv4/ip_output.c | 3 --
net/ipv4/netfilter/nf_nat_snmp_basic.c | 4 +--
net/ipv6/ip6_output.c | 3 --
net/netfilter/nf_conntrack_netlink.c | 35 ++++++++------------
net/netfilter/nf_nat_core.c | 56 ++++++++++++++++++++------------
net/netfilter/nft_meta.c | 4 +--
net/netfilter/nft_payload.c | 3 +-
net/netfilter/nft_reject_inet.c | 4 +--
10 files changed, 61 insertions(+), 59 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/1] net: tcp: RTO restart
From: Per Hurtig @ 2014-02-19 10:50 UTC (permalink / raw)
To: Yuchung Cheng
Cc: netdev, David Miller, Eric Dumazet, Anna Brunstrom,
Andreas Petlund, Michael Welzl, Ilpo Järvinen
In-Reply-To: <CAK6E8=ekVXA9JrajwmJk88bUKcJSi=T5+SFXs0VoG5iQkWBcRw@mail.gmail.com>
Hi Yuchung,
see inline
On Tue, Feb 18, 2014 at 10:46:18AM -0800, Yuchung Cheng wrote:
> On Tue, Feb 18, 2014 at 10:12 AM, Per Hurtig <per.hurtig@kau.se> wrote:
> > This patch implements the RTO restart modification described in
> > http://tools.ietf.org/html/draft-ietf-tcpm-rtorestart-02
> >
> > RTO Restart's goal is to provide quicker loss recovery for segments lost in the
> > end of a burst/connection. To accomplish this the algorithm adjusts the RTO
> > value on each rearm of the retransmission timer to be exactly RTO ms after the
> > earliest outstanding segment was sent. The offsetting against the earliest
> > outstanding segment is not done by the regular rearm algorithm, which causes
> > RTOs to occur, on average, after RTO+RTT ms.
> >
> > As a faster timeout is only beneficial in scenarios where fast retransmit/early
> > retransmit cannot be triggered the algorithm will only kick in when there is a
> > small amount of outstanding segments.
> (repost in plaine-text, sorry for the duplication)
>
> I am not sure this works well with Linux min-RTO=200ms, and don't feel
> comfortable this is default on without some real experiments.
>
> I've implemented (a nearly identical version of) rto-restart on Google
> web servers and found #timeouts increased by ~30%. Although the fast
> timeout helps really short flows, it has a very negative side-effect:
> resetting cwnd to 1. Thus the next HTTP response may start with a
> small cwnd. In contrast, TCP loss probe has fast timeout (1.5RTT) to
> do fast recovery to avoid the side-effect. In other words, I am
> doubtful we need rto-restart with TCP loss probe, but applying
> RTO-restart on TLP timer may be useful.
As we have discussed, I agree that RTO restart can also be applied to TLP and
this is also mentioned in the ietf draft. However, I think they are also
complementary in reducing loss recovery delay.
>
> I've voiced this concern multiple times in ietf tcpm discussion when
> we discuss this draft: that the idea itself is fine, but we'll need to
> change Linux RTO algorithm together, not just the timer itself.
>
> I am happy to post some more detailed data if people are interested.
We would love to see the data. The last time we discussed this, you were not
able to find it. From the latest discussions in the tcpm group I understood you
didn't think it was that big of a problem anymore.
Best Regards,
Per Hurtig
>
> >
> > The RTO Restart proposal is accepted as a working group item in the IETF TCP
> > Maintenance and Minor Extensions (tcpm) TCP wg and is intended for experimental
> > RFC status.
> >
> > Signed-off-by: Per Hurtig <per.hurtig@kau.se>
> > ---
> > include/net/tcp.h | 1 +
> > net/ipv4/sysctl_net_ipv4.c | 7 +++++++
> > net/ipv4/tcp_input.c | 11 +++++++++++
> > 3 files changed, 19 insertions(+)
> >
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index 56fc366..575e82a 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -278,6 +278,7 @@ extern int sysctl_tcp_slow_start_after_idle;
> > extern int sysctl_tcp_thin_linear_timeouts;
> > extern int sysctl_tcp_thin_dupack;
> > extern int sysctl_tcp_early_retrans;
> > +extern int sysctl_tcp_rto_restart;
> > extern int sysctl_tcp_limit_output_bytes;
> > extern int sysctl_tcp_challenge_ack_limit;
> > extern unsigned int sysctl_tcp_notsent_lowat;
> > diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
> > index 44eba05..c605f4f 100644
> > --- a/net/ipv4/sysctl_net_ipv4.c
> > +++ b/net/ipv4/sysctl_net_ipv4.c
> > @@ -717,6 +717,13 @@ static struct ctl_table ipv4_table[] = {
> > .extra2 = &four,
> > },
> > {
> > + .procname = "tcp_rto_restart",
> > + .data = &sysctl_tcp_rto_restart,
> > + .maxlen = sizeof(int),
> > + .mode = 0644,
> > + .proc_handler = proc_dointvec,
> > + },
> > + {
> > .procname = "tcp_min_tso_segs",
> > .data = &sysctl_tcp_min_tso_segs,
> > .maxlen = sizeof(int),
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index 227cba7..450ee30 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -98,6 +98,7 @@ int sysctl_tcp_thin_dupack __read_mostly;
> >
> > int sysctl_tcp_moderate_rcvbuf __read_mostly = 1;
> > int sysctl_tcp_early_retrans __read_mostly = 3;
> > +int sysctl_tcp_rto_restart __read_mostly = 1;
> >
> > #define FLAG_DATA 0x01 /* Incoming frame contained data. */
> > #define FLAG_WIN_UPDATE 0x02 /* Incoming ACK was a window update. */
> > @@ -2972,6 +2973,16 @@ void tcp_rearm_rto(struct sock *sk)
> > */
> > if (delta > 0)
> > rto = delta;
> > + } else if (icsk->icsk_pending == ICSK_TIME_RETRANS &&
> > + sysctl_tcp_rto_restart &&
> > + sk->sk_send_head == NULL &&
> > + tp->packets_out < 4) {
> > + struct sk_buff *skb = tcp_write_queue_head(sk);
> > + const u32 rto_time_stamp = TCP_SKB_CB(skb)->when;
> > + s32 delta = (s32)(tcp_time_stamp - rto_time_stamp);
> > +
> > + if (delta > 0)
> > + rto -= delta;
> > }
> > inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, rto,
> > TCP_RTO_MAX);
> > --
> > 1.7.9.5
> >
> > --
> > 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
> --
> 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
^ permalink raw reply
* Re: [PATCH net v2 0/2] Fix RTNL: assertion failed at net/core/rtnetlink.c
From: Thomas Glanzmann @ 2014-02-19 10:52 UTC (permalink / raw)
To: Ding Tianhong
Cc: fubar, vfalico, andy, cwang, jiri, eric.dumazet, sfeldma, davem,
netdev
In-Reply-To: <1392800711-19264-1-git-send-email-dingtianhong@huawei.com>
Hello Ding,
> v1->v2: Add new micro to indicate that the notification should be send
> later, not never.
> And add a new patch to fix the same problem for ARP mode.
> Ding Tianhong (2):
> bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for
> 802.3ad mode
> bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for ab
> arp monitor
the patch did not apply cleanly for me, so I manually merged it.
The resulting patch is here:
(node-62) [~/work/linux-2.6] git diff | pbot
http://pbot.rmdir.de/TDMvhGB775BVPpKvm6NOXg
Find the results here:
(node-62) [~] dmesg | pbot
http://pbot.rmdir.de/nogPmqfGRxxk9zMNBlr0QQ
(node-62) [~] pbot /proc/net/bonding/bond0
http://pbot.rmdir.de/zNSSqmjSI0o1Qvt6DrSEQw
(node-62) [~] pbot /proc/net/bonding/bond1
http://pbot.rmdir.de/CoI00Rguie2P-kOQytOM9w
Tested-by: Thomas Glanzmann <thomas@glanzmann.de>
Cheers,
Thomas
^ permalink raw reply
* Re: [PATCH net-next] ipcomp: Convert struct xt_ipcomp spis into 16bits
From: Pablo Neira Ayuso @ 2014-02-19 10:14 UTC (permalink / raw)
To: Fan Du; +Cc: steffen.klassert, davem, netdev, netfilter-devel
In-Reply-To: <52DC821E.3000309@windriver.com>
On Mon, Jan 20, 2014 at 09:55:42AM +0800, Fan Du wrote:
> On 2014年01月18日 20:24, Pablo Neira Ayuso wrote:
> >Didn't you find any way to fix this without change the layout of
> >xt_ipcomp?
>
> My bad for not catching this in the initial ipcomp version.
> It would be easier to just use ntohs here to ease sparse checking.
>
>
> From dbf796e63d600256fd40000669b83227ddccebc4 Mon Sep 17 00:00:00 2001
> From: Fan Du <fan.du@windriver.com>
> Date: Mon, 20 Jan 2014 09:42:12 +0800
> Subject: [PATCHv2 net-next] ipcomp: Use ntohs to ease sparse warning
>
> 0-DAY kernel build testing backend reported:
>
> sparse warnings: (new ones prefixed by >>)
>
> >> >> net/netfilter/xt_ipcomp.c:63:26: sparse: restricted __be16 degrades to integer
> >> >> net/netfilter/xt_ipcomp.c:63:26: sparse: cast to restricted __be32
>
> Fix this by using ntohs without shifting.
>
> Tested with: make C=1 CF=-D__CHECK_ENDIAN__
Applied to nf-next, thanks.
^ permalink raw reply
* Re: [PATCH net-next] netfilter: remove double colon
From: Pablo Neira Ayuso @ 2014-02-19 10:08 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, netdev, netfilter-devel
In-Reply-To: <20140115081250.56958f9a@nehalam.linuxnetplumber.net>
On Wed, Jan 15, 2014 at 08:12:50AM -0800, Stephen Hemminger wrote:
> This is C not shell script
This one didn't reach in time to the previous merge window.
Now applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next v5 1/9] xen-netback: Introduce TX grant map definitions
From: Ian Campbell @ 2014-02-19 10:05 UTC (permalink / raw)
To: Zoltan Kiss; +Cc: wei.liu2, xen-devel, netdev, linux-kernel, jonathan.davies
In-Reply-To: <5303C44D.4070500@citrix.com>
On Tue, 2014-02-18 at 20:36 +0000, Zoltan Kiss wrote:
> On 18/02/14 17:06, Ian Campbell wrote:
> > On Mon, 2014-01-20 at 21:24 +0000, Zoltan Kiss wrote:
> >> This patch contains the new definitions necessary for grant mapping.
> >
> > Is this just adding a bunch of (currently) unused functions? That's a
> > slightly odd way to structure a series. They don't seem to be "generic
> > helpers" or anything so it would be more normal to introduce these as
> > they get used -- it's a bit hard to review them out of context.
> I've created two patches because they are quite huge even now,
> separately. Together they would be a ~500 line change. That was the best
> I could figure out keeping in mind that bisect should work. But as I
> wrote in the first email, I welcome other suggestions. If you and Wei
> prefer this two patch in one big one, I merge them in the next version.
I suppose it is hard to split a change like this up in a sensible way,
but it is rather hard to review something which is split in two parts
sensibly.
If the combined patch too large to fit on the lists?
> >> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> >> index 7669d49..f0f0c3d 100644
> >> --- a/drivers/net/xen-netback/interface.c
> >> +++ b/drivers/net/xen-netback/interface.c
> >> @@ -38,6 +38,7 @@
> >>
> >> #include <xen/events.h>
> >> #include <asm/xen/hypercall.h>
> >> +#include <xen/balloon.h>
> >
> > What is this for?
> For alloc/free_xenballooned_pages
I think I was confused because those changes aren't in this patch.
> >
> >> + struct xenvif,
> >> + pending_tx_info[0]);
> >> +
> >> + spin_lock_irqsave(&vif->dealloc_lock, flags);
> >> + do {
> >> + pending_idx = ubuf->desc;
> >> + ubuf = (struct ubuf_info *) ubuf->ctx;
> >> + index = pending_index(vif->dealloc_prod);
> >> + vif->dealloc_ring[index] = pending_idx;
> >> + /* Sync with xenvif_tx_dealloc_action:
> >> + * insert idx then incr producer.
> >> + */
> >> + smp_wmb();
> >
> > Is this really needed given that there is a lock held?
> Yes, as the comment right above explains.
My question is why do you need this sync if you are holding a lock, the
comment doesn't tell me that. I suppose xenvif_tx_dealloc_action doesn't
hold the dealloc_lock, but that is non-obvious from the names.
I think I asked in a subsequent patch for an improved description of the
locking going on here.
> This actually comes from
> classic kernel's netif_idx_release
> >
> > Or what is dealloc_lock protecting against?
> The callbacks from each other. So it is checjed only in this function.
> >
> >> + vif->dealloc_prod++;
> >
> > What happens if the dealloc ring becomes full, will this wrap and cause
> > havoc?
> Nope, if the dealloc ring is full, the value of the last increment won't
> be used to index the dealloc ring again until some space made available.
I don't follow -- what makes this the case?
> Of course if something broke and we have more pending slots than tx ring
> or dealloc slots then it can happen. Do you suggest a
> BUG_ON(vif->dealloc_prod - vif->dealloc_cons >= MAX_PENDING_REQS)?
A
BUG_ON(space in dealloc ring < number of slots needed to dealloc this skb)
would seem to be the right thing, if that really is the invariant the
code is supposed to be implementing.
> >> + } while (ubuf);
> >> + wake_up(&vif->dealloc_wq);
> >> + spin_unlock_irqrestore(&vif->dealloc_lock, flags);
> >> +}
> >> +
> >> +static inline void xenvif_tx_dealloc_action(struct xenvif *vif)
> >> +{
> >> + struct gnttab_unmap_grant_ref *gop;
> >> + pending_ring_idx_t dc, dp;
> >> + u16 pending_idx, pending_idx_release[MAX_PENDING_REQS];
> >> + unsigned int i = 0;
> >> +
> >> + dc = vif->dealloc_cons;
> >> + gop = vif->tx_unmap_ops;
> >> +
> >> + /* Free up any grants we have finished using */
> >> + do {
> >> + dp = vif->dealloc_prod;
> >> +
> >> + /* Ensure we see all indices enqueued by all
> >> + * xenvif_zerocopy_callback().
> >> + */
> >> + smp_rmb();
> >> +
> >> + while (dc != dp) {
> >> + pending_idx =
> >> + vif->dealloc_ring[pending_index(dc++)];
> >> +
> >> + /* Already unmapped? */
> >> + if (vif->grant_tx_handle[pending_idx] ==
> >> + NETBACK_INVALID_HANDLE) {
> >> + netdev_err(vif->dev,
> >> + "Trying to unmap invalid handle! "
> >> + "pending_idx: %x\n", pending_idx);
> >> + BUG();
> >> + }
> >> +
> >> + pending_idx_release[gop-vif->tx_unmap_ops] =
> >> + pending_idx;
> >> + vif->pages_to_unmap[gop-vif->tx_unmap_ops] =
> >> + vif->mmap_pages[pending_idx];
> >> + gnttab_set_unmap_op(gop,
> >> + idx_to_kaddr(vif, pending_idx),
> >> + GNTMAP_host_map,
> >> + vif->grant_tx_handle[pending_idx]);
> >> + vif->grant_tx_handle[pending_idx] =
> >> + NETBACK_INVALID_HANDLE;
> >> + ++gop;
> >
> > Can we run out of space in the gop array?
> No, unless the same thing happen as at my previous answer. BUG_ON() here
> as well?
Yes, or at the very least a comment explaining how/why gop is bounded
elsewhere.
> >
> >> + }
> >> +
> >> + } while (dp != vif->dealloc_prod);
> >> +
> >> + vif->dealloc_cons = dc;
> >
> > No barrier here?
> dealloc_cons only used in the dealloc_thread. dealloc_prod is used by
> the callback and the thread as well, that's why we need mb() in
> previous. Btw. this function comes from classic's net_tx_action_dealloc
Is this code close enough to that code architecturally that you can
infer correctness due to that though?
So long as you have considered the barrier semantics in the context of
the current code and you think it is correct to not have one here then
I'm ok. But if you have just assumed it is OK because some older code
didn't have it then I'll have to ask you to consider it again...
> >> + netdev_err(vif->dev,
> >> + " host_addr: %llx handle: %x status: %d\n",
> >> + gop[i].host_addr,
> >> + gop[i].handle,
> >> + gop[i].status);
> >> + }
> >> + BUG();
> >> + }
> >> + }
> >> +
> >> + for (i = 0; i < gop - vif->tx_unmap_ops; ++i)
> >> + xenvif_idx_release(vif, pending_idx_release[i],
> >> + XEN_NETIF_RSP_OKAY);
> >> +}
> >> +
> >> +
> >> /* Called after netfront has transmitted */
> >> int xenvif_tx_action(struct xenvif *vif, int budget)
> >> {
> >> @@ -1678,6 +1793,25 @@ static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
> >> vif->mmap_pages[pending_idx] = NULL;
> >> }
> >>
> >> +void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx)
> >
> > This is a single shot version of the batched xenvif_tx_dealloc_action
> > version? Why not just enqueue the idx to be unmapped later?
> This is called only from the NAPI instance. Using the dealloc ring
> require synchronization with the callback which can increase lock
> contention. On the other hand, if the guest sends small packets
> (<PAGE_SIZE), the TLB flushing can cause performance penalty.
Right. When/How often is this called from the NAPI instance?
Is the locking contention from this case so severe that it out weighs
the benefits of batching the unmaps? That would surprise me. After all
the locking contention is there for the zerocopy_callback case too
> The above
> mentioned upcoming patch which gntcopy the header can prevent that
So this is only called when doing the pull-up to the linear area?
Ian.
^ permalink raw reply
* Re: [Xen-devel] [PATCH net-next v5 2/9] xen-netback: Change TX path from grant copy to mapping
From: Ian Campbell @ 2014-02-19 9:54 UTC (permalink / raw)
To: David Vrabel
Cc: Zoltan Kiss, xen-devel, jonathan.davies, wei.liu2, linux-kernel,
netdev
In-Reply-To: <5303AA97.3010202@citrix.com>
On Tue, 2014-02-18 at 18:46 +0000, David Vrabel wrote:
> On 18/02/14 17:40, Ian Campbell wrote:
> > On Mon, 2014-01-20 at 21:24 +0000, Zoltan Kiss wrote:
> >>
> >> @@ -344,8 +346,26 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
> >> vif->pending_prod = MAX_PENDING_REQS;
> >> for (i = 0; i < MAX_PENDING_REQS; i++)
> >> vif->pending_ring[i] = i;
> >> - for (i = 0; i < MAX_PENDING_REQS; i++)
> >> - vif->mmap_pages[i] = NULL;
> >> + spin_lock_init(&vif->dealloc_lock);
> >> + spin_lock_init(&vif->response_lock);
> >> + /* If ballooning is disabled, this will consume real memory, so you
> >> + * better enable it.
> >
> > Almost no one who would be affected by this is going to read this
> > comment. And it doesn't just require enabling ballooning, but actually
> > booting with some maxmem "slack" to leave space.
> >
> > Classic-xen kernels used to add 8M of slop to the physical address space
> > to leave a suitable pool for exactly this sort of thing. I never liked
> > that but perhaps it should be reconsidered (or at least raised as a
> > possibility with the core-Xen Linux guys).
>
> I plan to fix the balloon memory hotplug stuff to do the right thing
Which is for alloc_xenballoon_pages to hotplug a new empty region,
rather than inflating the balloon if it doesn't have enough pages to
satisfy the allocation? Or something else?
> (it's almost there -- it just tries to overlap the new memory with
> existing stuff).
>
> David
^ permalink raw reply
* Re: [Xen-devel] [RFC v2 1/4] bridge: enable interfaces to opt out from becoming the root bridge
From: Ian Campbell @ 2014-02-19 9:52 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: kvm, netdev@vger.kernel.org, bridge, linux-kernel@vger.kernel.org,
Stephen Hemminger, xen-devel
In-Reply-To: <CAB=NE6WwbOXNLO_Mn42i7y62pEsmDOWd35B621eiJk4iaE3Hfg@mail.gmail.com>
On Tue, 2014-02-18 at 13:02 -0800, Luis R. Rodriguez wrote:
> On Sun, Feb 16, 2014 at 10:57 AM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > On Fri, 14 Feb 2014 18:59:37 -0800
> > "Luis R. Rodriguez" <mcgrof@do-not-panic.com> wrote:
> >
> >> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >>
> >> It doesn't make sense for some interfaces to become a root bridge
> >> at any point in time. One example is virtual backend interfaces
> >> which rely on other entities on the bridge for actual physical
> >> connectivity. They only provide virtual access.
> >>
> >> Device drivers that know they should never become part of the
> >> root bridge have been using a trick of setting their MAC address
> >> to a high broadcast MAC address such as FE:FF:FF:FF:FF:FF. Instead
> >> of using these hacks lets the interfaces annotate its intent and
> >> generalizes a solution for multiple drivers, while letting the
> >> drivers use a random MAC address or one prefixed with a proper OUI.
> >> This sort of hack is used by both qemu and xen for their backend
> >> interfaces.
> >>
> >> Cc: Stephen Hemminger <stephen@networkplumber.org>
> >> Cc: bridge@lists.linux-foundation.org
> >> Cc: netdev@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> >
> > This is already supported in a more standard way via the root
> > block flag.
>
> Great! For documentation purposes the root_block flag is a sysfs
> attribute, added via 3.8 through commit 1007dd1a. The respective
> interface flag is IFLA_BRPORT_PROTECT and can be set via the iproute2
> bridge utility or through sysfs:
>
> mcgrof@garbanzo ~/linux (git::master)$ find /sys/ -name root_block
> /sys/devices/pci0000:00/0000:00:04.0/0000:02:00.0/net/eth0/brport/root_block
> /sys/devices/vif-3-0/net/vif3.0/brport/root_block
> /sys/devices/virtual/net/vif3.0-emu/brport/root_block
>
> mcgrof@garbanzo ~/devel/iproute2 (git::master)$ cat
> /sys/devices/vif-3-0/net/vif3.0/brport/root_block
> 0
> mcgrof@garbanzo ~/devel/iproute2 (git::master)$ sudo bridge link set
> dev vif3.0 root_block on
> mcgrof@garbanzo ~/devel/iproute2 (git::master)$ cat
> /sys/devices/vif-3-0/net/vif3.0/brport/root_block
> 1
>
> So if we'd want to avoid using the MAC address hack alternative to
> skip a root port userspace would need to be updated to simply set this
> attribute after adding the device to the bridge. Based on Zoltan's
> feedback there seems to be use cases to not enable this always for all
> xen-netback interfaces though as such we can just punt this to
> userspace for the topologies that require this.
>
> The original motivation for this series was to avoid the IPv6
> duplicate address incurred by the MAC address hack for avoiding the
> root bridge. Given that Zoltan also noted a use case whereby IPv4 and
> IPv6 addresses can be assigned to the backend interfaces we should be
> able to avoid the duplicate address situation for IPv6 by using a
> proper random MAC address *once* userspace has been updated also to
> use IFLA_BRPORT_PROTECT. New userspace can't and won't need to set
> this flag for older kernels (older than 3.8) as root_block is not
> implemented on those kernels and the MAC address hack would still be
> used there. This strategy however does put a requirement on new
> kernels to use new userspace as otherwise the MAC address workaround
> would not be in place and root_block would not take effect.
Can't we arrange things in the Xen hotplug scripts such that if the
root_block stuff isn't available/doesn't work we fallback to the
existing fe:ff:ff:ff:ff usage?
That would avoid concerns about forward/backwards compat I think. It
wouldn't solve the issue you are targeting on old systems, but it also
doesn't regress them any further.
Ian.
^ permalink raw reply
* Re: [PATCH net-next v5 0/9] xen-netback: TX grant mapping with SKBTX_DEV_ZEROCOPY instead of copy
From: Ian Campbell @ 2014-02-19 9:50 UTC (permalink / raw)
To: Zoltan Kiss; +Cc: wei.liu2, xen-devel, netdev, linux-kernel, jonathan.davies
In-Reply-To: <1390253069-25507-1-git-send-email-zoltan.kiss@citrix.com>
On Mon, 2014-01-20 at 21:24 +0000, Zoltan Kiss wrote:
> A long known problem of the upstream netback implementation that on the TX
> path (from guest to Dom0) it copies the whole packet from guest memory into
> Dom0. That simply became a bottleneck with 10Gb NICs, and generally it's a
> huge perfomance penalty. The classic kernel version of netback used grant
> mapping, and to get notified when the page can be unmapped, it used page
> destructors. Unfortunately that destructor is not an upstreamable solution.
> Ian Campbell's skb fragment destructor patch series [1] tried to solve this
> problem, however it seems to be very invasive on the network stack's code,
> and therefore haven't progressed very well.
> This patch series use SKBTX_DEV_ZEROCOPY flags to tell the stack it needs to
> know when the skb is freed up. That is the way KVM solved the same problem,
> and based on my initial tests it can do the same for us. Avoiding the extra
> copy boosted up TX throughput from 6.8 Gbps to 7.9 (I used a slower
> Interlagos box, both Dom0 and guest on upstream kernel, on the same NUMA node,
> running iperf 2.0.5, and the remote end was a bare metal box on the same 10Gb
> switch)
> Based on my investigations the packet get only copied if it is delivered to
> Dom0 stack,
This is not quite complete/accurate since you previously told me that it
is copied in the NAT/routed rather than bridged network topologies.
Please can you cover that aspect here too.
Ian.
^ permalink raw reply
* Re: [Xen-devel] [RFC v2 0/4] net: bridge / ip optimizations for virtual net backends
From: Ian Campbell @ 2014-02-19 9:48 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: David Vrabel, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kvm, xen-devel
In-Reply-To: <CAB=NE6VxNByeWGk6_Ow7WgxA3HCwGBjrjL9MNVRGsEfFyeKTdw@mail.gmail.com>
On Tue, 2014-02-18 at 11:43 -0800, Luis R. Rodriguez wrote:
>
> New motivation: removing IPv4 and IPv6 from the backend interfaces can
> save up a lot of boiler plate run time code, triggers from ever taking
> place, and simplifying the backend interaces. If there is no use for
> IPv4 and IPv6 interfaces why do we have them? Note: I have yet to test
> the NAT case.
I think you need to do that test that before you can unequivocally state
that there is no use for IPv4/6 interfaces here.
Ian.
^ permalink raw reply
* Re: [Xen-devel] [RFC v2 4/4] xen-netback: skip IPv4 and IPv6 interfaces
From: Ian Campbell @ 2014-02-19 9:47 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Zoltan Kiss, netdev@vger.kernel.org, Wei Liu, kvm,
linux-kernel@vger.kernel.org, Paul Durrant, xen-devel
In-Reply-To: <CAB=NE6WwPfPi-8Yudxs4-OA2LPOjo8-XxUiVbtu6=BQ8FhEEOA@mail.gmail.com>
On Tue, 2014-02-18 at 12:16 -0800, Luis R. Rodriguez wrote:
> On Mon, Feb 17, 2014 at 6:36 AM, Zoltan Kiss <zoltan.kiss@citrix.com> wrote:
> > Also, the backend is not necessarily Dom0, you can connect two guests with
> > backend/frontend pairs.
>
> Can you elaborate a bit more on this type of setup?
The domain providing backend networking services is not necessarily
dom0, it might be a driver domain:
http://wiki.xen.org/wiki/Driver_Domain
I think from your PoV here it probably doesn't matter whether the driver
domain is dom0 or some other domain.
Ian.
^ permalink raw reply
* [PATCH net-next] xfrm: Correctly parse netlink msg from 32bits ip command on 64bits host
From: Fan Du @ 2014-02-19 9:12 UTC (permalink / raw)
To: steffen.klassert; +Cc: davem, netdev
When trying to setup IPsec configuration on a 64bits host with
iproute2(32bits compiled), the intended xfrm policy and sa is
either deficit or wrong when kernel trying to parse user land
information.
Add compat support for 32bits compiled ip command for 64bits host.
Signed-off-by: Fan Du <fan.du@windriver.com>
---
include/uapi/linux/xfrm.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
net/xfrm/xfrm_user.c | 16 +++++++++++++++-
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index a8cd6a4..8d2fa8d 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -506,4 +506,48 @@ enum xfrm_nlgroups {
};
#define XFRMNLGRP_MAX (__XFRMNLGRP_MAX - 1)
+#ifdef CONFIG_COMPAT
+
+struct compat_xfrm_userpolicy_info {
+ struct xfrm_selector sel;
+ struct xfrm_lifetime_cfg lft;
+ struct xfrm_lifetime_cur curlft;
+ __u32 priority;
+ __u32 index;
+ __u8 dir;
+ __u8 action;
+#define XFRM_POLICY_ALLOW 0
+#define XFRM_POLICY_BLOCK 1
+ __u8 flags;
+#define XFRM_POLICY_LOCALOK 1 /* Allow user to override global policy */
+ /* Automatically expand selector to include matching ICMP payloads. */
+#define XFRM_POLICY_ICMP 2
+ __u8 share;
+} __attribute__((packed));
+
+struct compat_xfrm_usersa_info {
+ struct xfrm_selector sel;
+ struct xfrm_id id;
+ xfrm_address_t saddr;
+ struct xfrm_lifetime_cfg lft;
+ struct xfrm_lifetime_cur curlft;
+ struct xfrm_stats stats;
+ __u32 seq;
+ __u32 reqid;
+ __u16 family;
+ __u8 mode; /* XFRM_MODE_xxx */
+ __u8 replay_window;
+ __u8 flags;
+#define XFRM_STATE_NOECN 1
+#define XFRM_STATE_DECAP_DSCP 2
+#define XFRM_STATE_NOPMTUDISC 4
+#define XFRM_STATE_WILDRECV 8
+#define XFRM_STATE_ICMP 16
+#define XFRM_STATE_AF_UNSPEC 32
+#define XFRM_STATE_ALIGN4 64
+#define XFRM_STATE_ESN 128
+} __attribute__((packed));
+
+#endif
+
#endif /* _LINUX_XFRM_H */
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 1ae3ec7..8bfbd95 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2281,6 +2281,12 @@ static const int xfrm_msg_min[XFRM_NR_MSGTYPES] = {
[XFRM_MSG_GETSPDINFO - XFRM_MSG_BASE] = sizeof(u32),
};
+static const int compat_xfrm_msg_min[XFRM_NR_MSGTYPES] = {
+ [XFRM_MSG_NEWSA - XFRM_MSG_BASE] = XMSGSIZE(compat_xfrm_usersa_info),
+ [XFRM_MSG_NEWPOLICY - XFRM_MSG_BASE] = XMSGSIZE(compat_xfrm_userpolicy_info),
+ [XFRM_MSG_UPDPOLICY - XFRM_MSG_BASE] = XMSGSIZE(compat_xfrm_userpolicy_info),
+};
+
#undef XMSGSIZE
static const struct nla_policy xfrma_policy[XFRMA_MAX+1] = {
@@ -2346,6 +2352,7 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
struct nlattr *attrs[XFRMA_MAX+1];
const struct xfrm_link *link;
int type, err;
+ int len;
type = nlh->nlmsg_type;
if (type > XFRM_MSG_MAX)
@@ -2373,7 +2380,14 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
}
}
- err = nlmsg_parse(nlh, xfrm_msg_min[type], attrs, XFRMA_MAX,
+#ifdef CONFIG_COMPAT
+ if (is_compat_task())
+ len = compat_xfrm_msg_min[type];
+ else
+#endif
+ len = xfrm_msg_min[type];
+
+ err = nlmsg_parse(nlh, len, attrs, XFRMA_MAX,
xfrma_policy);
if (err < 0)
return err;
--
1.7.9.5
^ permalink raw reply related
* [PATCH net] qeth: postpone freeing of qdio memory
From: Frank Blaschka @ 2014-02-19 9:14 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390
From: Ursula Braun <ursula.braun@de.ibm.com>
To guarantee that a qdio ccw_device no longer touches the
qdio memory shared with Linux, the qdio ccw_device should
be offline when freeing the qdio memory. Thus this patch
postpones freeing of qdio memory.
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
Reviewed-by: Sebastian Ott <sebott@linux.vnet.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 3 ++-
drivers/s390/net/qeth_l2_main.c | 3 +++
drivers/s390/net/qeth_l3_main.c | 3 +++
3 files changed, 8 insertions(+), 1 deletion(-)
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -1660,7 +1660,6 @@ int qeth_qdio_clear_card(struct qeth_car
QDIO_FLAG_CLEANUP_USING_CLEAR);
if (rc)
QETH_CARD_TEXT_(card, 3, "1err%d", rc);
- qdio_free(CARD_DDEV(card));
atomic_set(&card->qdio.state, QETH_QDIO_ALLOCATED);
break;
case QETH_QDIO_CLEANING:
@@ -2605,6 +2604,7 @@ static int qeth_mpc_initialize(struct qe
return 0;
out_qdio:
qeth_qdio_clear_card(card, card->info.type != QETH_CARD_TYPE_IQD);
+ qdio_free(CARD_DDEV(card));
return rc;
}
@@ -4919,6 +4919,7 @@ retry:
if (rc)
goto retriable;
rc = qeth_qdio_clear_card(card, card->info.type != QETH_CARD_TYPE_IQD);
+ qdio_free(CARD_DDEV(card));
retriable:
if (rc == -ERESTARTSYS) {
QETH_DBF_TEXT(SETUP, 2, "break1");
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -1091,6 +1091,7 @@ out_remove:
ccw_device_set_offline(CARD_DDEV(card));
ccw_device_set_offline(CARD_WDEV(card));
ccw_device_set_offline(CARD_RDEV(card));
+ qdio_free(CARD_DDEV(card));
if (recover_flag == CARD_STATE_RECOVER)
card->state = CARD_STATE_RECOVER;
else
@@ -1132,6 +1133,7 @@ static int __qeth_l2_set_offline(struct
rc = (rc2) ? rc2 : rc3;
if (rc)
QETH_DBF_TEXT_(SETUP, 2, "1err%d", rc);
+ qdio_free(CARD_DDEV(card));
if (recover_flag == CARD_STATE_UP)
card->state = CARD_STATE_RECOVER;
/* let user_space know that device is offline */
@@ -1194,6 +1196,7 @@ static void qeth_l2_shutdown(struct ccwg
qeth_hw_trap(card, QETH_DIAGS_TRAP_DISARM);
qeth_qdio_clear_card(card, 0);
qeth_clear_qdio_buffers(card);
+ qdio_free(CARD_DDEV(card));
}
static int qeth_l2_pm_suspend(struct ccwgroup_device *gdev)
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -3447,6 +3447,7 @@ out_remove:
ccw_device_set_offline(CARD_DDEV(card));
ccw_device_set_offline(CARD_WDEV(card));
ccw_device_set_offline(CARD_RDEV(card));
+ qdio_free(CARD_DDEV(card));
if (recover_flag == CARD_STATE_RECOVER)
card->state = CARD_STATE_RECOVER;
else
@@ -3493,6 +3494,7 @@ static int __qeth_l3_set_offline(struct
rc = (rc2) ? rc2 : rc3;
if (rc)
QETH_DBF_TEXT_(SETUP, 2, "1err%d", rc);
+ qdio_free(CARD_DDEV(card));
if (recover_flag == CARD_STATE_UP)
card->state = CARD_STATE_RECOVER;
/* let user_space know that device is offline */
@@ -3545,6 +3547,7 @@ static void qeth_l3_shutdown(struct ccwg
qeth_hw_trap(card, QETH_DIAGS_TRAP_DISARM);
qeth_qdio_clear_card(card, 0);
qeth_clear_qdio_buffers(card);
+ qdio_free(CARD_DDEV(card));
}
static int qeth_l3_pm_suspend(struct ccwgroup_device *gdev)
^ permalink raw reply
* [PATCH net v2 0/2] Fix RTNL: assertion failed at net/core/rtnetlink.c
From: Ding Tianhong @ 2014-02-19 9:05 UTC (permalink / raw)
To: fubar, vfalico, andy
Cc: cwang, jiri, thomas, eric.dumazet, sfeldma, davem, netdev
The commit 1d3ee88ae0d
(bonding: add netlink attributes to slave link dev)
make the bond_set_active_slave() and bond_set_backup_slave()
use rtmsg_ifinfo to send slave's states and this functions
should be called in RTNL.
But the 902.3ad and ARP monitor did not hold the RTNL when calling
thses two functions, so fix them.
v1->v2: Add new micro to indicate that the notification should be send
later, not never.
And add a new patch to fix the same problem for ARP mode.
Ding Tianhong (2):
bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for
802.3ad mode
bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for ab
arp monitor
drivers/net/bonding/bond_3ad.c | 16 +++++-
drivers/net/bonding/bond_main.c | 117 ++++++++++++++++++++++------------------
drivers/net/bonding/bonding.h | 47 ++++++++++++++--
3 files changed, 121 insertions(+), 59 deletions(-)
--
1.8.0
^ permalink raw reply
* [PATCH net v2 2/2] bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for ab arp monitor
From: Ding Tianhong @ 2014-02-19 9:05 UTC (permalink / raw)
To: fubar, vfalico, andy
Cc: cwang, jiri, thomas, eric.dumazet, sfeldma, davem, netdev
In-Reply-To: <1392800711-19264-1-git-send-email-dingtianhong@huawei.com>
Veaceslav has reported and fix this problem by commit f2ebd477f141bc0
(bonding: restructure locking of bond_ab_arp_probe()). According Jay's
opinion, the current solution is not very well, because the notification
is to indicate that the interface has actually changed state in a meaningful
way, but these calls in the ab ARP monitor are internal settings of the flags
to allow the ARP monitor to search for a slave to become active when there are
no active slaves. The flag setting to active or backup is to permit the ARP
monitor's response logic to do the right thing when deciding if the test
slave (current_arp_slave) is up or not.
So the best way to fix the problem is that we should not send a notification
when the slave is in testing state, and check the state at the end of the
monitor, if the slave's state recover, avoid to send pointless notification
twice. And RTNL is really a big lock, hold it regardless the slave's state
changed or not when the current_active_slave is null will loss performance
(every 100ms), so we should hold it only when the slave's state changed and
need to notify.
I revert the old commit and add new modifications.
Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Veaceslav Falico <vfalico@redhat.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_3ad.c | 8 +---
drivers/net/bonding/bond_main.c | 82 +++++++++++++++++++++--------------------
drivers/net/bonding/bonding.h | 13 +++++++
3 files changed, 56 insertions(+), 47 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index d4ace0b..bbe8448 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2130,13 +2130,7 @@ re_arm:
read_unlock(&bond->lock);
if (slave_should_notify && rtnl_trylock()) {
- bond_for_each_slave(bond, slave, iter) {
- if (slave->should_notify) {
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0,
- GFP_KERNEL);
- slave->should_notify = 0;
- }
- }
+ bond_slave_state_notify(bond);
rtnl_unlock();
}
queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e02029b..f107be9 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2623,17 +2623,17 @@ do_failover:
/*
* Send ARP probes for active-backup mode ARP monitor.
+ *
+ * Called with rcu_read_lock hold.
*/
-static bool bond_ab_arp_probe(struct bonding *bond)
+static int bond_ab_arp_probe(struct bonding *bond)
{
struct slave *slave, *before = NULL, *new_slave = NULL,
- *curr_arp_slave, *curr_active_slave;
+ *curr_arp_slave = rcu_dereference(bond->current_arp_slave),
+ *curr_active_slave = rcu_dereference(bond->curr_active_slave);
struct list_head *iter;
bool found = false;
-
- rcu_read_lock();
- curr_arp_slave = rcu_dereference(bond->current_arp_slave);
- curr_active_slave = rcu_dereference(bond->curr_active_slave);
+ int slave_should_notify = 0;
if (curr_arp_slave && curr_active_slave)
pr_info("PROBE: c_arp %s && cas %s BAD\n",
@@ -2642,32 +2642,23 @@ static bool bond_ab_arp_probe(struct bonding *bond)
if (curr_active_slave) {
bond_arp_send_all(bond, curr_active_slave);
- rcu_read_unlock();
- return true;
+ return slave_should_notify;
}
- rcu_read_unlock();
/* if we don't have a curr_active_slave, search for the next available
* backup slave from the current_arp_slave and make it the candidate
* for becoming the curr_active_slave
*/
- if (!rtnl_trylock())
- return false;
- /* curr_arp_slave might have gone away */
- curr_arp_slave = ACCESS_ONCE(bond->current_arp_slave);
-
if (!curr_arp_slave) {
- curr_arp_slave = bond_first_slave(bond);
- if (!curr_arp_slave) {
- rtnl_unlock();
- return true;
- }
+ curr_arp_slave = bond_first_slave_rcu(bond);
+ if (!curr_arp_slave)
+ return slave_should_notify;
}
- bond_set_slave_inactive_flags(curr_arp_slave, BOND_SLAVE_NOTIFY_NOW);
+ bond_set_slave_inactive_flags(curr_arp_slave, BOND_SLAVE_NOTIFY_LATER);
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
if (!found && !before && IS_UP(slave->dev))
before = slave;
@@ -2686,7 +2677,7 @@ static bool bond_ab_arp_probe(struct bonding *bond)
slave->link_failure_count++;
bond_set_slave_inactive_flags(slave,
- BOND_SLAVE_NOTIFY_NOW);
+ BOND_SLAVE_NOTIFY_LATER);
pr_info("%s: backup interface %s is now down.\n",
bond->dev->name, slave->dev->name);
@@ -2698,27 +2689,32 @@ static bool bond_ab_arp_probe(struct bonding *bond)
if (!new_slave && before)
new_slave = before;
- if (!new_slave) {
- rtnl_unlock();
- return true;
- }
+ if (!new_slave)
+ goto check_state;
new_slave->link = BOND_LINK_BACK;
- bond_set_slave_active_flags(new_slave, BOND_SLAVE_NOTIFY_NOW);
+ bond_set_slave_active_flags(new_slave, BOND_SLAVE_NOTIFY_LATER);
bond_arp_send_all(bond, new_slave);
new_slave->jiffies = jiffies;
rcu_assign_pointer(bond->current_arp_slave, new_slave);
- rtnl_unlock();
- return true;
+check_state:
+ bond_for_each_slave_rcu(bond, slave, iter) {
+ if (slave->should_notify) {
+ slave_should_notify = 1;
+ break;
+ }
+ }
+ return slave_should_notify;
}
static void bond_activebackup_arp_mon(struct work_struct *work)
{
struct bonding *bond = container_of(work, struct bonding,
arp_work.work);
- bool should_notify_peers = false, should_commit = false;
+ bool should_notify_peers = false;
int delta_in_ticks;
+ int slave_should_notify = 0;
delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval);
@@ -2726,11 +2722,12 @@ static void bond_activebackup_arp_mon(struct work_struct *work)
goto re_arm;
rcu_read_lock();
+
should_notify_peers = bond_should_notify_peers(bond);
- should_commit = bond_ab_arp_inspect(bond);
- rcu_read_unlock();
- if (should_commit) {
+ if (bond_ab_arp_inspect(bond)) {
+ rcu_read_unlock();
+
/* Race avoidance with bond_close flush of workqueue */
if (!rtnl_trylock()) {
delta_in_ticks = 1;
@@ -2739,23 +2736,28 @@ static void bond_activebackup_arp_mon(struct work_struct *work)
}
bond_ab_arp_commit(bond);
+
rtnl_unlock();
+ rcu_read_lock();
}
- if (!bond_ab_arp_probe(bond)) {
- /* rtnl locking failed, re-arm */
- delta_in_ticks = 1;
- should_notify_peers = false;
- }
+ slave_should_notify = bond_ab_arp_probe(bond);
+ rcu_read_unlock();
re_arm:
if (bond->params.arp_interval)
queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
- if (should_notify_peers) {
+ if (should_notify_peers || slave_should_notify) {
if (!rtnl_trylock())
return;
- call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
+
+ if (should_notify_peers)
+ call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
+ bond->dev);
+ if (slave_should_notify)
+ bond_slave_state_notify(bond);
+
rtnl_unlock();
}
}
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index b4e6c53..aeed642 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -335,6 +335,19 @@ static inline void bond_slave_state_change(struct bonding *bond)
}
}
+static inline void bond_slave_state_notify(struct bonding *bond)
+{
+ struct list_head *iter;
+ struct slave *tmp;
+
+ bond_for_each_slave(bond, tmp, iter) {
+ if (tmp->should_notify) {
+ rtmsg_ifinfo(RTM_NEWLINK, tmp->dev, 0, GFP_KERNEL);
+ tmp->should_notify = 0;
+ }
+ }
+}
+
static inline int bond_slave_state(struct slave *slave)
{
return slave->backup;
--
1.8.0
^ permalink raw reply related
* [PATCH net v2 1/2] bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for 802.3ad mode
From: Ding Tianhong @ 2014-02-19 9:05 UTC (permalink / raw)
To: fubar, vfalico, andy
Cc: cwang, jiri, thomas, eric.dumazet, sfeldma, davem, netdev
In-Reply-To: <1392800711-19264-1-git-send-email-dingtianhong@huawei.com>
The problem was introduced by the commit 1d3ee88ae0d
(bonding: add netlink attributes to slave link dev).
The bond_set_active_slave() and bond_set_backup_slave()
will use rtmsg_ifinfo to send slave's states, so these
two functions should be called in RTNL.
In 802.3ad mode, acquiring RTNL for the __enable_port and
__disable_port cases is difficult, as those calls generally
already hold the state machine lock, and cannot unconditionally
call rtnl_lock because either they already hold RTNL (for calls
via bond_3ad_unbind_slave) or due to the potential for deadlock
with bond_3ad_adapter_speed_changed, bond_3ad_adapter_duplex_changed,
bond_3ad_link_change, or bond_3ad_update_lacp_rate. All four of
those are called with RTNL held, and acquire the state machine lock
second. The calling contexts for __enable_port and __disable_port
already hold the state machine lock, and may or may not need RTNL.
According to the Jay's opinion, I don't think it is a problem that
the slave don't send notify message synchronously when the status
changed, normally the state machine is running every 100 ms, send
the notify message at the end of the state machine if the slave's
state changed should be better.
I fix the problem through these steps:
1). add a new function bond_set_slave_state() which could change
the slave's state and call rtmsg_ifinfo() according to the input
parameters called notify.
2). Add a new slave parameter which called should_notify, if the slave's state
changed and don't notify yet, the parameter will be set to 1, and then if
the slave's state changed again, the param will be set to 0, it indicate that
the slave's state has been restored, no need to notify any one.
3). the __enable_port and __disable_port should not call rtmsg_ifinfo
in the state machine lock, any change in the state of slave could
set a flag in the slave, it will indicated that an rtmsg_ifinfo
should be called at the end of the state machine.
Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Veaceslav Falico <vfalico@redhat.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_3ad.c | 22 ++++++++++++++++++++--
drivers/net/bonding/bond_main.c | 41 ++++++++++++++++++++++++++---------------
drivers/net/bonding/bonding.h | 34 +++++++++++++++++++++++++++++-----
3 files changed, 75 insertions(+), 22 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 6d20fbd..d4ace0b 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -181,7 +181,7 @@ static inline int __agg_has_partner(struct aggregator *agg)
*/
static inline void __disable_port(struct port *port)
{
- bond_set_slave_inactive_flags(port->slave);
+ bond_set_slave_inactive_flags(port->slave, BOND_SLAVE_NOTIFY_LATER);
}
/**
@@ -193,7 +193,7 @@ static inline void __enable_port(struct port *port)
struct slave *slave = port->slave;
if ((slave->link == BOND_LINK_UP) && IS_UP(slave->dev))
- bond_set_slave_active_flags(slave);
+ bond_set_slave_active_flags(slave, BOND_SLAVE_NOTIFY_LATER);
}
/**
@@ -2062,6 +2062,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
struct list_head *iter;
struct slave *slave;
struct port *port;
+ int slave_should_notify = 0;
read_lock(&bond->lock);
rcu_read_lock();
@@ -2119,8 +2120,25 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
}
re_arm:
+ bond_for_each_slave_rcu(bond, slave, iter) {
+ if (slave->should_notify) {
+ slave_should_notify = 1;
+ break;
+ }
+ }
rcu_read_unlock();
read_unlock(&bond->lock);
+
+ if (slave_should_notify && rtnl_trylock()) {
+ bond_for_each_slave(bond, slave, iter) {
+ if (slave->should_notify) {
+ rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0,
+ GFP_KERNEL);
+ slave->should_notify = 0;
+ }
+ }
+ rtnl_unlock();
+ }
queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
}
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 1c6104d..e02029b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -829,21 +829,25 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
if (bond_is_lb(bond)) {
bond_alb_handle_active_change(bond, new_active);
if (old_active)
- bond_set_slave_inactive_flags(old_active);
+ bond_set_slave_inactive_flags(old_active,
+ BOND_SLAVE_NOTIFY_NOW);
if (new_active)
- bond_set_slave_active_flags(new_active);
+ bond_set_slave_active_flags(new_active,
+ BOND_SLAVE_NOTIFY_NOW);
} else {
rcu_assign_pointer(bond->curr_active_slave, new_active);
}
if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
if (old_active)
- bond_set_slave_inactive_flags(old_active);
+ bond_set_slave_inactive_flags(old_active,
+ BOND_SLAVE_NOTIFY_NOW);
if (new_active) {
bool should_notify_peers = false;
- bond_set_slave_active_flags(new_active);
+ bond_set_slave_active_flags(new_active,
+ BOND_SLAVE_NOTIFY_NOW);
if (bond->params.fail_over_mac)
bond_do_fail_over_mac(bond, new_active,
@@ -1463,14 +1467,15 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
switch (bond->params.mode) {
case BOND_MODE_ACTIVEBACKUP:
- bond_set_slave_inactive_flags(new_slave);
+ bond_set_slave_inactive_flags(new_slave,
+ BOND_SLAVE_NOTIFY_NOW);
break;
case BOND_MODE_8023AD:
/* in 802.3ad mode, the internal mechanism
* will activate the slaves in the selected
* aggregator
*/
- bond_set_slave_inactive_flags(new_slave);
+ bond_set_slave_inactive_flags(new_slave, BOND_SLAVE_NOTIFY_NOW);
/* if this is the first slave */
if (!prev_slave) {
SLAVE_AD_INFO(new_slave).id = 1;
@@ -1488,7 +1493,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
case BOND_MODE_TLB:
case BOND_MODE_ALB:
bond_set_active_slave(new_slave);
- bond_set_slave_inactive_flags(new_slave);
+ bond_set_slave_inactive_flags(new_slave, BOND_SLAVE_NOTIFY_NOW);
break;
default:
pr_debug("This slave is always active in trunk mode\n");
@@ -2015,7 +2020,8 @@ static void bond_miimon_commit(struct bonding *bond)
if (bond->params.mode == BOND_MODE_ACTIVEBACKUP ||
bond->params.mode == BOND_MODE_8023AD)
- bond_set_slave_inactive_flags(slave);
+ bond_set_slave_inactive_flags(slave,
+ BOND_SLAVE_NOTIFY_NOW);
pr_info("%s: link status definitely down for interface %s, disabling it\n",
bond->dev->name, slave->dev->name);
@@ -2562,7 +2568,8 @@ static void bond_ab_arp_commit(struct bonding *bond)
slave->link = BOND_LINK_UP;
if (bond->current_arp_slave) {
bond_set_slave_inactive_flags(
- bond->current_arp_slave);
+ bond->current_arp_slave,
+ BOND_SLAVE_NOTIFY_NOW);
bond->current_arp_slave = NULL;
}
@@ -2582,7 +2589,8 @@ static void bond_ab_arp_commit(struct bonding *bond)
slave->link_failure_count++;
slave->link = BOND_LINK_DOWN;
- bond_set_slave_inactive_flags(slave);
+ bond_set_slave_inactive_flags(slave,
+ BOND_SLAVE_NOTIFY_NOW);
pr_info("%s: link status definitely down for interface %s, disabling it\n",
bond->dev->name, slave->dev->name);
@@ -2657,7 +2665,7 @@ static bool bond_ab_arp_probe(struct bonding *bond)
}
}
- bond_set_slave_inactive_flags(curr_arp_slave);
+ bond_set_slave_inactive_flags(curr_arp_slave, BOND_SLAVE_NOTIFY_NOW);
bond_for_each_slave(bond, slave, iter) {
if (!found && !before && IS_UP(slave->dev))
@@ -2677,7 +2685,8 @@ static bool bond_ab_arp_probe(struct bonding *bond)
if (slave->link_failure_count < UINT_MAX)
slave->link_failure_count++;
- bond_set_slave_inactive_flags(slave);
+ bond_set_slave_inactive_flags(slave,
+ BOND_SLAVE_NOTIFY_NOW);
pr_info("%s: backup interface %s is now down.\n",
bond->dev->name, slave->dev->name);
@@ -2695,7 +2704,7 @@ static bool bond_ab_arp_probe(struct bonding *bond)
}
new_slave->link = BOND_LINK_BACK;
- bond_set_slave_active_flags(new_slave);
+ bond_set_slave_active_flags(new_slave, BOND_SLAVE_NOTIFY_NOW);
bond_arp_send_all(bond, new_slave);
new_slave->jiffies = jiffies;
rcu_assign_pointer(bond->current_arp_slave, new_slave);
@@ -3046,9 +3055,11 @@ static int bond_open(struct net_device *bond_dev)
bond_for_each_slave(bond, slave, iter) {
if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP)
&& (slave != bond->curr_active_slave)) {
- bond_set_slave_inactive_flags(slave);
+ bond_set_slave_inactive_flags(slave,
+ BOND_SLAVE_NOTIFY_NOW);
} else {
- bond_set_slave_active_flags(slave);
+ bond_set_slave_active_flags(slave,
+ BOND_SLAVE_NOTIFY_NOW);
}
}
read_unlock(&bond->curr_slave_lock);
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 86ccfb9..b4e6c53 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -195,7 +195,8 @@ struct slave {
s8 new_link;
u8 backup:1, /* indicates backup slave. Value corresponds with
BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
- inactive:1; /* indicates inactive slave */
+ inactive:1, /* indicates inactive slave */
+ should_notify:1; /* indicateds whether the state changed */
u8 duplex;
u32 original_mtu;
u32 link_failure_count;
@@ -303,6 +304,24 @@ static inline void bond_set_backup_slave(struct slave *slave)
}
}
+static inline void bond_set_slave_state(struct slave *slave,
+ int slave_state, bool notify)
+{
+ if (slave->backup == slave_state)
+ return;
+
+ slave->backup = slave_state;
+ if (notify) {
+ rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_KERNEL);
+ slave->should_notify = 0;
+ } else {
+ if (slave->should_notify)
+ slave->should_notify = 0;
+ else
+ slave->should_notify = 1;
+ }
+}
+
static inline void bond_slave_state_change(struct bonding *bond)
{
struct list_head *iter;
@@ -343,6 +362,9 @@ static inline bool bond_is_active_slave(struct slave *slave)
#define BOND_ARP_VALIDATE_ALL (BOND_ARP_VALIDATE_ACTIVE | \
BOND_ARP_VALIDATE_BACKUP)
+#define BOND_SLAVE_NOTIFY_NOW true
+#define BOND_SLAVE_NOTIFY_LATER false
+
static inline int slave_do_arp_validate(struct bonding *bond,
struct slave *slave)
{
@@ -394,17 +416,19 @@ static inline void bond_netpoll_send_skb(const struct slave *slave,
}
#endif
-static inline void bond_set_slave_inactive_flags(struct slave *slave)
+static inline void bond_set_slave_inactive_flags(struct slave *slave,
+ bool notify)
{
if (!bond_is_lb(slave->bond))
- bond_set_backup_slave(slave);
+ bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
if (!slave->bond->params.all_slaves_active)
slave->inactive = 1;
}
-static inline void bond_set_slave_active_flags(struct slave *slave)
+static inline void bond_set_slave_active_flags(struct slave *slave,
+ bool notify)
{
- bond_set_active_slave(slave);
+ bond_set_slave_state(slave, BOND_STATE_ACTIVE, notify);
slave->inactive = 0;
}
--
1.8.0
^ permalink raw reply related
* [PATCH] be2net: Bugfix for packet drop with kernel param swiotlb=force
From: jiang.biao2 @ 2014-02-19 8:25 UTC (permalink / raw)
To: netdev
Cc: Sathya Perla, Subbu Seetharaman, Ajit Khaparde, wang.liang82,
cai.qu, li.fengmao, long.chun
From: Li Fengmao <li.fengmao@zte.com.cn>
There will be packet drop with kernel param "swiotlb = force" on
Emulex 10Gb NIC using be2net driver. The problem is caused by
receiving skb without calling pci_unmap_page() in get_rx_page_info().
rx_page_info->last_page_user is initialized to false in
be_post_rx_frags() when current frag are mapped in the first half of
the same page with another frag. But in that case with
"swiotlb = force" param, data can not be copied into the page of
rx_page_info without calling pci_unmap_page, so the data frag mapped
in the first half of the page will be dropped.
It can be solved by creating only a mapping relation between frag
and page, and deleting rx_page_info->last_page_user to ensure
calling pci_unmap_page when handling each receiving frag.
Steps to reproduce the bug:
1. Prepare a Emulex Corporation OneConnect 10Gb NIC.
2. Add the kernel param like "swiotlb = force" in /boot/grub/grub.conf .
3. Reboot the system. (e.g exec reboot command)
3. Activate the interface. (e.g ifconfig eth0 192.168.1.2 up)
4. There will be packet drop when ping 192.168.1.2 from another host.
Signed-off-by: Li Fengmao <li.fengmao@zte.com.cn>
Signed-off-by: Long Chun <long.chun@zte.com.cn>
Reviewed-by: Wang Liang <wang.liang82@zte.com.cn>
Reviewed-by: Cai Qu <cai.qu@zte.com.cn>
Reviewed-by: Jiang Biao <jiang.biao2@zte.com.cn>
--- old/drivers/net/ethernet/emulex/benet/be_main.c 2014-02-18 03:34:15.206388270 -0500
+++ new/drivers/net/ethernet/emulex/benet/be_main.c 2014-02-18 03:44:17.368388223 -0500
@@ -1018,13 +1018,9 @@ get_rx_page_info(struct be_adapter *adap
rx_page_info = &rxo->page_info_tbl[frag_idx];
BUG_ON(!rx_page_info->page);
- if (rx_page_info->last_page_user) {
- dma_unmap_page(&adapter->pdev->dev,
- dma_unmap_addr(rx_page_info, bus),
- adapter->big_page_size, DMA_FROM_DEVICE);
- rx_page_info->last_page_user = false;
- }
-
+ dma_unmap_page(&adapter->pdev->dev,
+ dma_unmap_addr(rx_page_info, bus),
+ rx_frag_size, DMA_FROM_DEVICE);
atomic_dec(&rxq->used);
return rx_page_info;
}
@@ -1344,20 +1340,15 @@ static void be_post_rx_frags(struct be_r
page_info = &rxo->page_info_tbl[rxq->head];
for (posted = 0; posted < MAX_RX_POST && !page_info->page; posted++) {
- if (!pagep) {
- pagep = be_alloc_pages(adapter->big_page_size, gfp);
- if (unlikely(!pagep)) {
- rx_stats(rxo)->rx_post_fail++;
- break;
- }
- page_dmaaddr = dma_map_page(&adapter->pdev->dev, pagep,
- 0, adapter->big_page_size,
- DMA_FROM_DEVICE);
- page_info->page_offset = 0;
- } else {
- get_page(pagep);
- page_info->page_offset = page_offset + rx_frag_size;
+ pagep = be_alloc_pages(rx_frag_size, gfp);
+ if (unlikely(!pagep)) {
+ rx_stats(rxo)->rx_post_fail++;
+ break;
}
+ page_dmaaddr = dma_map_page(&adapter->pdev->dev, pagep,
+ 0, rx_frag_size,
+ DMA_FROM_DEVICE);
+ page_info->page_offset = 0;
page_offset = page_info->page_offset;
page_info->page = pagep;
dma_unmap_addr_set(page_info, bus, page_dmaaddr);
@@ -1367,12 +1358,7 @@ static void be_post_rx_frags(struct be_r
rxd->fragpa_lo = cpu_to_le32(frag_dmaaddr & 0xFFFFFFFF);
rxd->fragpa_hi = cpu_to_le32(upper_32_bits(frag_dmaaddr));
- /* Any space left in the current big page for another frag? */
- if ((page_offset + rx_frag_size + rx_frag_size) >
- adapter->big_page_size) {
- pagep = NULL;
- page_info->last_page_user = true;
- }
+ pagep = NULL;
prev_page_info = page_info;
queue_head_inc(rxq);
^ permalink raw reply
* I want to invest this fund.
From: Ms.Sarah Angus @ 2014-02-19 8:10 UTC (permalink / raw)
Hi,
Can you assist me to invest funds in your Country? if yes, then get back to
me, so I can give you details and areas you can
participate by being my partner.
Ms. Sarah
^ permalink raw reply
* Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text
From: Thomas Petazzoni @ 2014-02-19 8:08 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Jason Cooper, Andrew Lunn, netdev@vger.kernel.org,
Ezequiel Garcia, Gregory Clement, David S. Miller,
linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth
In-Reply-To: <20140219101043.5d403398@xhacker>
Dear Jisheng Zhang,
On Wed, 19 Feb 2014 10:10:43 +0800, Jisheng Zhang wrote:
> > At this point, I don't expect to see this IP in any other SOCs than
> > Marvell Armada EBU, so I'll resend with a "depends on PLAT_ORION", as I
>
> This IP is used in Marvell Berlin SoCs too ;)
Ah good to know!
Then when this IP will be enabled on Marvell Berlin SoCs, we can extend
the "depends on PLAT_ORION" to "depends on PLAT_ORION || ARCH_BERLIN".
> But Berlin SoC doesn't have a mbus concept. Currently, we just hack the
> mvneta to configure the mbus window.
We can simply introduce a separate compatible string, and make the
driver behave differently depending on which compatible string was used
to probe the device.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v2 net-next] ethtool: Remove inactive code snippet in ethtool_set_rxfh_indir
From: Venkat Duvvuru @ 2014-02-19 7:50 UTC (permalink / raw)
To: netdev; +Cc: Venkat Duvvuru
ethtool_rxfh_indir_default is invoked inside ethtool_set_rxfh_indir when
user_size is zero. However user_size can only be zero when dev_size is
zero in which case ethtool_get_rxfh_indir itself would return
EOPNOTSUPP error. "user_size = 0" can never happen unless userspace
ethtool sends user_size as zero beforing invoking the ioctl for
ETHTOOL_SRSSH, in which case we should return -EINVAL.
v2: Modified commit message.
Signed-off-by: Venkat Duvvuru <VenkatKumar.Duvvuru@Emulex.com>
---
net/core/ethtool.c | 33 ++++++++++++++-------------------
1 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 30071de..9d96516 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -626,7 +626,7 @@ static noinline_for_stack int ethtool_set_rxfh_indir(struct net_device *dev,
sizeof(user_size)))
return -EFAULT;
- if (user_size != 0 && user_size != dev_size)
+ if (!user_size || user_size != dev_size)
return -EINVAL;
indir = kcalloc(dev_size, sizeof(indir[0]), GFP_USER);
@@ -638,25 +638,20 @@ static noinline_for_stack int ethtool_set_rxfh_indir(struct net_device *dev,
if (ret)
goto out;
- if (user_size == 0) {
- for (i = 0; i < dev_size; i++)
- indir[i] = ethtool_rxfh_indir_default(i, rx_rings.data);
- } else {
- if (copy_from_user(indir,
- useraddr +
- offsetof(struct ethtool_rxfh_indir,
- ring_index[0]),
- dev_size * sizeof(indir[0]))) {
- ret = -EFAULT;
- goto out;
- }
+ if (copy_from_user(indir,
+ useraddr +
+ offsetof(struct ethtool_rxfh_indir,
+ ring_index[0]),
+ dev_size * sizeof(indir[0]))) {
+ ret = -EFAULT;
+ goto out;
+ }
- /* Validate ring indices */
- for (i = 0; i < dev_size; i++) {
- if (indir[i] >= rx_rings.data) {
- ret = -EINVAL;
- goto out;
- }
+ /* Validate ring indices */
+ for (i = 0; i < dev_size; i++) {
+ if (indir[i] >= rx_rings.data) {
+ ret = -EINVAL;
+ goto out;
}
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH net-next 12/14] r8152: replace netif_rx withnetif_receive_skb
From: Francois Romieu @ 2014-02-19 7:46 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <79F93FE4FECD48E7B7B6C6EE93151CB6@realtek.com.tw>
hayeswang <hayeswang@realtek.com> :
> Francois Romieu [mailto:romieu@fr.zoreil.com]
> > Hayes Wang <hayeswang@realtek.com> :
> > > Replace netif_rx with netif_receive_skb to avoid disabling irq frequently
> > > for increasing the efficiency.
> >
> > read_bulk_callback is issued in irq context. It could thus use plain
> > spin_lock / spin_unlock instead of the irq disabling version.
>
> The rx_bottom() is called in tasklet, so I just think I could use
> netif_receive_skb directly. The netif_rx seems to queue the packet,
> and local_irq_disable() would be called before dequeuing the skb.
The change in rx_bottom is fine. My point is about read_bulk_callback.
rx_bottom races with read_bulk_callback. rx_bottom is issued in
tasklet (softirq) context. read_bulk_callback is issued in irq
context, with irq disabled. read_bulk_callback does not need to
disable irq itself and could go with spin_lock in place of
spin_lock_irqsave (rx_bottom can't, of course).
--
Ueimor
^ permalink raw reply
* [PATCH net-next v2] tipc: failed transmissions should return error
From: erik.hugne @ 2014-02-19 7:37 UTC (permalink / raw)
To: netdev
Cc: tipc-discussion, ying.xue, paul.gortmaker, jon.maloy, maloy,
Erik Hugne
From: Erik Hugne <erik.hugne@ericsson.com>
When a message could not be sent out because the destination node
or link could not be found, the full message size is returned from
sendmsg() as if it had been sent successfully. An application will
then get a false indication that it's making forward progress. This
problem has existed since the initial commit in 2.6.16.
We change this to return -ENETUNREACH if the message cannot be
delivered due to the destination node/link being unavailable. We
also get rid of the redundant tipc_reject_msg call since freeing
the buffer and doing a tipc_port_iovec_reject accomplishes exactly
the same thing.
Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
---
v2: Respin on top of 247f0f3c3176c55b46cb9a20011d3d6757634815
tipc: align tipc function names with common naming practice in the network
net/tipc/link.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/net/tipc/link.c b/net/tipc/link.c
index e4f233d..e998dba 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1020,12 +1020,9 @@ exit:
read_unlock_bh(&tipc_net_lock);
/* Couldn't find a link to the destination node */
- if (buf)
- return tipc_reject_msg(buf, TIPC_ERR_NO_NODE);
- if (res >= 0)
- return tipc_port_iovec_reject(sender, hdr, msg_sect, len,
- TIPC_ERR_NO_NODE);
- return res;
+ kfree_skb(buf);
+ tipc_port_iovec_reject(sender, hdr, msg_sect, len, TIPC_ERR_NO_NODE);
+ return -ENETUNREACH;
}
/*
@@ -1163,8 +1160,9 @@ error:
} else {
reject:
kfree_skb_list(buf_chain);
- return tipc_port_iovec_reject(sender, hdr, msg_sect,
- len, TIPC_ERR_NO_NODE);
+ tipc_port_iovec_reject(sender, hdr, msg_sect, len,
+ TIPC_ERR_NO_NODE);
+ return -ENETUNREACH;
}
/* Append chain of fragments to send queue & send them */
--
1.7.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox