* [PATCH net-next RFC 4/5] xen-netback: Fix indentations
From: Zoltan Kiss @ 2013-10-30 0:50 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1383094220-14775-1-git-send-email-zoltan.kiss@citrix.com>
I've left intentionally these indentations in this way, to improve readability of previous patches.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/netback.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index c91dd36..204fa46 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -882,8 +882,8 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
for (shinfo->nr_frags = start; shinfo->nr_frags < nr_slots;
shinfo->nr_frags++, txp++, gop++) {
- index = pending_index(vif->pending_cons++);
- pending_idx = vif->pending_ring[index];
+ index = pending_index(vif->pending_cons++);
+ pending_idx = vif->pending_ring[index];
xenvif_tx_create_gop(vif, pending_idx, txp, gop);
frag_set_pending_idx(&frags[shinfo->nr_frags], pending_idx);
}
@@ -929,7 +929,7 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
tx_info = &vif->pending_tx_info[pending_idx];
/* Check error status: if okay then remember grant handle. */
- newerr = (++gop)->status;
+ newerr = (++gop)->status;
if (likely(!newerr)) {
if (vif->grant_tx_handle[pending_idx] !=
@@ -1717,10 +1717,10 @@ static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
struct pending_tx_info *pending_tx_info;
pending_ring_idx_t index;
- pending_tx_info = &vif->pending_tx_info[pending_idx];
- make_tx_response(vif, &pending_tx_info->req, status);
- index = pending_index(vif->pending_prod++);
- vif->pending_ring[index] = pending_idx;
+ pending_tx_info = &vif->pending_tx_info[pending_idx];
+ make_tx_response(vif, &pending_tx_info->req, status);
+ index = pending_index(vif->pending_prod++);
+ vif->pending_ring[index] = pending_idx;
}
void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx)
^ permalink raw reply related
* [PATCH net-next RFC 4/5] xen-netback: Change RX path for mapped SKB fragments
From: Zoltan Kiss @ 2013-10-30 0:50 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1383094220-14775-1-git-send-email-zoltan.kiss@citrix.com>
RX path need to know if the SKB fragments are stored on pages from another
domain.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/netback.c | 46 +++++++++++++++++++++++++++++++++----
1 file changed, 41 insertions(+), 5 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 204fa46..0ffa412 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -322,7 +322,9 @@ static struct xenvif_rx_meta *get_next_rx_buffer(struct xenvif *vif,
static void xenvif_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
struct netrx_pending_operations *npo,
struct page *page, unsigned long size,
- unsigned long offset, int *head)
+ unsigned long offset, int *head,
+ struct xenvif *foreign_vif,
+ grant_ref_t foreign_gref)
{
struct gnttab_copy *copy_gop;
struct xenvif_rx_meta *meta;
@@ -364,8 +366,15 @@ static void xenvif_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
copy_gop->flags = GNTCOPY_dest_gref;
copy_gop->len = bytes;
- copy_gop->source.domid = DOMID_SELF;
- copy_gop->source.u.gmfn = virt_to_mfn(page_address(page));
+ if (foreign_vif) {
+ copy_gop->source.domid = foreign_vif->domid;
+ copy_gop->source.u.ref = foreign_gref;
+ copy_gop->flags |= GNTCOPY_source_gref;
+ } else {
+ copy_gop->source.domid = DOMID_SELF;
+ copy_gop->source.u.gmfn =
+ virt_to_mfn(page_address(page));
+ }
copy_gop->source.offset = offset;
copy_gop->dest.domid = vif->domid;
@@ -426,6 +435,9 @@ static int xenvif_gop_skb(struct sk_buff *skb,
int old_meta_prod;
int gso_type;
int gso_size;
+ struct ubuf_info *ubuf = skb_shinfo(skb)->destructor_arg;
+ grant_ref_t foreign_grefs[MAX_SKB_FRAGS];
+ struct xenvif *foreign_vif = NULL;
old_meta_prod = npo->meta_prod;
@@ -466,6 +478,26 @@ static int xenvif_gop_skb(struct sk_buff *skb,
npo->copy_off = 0;
npo->copy_gref = req->gref;
+ if ((skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) &&
+ (ubuf->callback == &xenvif_zerocopy_callback)) {
+ u16 pending_idx = ubuf->desc;
+ int i = 0;
+ struct pending_tx_info *temp =
+ container_of(ubuf,
+ struct pending_tx_info,
+ callback_struct);
+ foreign_vif =
+ container_of(temp - pending_idx,
+ struct xenvif,
+ pending_tx_info[0]);
+ do {
+ pending_idx = ubuf->desc;
+ ubuf = (struct ubuf_info *) ubuf->ctx;
+ foreign_grefs[i++] =
+ foreign_vif->pending_tx_info[pending_idx].req.gref;
+ } while (ubuf);
+ }
+
data = skb->data;
while (data < skb_tail_pointer(skb)) {
unsigned int offset = offset_in_page(data);
@@ -475,7 +507,9 @@ static int xenvif_gop_skb(struct sk_buff *skb,
len = skb_tail_pointer(skb) - data;
xenvif_gop_frag_copy(vif, skb, npo,
- virt_to_page(data), len, offset, &head);
+ virt_to_page(data), len, offset, &head,
+ NULL,
+ 0);
data += len;
}
@@ -484,7 +518,9 @@ static int xenvif_gop_skb(struct sk_buff *skb,
skb_frag_page(&skb_shinfo(skb)->frags[i]),
skb_frag_size(&skb_shinfo(skb)->frags[i]),
skb_shinfo(skb)->frags[i].page_offset,
- &head);
+ &head,
+ foreign_vif,
+ foreign_grefs[i]);
}
return npo->meta_prod - old_meta_prod;
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: introduce gro_frag_list_enable sysctl
From: Eric Dumazet @ 2013-10-30 0:53 UTC (permalink / raw)
To: David Miller; +Cc: christoph.paasch, herbert, netdev, hkchu, mwdalton
In-Reply-To: <20131029.194446.2215574000648693370.davem@davemloft.net>
On Tue, 2013-10-29 at 19:44 -0400, David Miller wrote:
> I do not like the idea of packet actions indirectly changing sysctl
> values, even if you document it sufficiently as you have here.
>
Fair enough.
> I would suggest instead making it change in response to changes to
> ip_forward, as we do with per-device LRO settings. This means that,
> like ip_forward, you should also make this sysctl a global + devinet
> per-device sysctl.
>
> You might even emit a pr_info() when this logic triggers, and if you
> are ambitious enough keep track of the previous GRO sysctl state so
> you can restore it if ip_forward is set back to zero.
Ok, but this might take some time.
So should we apply the first fix to avoid the BUG_ON() ?
^ permalink raw reply
* [GIT PULL] Second Round of IPVS updates for v3.13
From: Simon Horman @ 2013-10-30 1:11 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Daniel Borkmann, Simon Horman
Hi Pablo,
please consider this second round of updates or IPVS for v3.13,
based on nf-next.
I realise this is rather late in the game as the v3.13 merge window
is just round the corner. So please let me know if you would
like me to handle things a different way.
There are two SCTP changes by Daniel Borkman.
* Correct verdict assignments in sctp_conn_schedule
This is a bug fix for incorrect handling of the error case
where skb_header_pointer() fails.
It appears that this problem has been present since SCTP was added
to IPVS in v2.6.34.
I would like this change considered for -stable all the way back
to v2.6.34. I have checked and it cherry-picks cleanly on top
of v3.12-rc7 and v3.11. And seems to apply with some trivial merge conflicts
on top of v3.4, v3.2 and v3.6.34. I am happy to supply versions of
the patch that resolve those conflicts.
* Do not recalculate SCTP checksum if ports don't change
This is an enhancement which should lead to increased performance
in some cases.
The following changes since commit 6b8dbcf2c44fd7aa716560d04e9857c870bd510c:
bridge: netfilter: orphan skb before invoking ip netfilter hooks (2013-10-27 21:44:33 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git tags/ipvs2-for-v3.13
for you to fetch changes up to 97203abe6bc41ee020f37c902bd1a761157f22c1:
net: ipvs: sctp: do not recalc sctp csum when ports didn't change (2013-10-30 09:48:16 +0900)
----------------------------------------------------------------
Second Round of IPVS updates for v3.13
Some SCTP changes by Daniel Borkman.
* Correct verdict assignments in sctp_conn_schedule
This is a bug fix for incorrect handling of the error case
where skb_header_pointer() fails.
It appears that this problem has been present since SCTP was added
to IPVS in v2.6.34.
* Do not recalculate SCTP checksum if ports don't change
This is an enhancement which should lead to increased performance
in some cases.
----------------------------------------------------------------
Daniel Borkmann (2):
net: ipvs: sctp: add missing verdict assignments in sctp_conn_schedule
net: ipvs: sctp: do not recalc sctp csum when ports didn't change
net/netfilter/ipvs/ip_vs_proto_sctp.c | 48 +++++++++++++++++++++++++++++------
1 file changed, 40 insertions(+), 8 deletions(-)
^ permalink raw reply
* [PATCH nf-next 1/2] net: ipvs: sctp: add missing verdict assignments in sctp_conn_schedule
From: Simon Horman @ 2013-10-30 1:11 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Daniel Borkmann, Simon Horman
In-Reply-To: <1383095486-5215-1-git-send-email-horms@verge.net.au>
From: Daniel Borkmann <dborkman@redhat.com>
If skb_header_pointer() fails, we need to assign a verdict, that is
NF_DROP in this case, otherwise, we would leave the verdict from
conn_schedule() uninitialized when returning.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_proto_sctp.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index 23e596e..9ca7aa0 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -20,13 +20,18 @@ sctp_conn_schedule(int af, struct sk_buff *skb, struct ip_vs_proto_data *pd,
sctp_sctphdr_t *sh, _sctph;
sh = skb_header_pointer(skb, iph->len, sizeof(_sctph), &_sctph);
- if (sh == NULL)
+ if (sh == NULL) {
+ *verdict = NF_DROP;
return 0;
+ }
sch = skb_header_pointer(skb, iph->len + sizeof(sctp_sctphdr_t),
sizeof(_schunkh), &_schunkh);
- if (sch == NULL)
+ if (sch == NULL) {
+ *verdict = NF_DROP;
return 0;
+ }
+
net = skb_net(skb);
ipvs = net_ipvs(net);
rcu_read_lock();
--
1.8.4
^ permalink raw reply related
* [PATCH nf-next 2/2] net: ipvs: sctp: do not recalc sctp csum when ports didn't change
From: Simon Horman @ 2013-10-30 1:11 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Daniel Borkmann, Simon Horman
In-Reply-To: <1383095486-5215-1-git-send-email-horms@verge.net.au>
From: Daniel Borkmann <dborkman@redhat.com>
Unlike UDP or TCP, we do not take the pseudo-header into
account in SCTP checksums. So in case port mapping is the
very same, we do not need to recalculate the whole SCTP
checksum in software, which is very expensive.
Also, similarly as in TCP, take into account when a private
helper mangled the packet. In that case, we also need to
recalculate the checksum even if ports might be same.
Thanks for feedback regarding skb->ip_summed checks from
Julian Anastasov; here's a discussion on these checks for
snat and dnat:
* For snat_handler(), we can see CHECKSUM_PARTIAL from
virtual devices, and from LOCAL_OUT, otherwise it
should be CHECKSUM_UNNECESSARY. In general, in snat it
is more complex. skb contains the original route and
ip_vs_route_me_harder() can change the route after
snat_handler. So, for locally generated replies from
local server we can not preserve the CHECKSUM_PARTIAL
mode. It is an chicken or egg dilemma: snat_handler
needs the device after rerouting (to check for
NETIF_F_SCTP_CSUM), while ip_route_me_harder() wants
the snat_handler() to put the new saddr for proper
rerouting.
* For dnat_handler(), we should not see CHECKSUM_COMPLETE
for SCTP, in fact the small set of drivers that support
SCTP offloading return CHECKSUM_UNNECESSARY on correctly
received SCTP csum. We can see CHECKSUM_PARTIAL from
local stack or received from virtual drivers. The idea is
that SCTP decides to avoid csum calculation if hardware
supports offloading. IPVS can change the device after
rerouting to real server but we can preserve the
CHECKSUM_PARTIAL mode if the new device supports
offloading too. This works because skb dst is changed
before dnat_handler and we see the new device. So, checks
in the 'if' part will decide whether it is ok to keep
CHECKSUM_PARTIAL for the output. If the packet was with
CHECKSUM_NONE, hence we deal with unknown checksum. As we
recalculate the sum for IP header in all cases, it should
be safe to use CHECKSUM_UNNECESSARY. We can forward wrong
checksum in this case (without cp->app). In case of
CHECKSUM_UNNECESSARY, the csum was valid on receive.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_proto_sctp.c | 39 +++++++++++++++++++++++++++++------
1 file changed, 33 insertions(+), 6 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index 9ca7aa0..2f7ea75 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -81,6 +81,7 @@ sctp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
{
sctp_sctphdr_t *sctph;
unsigned int sctphoff = iph->len;
+ bool payload_csum = false;
#ifdef CONFIG_IP_VS_IPV6
if (cp->af == AF_INET6 && iph->fragoffs)
@@ -92,19 +93,31 @@ sctp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
return 0;
if (unlikely(cp->app != NULL)) {
+ int ret;
+
/* Some checks before mangling */
if (pp->csum_check && !pp->csum_check(cp->af, skb, pp))
return 0;
/* Call application helper if needed */
- if (!ip_vs_app_pkt_out(cp, skb))
+ ret = ip_vs_app_pkt_out(cp, skb);
+ if (ret == 0)
return 0;
+ /* ret=2: csum update is needed after payload mangling */
+ if (ret == 2)
+ payload_csum = true;
}
sctph = (void *) skb_network_header(skb) + sctphoff;
- sctph->source = cp->vport;
- sctp_nat_csum(skb, sctph, sctphoff);
+ /* Only update csum if we really have to */
+ if (sctph->source != cp->vport || payload_csum ||
+ skb->ip_summed == CHECKSUM_PARTIAL) {
+ sctph->source = cp->vport;
+ sctp_nat_csum(skb, sctph, sctphoff);
+ } else {
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ }
return 1;
}
@@ -115,6 +128,7 @@ sctp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
{
sctp_sctphdr_t *sctph;
unsigned int sctphoff = iph->len;
+ bool payload_csum = false;
#ifdef CONFIG_IP_VS_IPV6
if (cp->af == AF_INET6 && iph->fragoffs)
@@ -126,19 +140,32 @@ sctp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
return 0;
if (unlikely(cp->app != NULL)) {
+ int ret;
+
/* Some checks before mangling */
if (pp->csum_check && !pp->csum_check(cp->af, skb, pp))
return 0;
/* Call application helper if needed */
- if (!ip_vs_app_pkt_in(cp, skb))
+ ret = ip_vs_app_pkt_in(cp, skb);
+ if (ret == 0)
return 0;
+ /* ret=2: csum update is needed after payload mangling */
+ if (ret == 2)
+ payload_csum = true;
}
sctph = (void *) skb_network_header(skb) + sctphoff;
- sctph->dest = cp->dport;
- sctp_nat_csum(skb, sctph, sctphoff);
+ /* Only update csum if we really have to */
+ if (sctph->dest != cp->dport || payload_csum ||
+ (skb->ip_summed == CHECKSUM_PARTIAL &&
+ !(skb_dst(skb)->dev->features & NETIF_F_SCTP_CSUM))) {
+ sctph->dest = cp->dport;
+ sctp_nat_csum(skb, sctph, sctphoff);
+ } else if (skb->ip_summed != CHECKSUM_PARTIAL) {
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ }
return 1;
}
--
1.8.4
^ permalink raw reply related
* Re: [PATCH net-next 04/11] openvswitch: Restructure datapath.c and flow.c
From: Joe Perches @ 2013-10-30 1:25 UTC (permalink / raw)
To: Jesse Gross; +Cc: David Miller, netdev, dev, Pravin B Shelar
In-Reply-To: <1383092544-50599-5-git-send-email-jesse@nicira.com>
On Tue, 2013-10-29 at 17:22 -0700, Jesse Gross wrote:
> Over the time datapath.c and flow.c has became pretty large files.
> Following patch restructures functionality of component into three
> different components:
[]
> net/openvswitch/Makefile | 2 +
> net/openvswitch/datapath.c | 528 +------------
> net/openvswitch/datapath.h | 1 +
> net/openvswitch/flow.c | 1605 +---------------------------------------
> net/openvswitch/flow.h | 128 +---
> net/openvswitch/flow_netlink.c | 1603 +++++++++++++++++++++++++++++++++++++++
Is this largish patch any different or easier to
read/verify when generated with git format-patch -M ?
^ permalink raw reply
* Re: [PATCH net-next 04/11] openvswitch: Restructure datapath.c and flow.c
From: Joe Perches @ 2013-10-30 1:32 UTC (permalink / raw)
To: Jesse Gross
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
David Miller
In-Reply-To: <1383096313.12439.44.camel@joe-AO722>
On Tue, 2013-10-29 at 18:25 -0700, Joe Perches wrote:
> On Tue, 2013-10-29 at 17:22 -0700, Jesse Gross wrote:
> > Over the time datapath.c and flow.c has became pretty large files.
> > Following patch restructures functionality of component into three
> > different components:
> []
> > net/openvswitch/Makefile | 2 +
> > net/openvswitch/datapath.c | 528 +------------
> > net/openvswitch/datapath.h | 1 +
> > net/openvswitch/flow.c | 1605 +---------------------------------------
> > net/openvswitch/flow.h | 128 +---
> > net/openvswitch/flow_netlink.c | 1603 +++++++++++++++++++++++++++++++++++++++
>
> Is this largish patch any different or easier to
> read/verify when generated with git format-patch -M ?
After a git pull, the short answer is no.
Oh well.
^ permalink raw reply
* Re: [PATCH net-next] tipc: remove two indentation levels in tipc_recv_msg routine
From: Ying Xue @ 2013-10-30 1:34 UTC (permalink / raw)
To: David Laight, Andreas Bofjäll, Erik Hugne
Cc: jon.maloy, netdev, tipc-discussion, davem, Paul.Gortmaker
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73BD@saturn3.aculab.com>
On 10/29/2013 08:42 PM, David Laight wrote:
>> continue;
>> +
>> +unlock:
>> + tipc_node_unlock(n_ptr);
>> cont:
>> kfree_skb(buf);
>> }
>
> Might be better to call the labels 'unlock_discard' and 'discard'.
>
Thanks for your valuable suggestions. I will submit next version soon.
Regards,
Ying
> David
>
>
>
> --
> 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
>
>
------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
^ permalink raw reply
* Re: Bug in skb_segment: fskb->len != len
From: Herbert Xu @ 2013-10-30 1:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Christoph Paasch, netdev
In-Reply-To: <1383059293.5464.31.camel@edumazet-glaptop.roam.corp.google.com>
On Tue, Oct 29, 2013 at 08:08:13AM -0700, Eric Dumazet wrote:
>
> GRO layer was updated to be able to stack two or three sk_buff,
> fully populated with page frags.
>
> Thats quite mandatory to support line rate for 40Gb links.
Indeed I missed this. Which commit introduced this change?
Thanks,
--
Email: Herbert Xu <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
* Re: [PATCH v2 net-next] net: introduce gro_frag_list_enable sysctl
From: David Miller @ 2013-10-30 2:02 UTC (permalink / raw)
To: eric.dumazet; +Cc: christoph.paasch, herbert, netdev, hkchu, mwdalton
In-Reply-To: <1383094428.4857.16.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 29 Oct 2013 17:53:48 -0700
> So should we apply the first fix to avoid the BUG_ON() ?
Please be more specific, are you talking about splitting up
this patch in some way?
^ permalink raw reply
* Re: [PATCH v2 net-next] net: introduce gro_frag_list_enable sysctl
From: Herbert Xu @ 2013-10-30 2:05 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, christoph.paasch, netdev, hkchu, mwdalton
In-Reply-To: <20131029.220253.1263087684709722001.davem@davemloft.net>
On Tue, Oct 29, 2013 at 10:02:53PM -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 29 Oct 2013 17:53:48 -0700
>
> > So should we apply the first fix to avoid the BUG_ON() ?
>
> Please be more specific, are you talking about splitting up
> this patch in some way?
I think Eric is referring to the patch that removes the BUG_ON
in skb_segment and deals with the new mega-GRO packets.
I think that's fine for stable, but for the long term we should
fix it properly as these new meag-GRO packets still retain the
existing packet boundaries and are trivially segmentable.
If we are indeed able to do that, I doubt we would even need
the sysctl patch since the GRO performance should be vastly
superior to the non-GRO case, even for a router/bridge.
Cheers,
--
Email: Herbert Xu <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
* Re: [PATCH net-next 08/11] openvswitch: Per cpu flow stats.
From: David Miller @ 2013-10-30 2:11 UTC (permalink / raw)
To: jesse-l0M0P4e3n4LQT0dZR+AlfA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1383092544-50599-9-git-send-email-jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
From: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
Date: Tue, 29 Oct 2013 17:22:21 -0700
> From: Pravin B Shelar <pshelar-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
>
> With mega flow implementation ovs flow can be shared between
> multiple CPUs which makes stats updates highly contended
> operation. Following patch allocates separate stats for each
> CPU to make stats update scalable.
>
> Signed-off-by: Pravin B Shelar <pshelar-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
Please properly use __percpu memory for this.
Using arrays for this is very strongly discouraged, please instead
use the kernel facility designed exactly for this.
^ permalink raw reply
* Re: [PATCH v2 net-next] net: introduce gro_frag_list_enable sysctl
From: Jerry Chu @ 2013-10-30 2:13 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, Eric Dumazet, Christoph Paasch,
netdev@vger.kernel.org, Michael Dalton
In-Reply-To: <20131030020543.GA1925@gondor.apana.org.au>
On Tue, Oct 29, 2013 at 7:05 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Tue, Oct 29, 2013 at 10:02:53PM -0400, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Tue, 29 Oct 2013 17:53:48 -0700
>>
>> > So should we apply the first fix to avoid the BUG_ON() ?
>>
>> Please be more specific, are you talking about splitting up
>> this patch in some way?
>
> I think Eric is referring to the patch that removes the BUG_ON
> in skb_segment and deals with the new mega-GRO packets.
>
> I think that's fine for stable, but for the long term we should
> fix it properly as these new meag-GRO packets still retain the
> existing packet boundaries and are trivially segmentable.
>
> If we are indeed able to do that, I doubt we would even need
> the sysctl patch since the GRO performance should be vastly
> superior to the non-GRO case, even for a router/bridge.
Probably not the case for the simple forwarding case. See my
test result of some small (5-8%) CPU+throughput penalty from
GRO (over GRE tunnel) posted previously. But I can believe
the number may be very different if the forwarding path involves
more work (NAT, iptables filtering,...,etc) resulting in a higher per
pkt cost.
Best,
Jerry
>
> Cheers,
> --
> Email: Herbert Xu <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
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2013-10-30 2:14 UTC (permalink / raw)
To: David Miller, netdev
Cc: linux-next, linux-kernel, Nikolay Aleksandrov, Joe Perches
[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in
drivers/net/netconsole.c between commit c7c6effdeffc ("netconsole: fix
multiple race conditions") from the net tree and commit 22ded57729e6
("netconsole: Convert to pr_<level>") from the net-next tree.
(Thanks for removing that spare blank line :-))
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc drivers/net/netconsole.c
index c9a15925a1f7,a8ef4c4b94be..000000000000
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@@ -333,18 -336,14 +335,18 @@@ static ssize_t store_enabled(struct net
netpoll_print_options(&nt->np);
err = netpoll_setup(&nt->np);
- if (err) {
- mutex_unlock(&nt->mutex);
+ if (err)
return err;
- }
- printk(KERN_INFO "netconsole: network logging started\n");
+ pr_info("network logging started\n");
-
} else { /* 0 */
+ /* We need to disable the netconsole before cleaning it up
+ * otherwise we might end up in write_msg() with
+ * nt->np.dev == NULL and nt->enabled == 1
+ */
+ spin_lock_irqsave(&target_list_lock, flags);
+ nt->enabled = 0;
+ spin_unlock_irqrestore(&target_list_lock, flags);
netpoll_cleanup(&nt->np);
}
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v2 net-next] net: introduce gro_frag_list_enable sysctl
From: Herbert Xu @ 2013-10-30 2:19 UTC (permalink / raw)
To: Jerry Chu
Cc: David Miller, Eric Dumazet, Christoph Paasch,
netdev@vger.kernel.org, Michael Dalton
In-Reply-To: <CAPshTCgykFMa=4rnQ3TKa2cfgWYj4TgByO2GbzLzq71Y_=oc=Q@mail.gmail.com>
On Tue, Oct 29, 2013 at 07:13:50PM -0700, Jerry Chu wrote:
> On Tue, Oct 29, 2013 at 7:05 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> > If we are indeed able to do that, I doubt we would even need
> > the sysctl patch since the GRO performance should be vastly
> > superior to the non-GRO case, even for a router/bridge.
>
> Probably not the case for the simple forwarding case. See my
> test result of some small (5-8%) CPU+throughput penalty from
> GRO (over GRE tunnel) posted previously. But I can believe
> the number may be very different if the forwarding path involves
> more work (NAT, iptables filtering,...,etc) resulting in a higher per
> pkt cost.
Your numbers are with Eric's current patch that just linearises
the packet, what I'm saying is that you don't need to linearise
these packets since the packet boundaries are still there, just
hidden inside each frag_list.
Cheers,
--
Email: Herbert Xu <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
* [PATCH v2] bgmac: don't update slot on skb alloc/dma mapping error
From: Nathan Hintz @ 2013-10-30 2:32 UTC (permalink / raw)
To: netdev; +Cc: zajec5
Don't update the slot in "bgmac_dma_rx_skb_for_slot" unless both the
skb alloc and dma mapping are successful; and free the newly allocated
skb if a dma mapping error occurs. This relieves the caller of the need
to deduce/execute the appropriate cleanup action required when an error
occurs.
Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
Acked-by: Rafał Miłecki <zajec5@gmail.com>
---
v2: Revised commit message
---
drivers/net/ethernet/broadcom/bgmac.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 7eca5a1..11e5d8d 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -252,25 +252,33 @@ static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
struct bgmac_slot_info *slot)
{
struct device *dma_dev = bgmac->core->dma_dev;
+ struct sk_buff *skb;
+ dma_addr_t dma_addr;
struct bgmac_rx_header *rx;
/* Alloc skb */
- slot->skb = netdev_alloc_skb(bgmac->net_dev, BGMAC_RX_BUF_SIZE);
- if (!slot->skb)
+ skb = netdev_alloc_skb(bgmac->net_dev, BGMAC_RX_BUF_SIZE);
+ if (!skb)
return -ENOMEM;
/* Poison - if everything goes fine, hardware will overwrite it */
- rx = (struct bgmac_rx_header *)slot->skb->data;
+ rx = (struct bgmac_rx_header *)skb->data;
rx->len = cpu_to_le16(0xdead);
rx->flags = cpu_to_le16(0xbeef);
/* Map skb for the DMA */
- slot->dma_addr = dma_map_single(dma_dev, slot->skb->data,
- BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
- if (dma_mapping_error(dma_dev, slot->dma_addr)) {
+ dma_addr = dma_map_single(dma_dev, skb->data,
+ BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
+ if (dma_mapping_error(dma_dev, dma_addr)) {
bgmac_err(bgmac, "DMA mapping error\n");
+ dev_kfree_skb(skb);
return -ENOMEM;
}
+
+ /* Update the slot */
+ slot->skb = skb;
+ slot->dma_addr = dma_addr;
+
if (slot->dma_addr & 0xC0000000)
bgmac_warn(bgmac, "DMA address using 0xC0000000 bit(s), it may need translation trick\n");
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: introduce gro_frag_list_enable sysctl
From: David Miller @ 2013-10-30 2:33 UTC (permalink / raw)
To: hkchu; +Cc: herbert, eric.dumazet, christoph.paasch, netdev, mwdalton
In-Reply-To: <CAPshTCgykFMa=4rnQ3TKa2cfgWYj4TgByO2GbzLzq71Y_=oc=Q@mail.gmail.com>
From: Jerry Chu <hkchu@google.com>
Date: Tue, 29 Oct 2013 19:13:50 -0700
> On Tue, Oct 29, 2013 at 7:05 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>> On Tue, Oct 29, 2013 at 10:02:53PM -0400, David Miller wrote:
>>> From: Eric Dumazet <eric.dumazet@gmail.com>
>>> Date: Tue, 29 Oct 2013 17:53:48 -0700
>>>
>>> > So should we apply the first fix to avoid the BUG_ON() ?
>>>
>>> Please be more specific, are you talking about splitting up
>>> this patch in some way?
>>
>> I think Eric is referring to the patch that removes the BUG_ON
>> in skb_segment and deals with the new mega-GRO packets.
>>
>> I think that's fine for stable, but for the long term we should
>> fix it properly as these new meag-GRO packets still retain the
>> existing packet boundaries and are trivially segmentable.
>>
>> If we are indeed able to do that, I doubt we would even need
>> the sysctl patch since the GRO performance should be vastly
>> superior to the non-GRO case, even for a router/bridge.
>
> Probably not the case for the simple forwarding case. See my
> test result of some small (5-8%) CPU+throughput penalty from
> GRO (over GRE tunnel) posted previously. But I can believe
> the number may be very different if the forwarding path involves
> more work (NAT, iptables filtering,...,etc) resulting in a higher per
> pkt cost.
It's that way because it's not implemented properly.
GRO should always win, even on a router, because it decreases the
number of fundamental operations (routing lookups) that the stack
needs to perform.
^ permalink raw reply
* Re: [PATCH v2 net-next] net: introduce gro_frag_list_enable sysctl
From: David Miller @ 2013-10-30 2:34 UTC (permalink / raw)
To: herbert; +Cc: hkchu, eric.dumazet, christoph.paasch, netdev, mwdalton
In-Reply-To: <20131030021905.GA2089@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed, 30 Oct 2013 10:19:06 +0800
> On Tue, Oct 29, 2013 at 07:13:50PM -0700, Jerry Chu wrote:
>> On Tue, Oct 29, 2013 at 7:05 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>>
>> > If we are indeed able to do that, I doubt we would even need
>> > the sysctl patch since the GRO performance should be vastly
>> > superior to the non-GRO case, even for a router/bridge.
>>
>> Probably not the case for the simple forwarding case. See my
>> test result of some small (5-8%) CPU+throughput penalty from
>> GRO (over GRE tunnel) posted previously. But I can believe
>> the number may be very different if the forwarding path involves
>> more work (NAT, iptables filtering,...,etc) resulting in a higher per
>> pkt cost.
>
> Your numbers are with Eric's current patch that just linearises
> the packet, what I'm saying is that you don't need to linearise
> these packets since the packet boundaries are still there, just
> hidden inside each frag_list.
Agreed.
^ permalink raw reply
* Re: [PATCH] net/cdc_ncm: fix null pointer panic at usbnet_link_change
From: David Miller @ 2013-10-30 2:38 UTC (permalink / raw)
To: changbinx.du; +Cc: oliver, linux-usb, netdev, linux-kernel
In-Reply-To: <0C18FE92A7765D4EB9EE5D38D86A563A019F450F@SHSMSX103.ccr.corp.intel.com>
From: "Du, ChangbinX" <changbinx.du@intel.com>
Date: Tue, 29 Oct 2013 03:30:42 +0000
> In cdc_ncm_bind() function, it call cdc_ncm_bind_common() to setup usb.
> But cdc_ncm_bind_common() may meet error and cause usbnet_disconnect()
> be called which calls free_netdev(net). Thus usbnet structure(alloced
> with net_device structure) will be freed,too.
> So we cannot call usbnet_link_change() if cdc_ncm_bind_common() return
> error.
This is not the bug.
The problem is in cdc_ncm_bind_common().
It seems to leave dangling interface data pointers in some cases, and
then branches just to "error" so that they don't get cleared back out.
This bypasses the protection present in cdc_ncm_disconnect() meant to
avoid this problem.
^ permalink raw reply
* Re: [PATCH] bnx2: Use dev_kfree_skb_any() in bnx2_tx_int()
From: David Miller @ 2013-10-30 2:42 UTC (permalink / raw)
To: tdmackey; +Cc: mchan, netdev, linux-kernel
In-Reply-To: <1383084998-22673-1-git-send-email-tdmackey@booleanhaiku.com>
From: David Mackey <tdmackey@booleanhaiku.com>
Date: Tue, 29 Oct 2013 15:16:38 -0700
> Using dev_kfree_skb_any() will resolve the below issue when a
> netconsole message is transmitted in an irq.
...
> Signed-off-by: David Mackey <tdmackey@booleanhaiku.com>
This is absolutely not the correct fix.
The netpoll facility must invoke ->poll() in an environment which
is compatible, locking and interrupt/soft-interrupt wise, as that
in which it is normally called.
Therefore, bnx2_tx_int(), which is invoked from the driver's ->poll()
method, should not need to use dev_kfree_skb_any(). The real problem
is somewhere else.
^ permalink raw reply
* Re: [PATCH net] virtio-net: correctly handle cpu hotplug notifier during resuming
From: David Miller @ 2013-10-30 2:44 UTC (permalink / raw)
To: jasowang; +Cc: mst, netdev, linux-kernel, virtualization
In-Reply-To: <1383030667-14343-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Tue, 29 Oct 2013 15:11:07 +0800
> commit 3ab098df35f8b98b6553edc2e40234af512ba877 (virtio-net: don't respond to
> cpu hotplug notifier if we're not ready) tries to bypass the cpu hotplug
> notifier by checking the config_enable and does nothing is it was false. So it
> need to try to hold the config_lock mutex which may happen in atomic
> environment which leads the following warnings:
...
> A correct fix is to unregister the hotcpu notifier during restore and register a
> new one in resume.
>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Tested-by: Fengguang Wu <fengguang.wu@intel.com>
> Cc: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> This patch is needed for 3.8 and above.
Applied and queued up for -stable, thanks Jason.
^ permalink raw reply
* Re: [PATCH net-next] tcp: temporarily disable Fast Open on SYN timeout
From: David Miller @ 2013-10-30 2:51 UTC (permalink / raw)
To: ycheng; +Cc: ncardwell, edumazet, netdev
In-Reply-To: <1383066545-27348-1-git-send-email-ycheng@google.com>
From: Yuchung Cheng <ycheng@google.com>
Date: Tue, 29 Oct 2013 10:09:05 -0700
> Fast Open currently has a fall back feature to address SYN-data
> being dropped by but it requires the middle-box to pass on regular
> SYN retry after SYN-data. This is implemented in commit aab487435
> ("net-tcp: Fast Open client - detecting SYN-data drops")
>
> However some NAT boxes will drop all subsequent packets after first
> SYN-data and blackholes the entire connections. An example is incommit
> 356d7d8 "netfilter: nf_conntrack: fix tcp_in_window for Fast Open".
>
> The sender should note such incidents and falls back to use regular
> TCP handshake on subsequent attempt temporarily as well: after the
> second SYN timeouts the original Fast Open SYN is most likely lost.
> When such an event recurs Fast Open is disabled based on the number
> of recurrences exponentially.
>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v2] octeon_mgmt: drop redundant mac address check
From: David Miller @ 2013-10-30 2:53 UTC (permalink / raw)
To: luka; +Cc: netdev
In-Reply-To: <1383088152-24869-1-git-send-email-luka@openwrt.org>
From: Luka Perkov <luka@openwrt.org>
Date: Wed, 30 Oct 2013 00:09:12 +0100
> Checking if MAC address is valid using is_valid_ether_addr() is already done in
> of_get_mac_address().
>
> Signed-off-by: Luka Perkov <luka@openwrt.org>
> Acked-by: David Daney <david.daney@cavium.com>
> CC: David Miller <davem@davemloft.net>
Applied.
^ permalink raw reply
* Re: [PATCH v2] mvneta: drop redundant mac address check
From: David Miller @ 2013-10-30 2:53 UTC (permalink / raw)
To: luka; +Cc: netdev
In-Reply-To: <1383088201-25136-1-git-send-email-luka@openwrt.org>
From: Luka Perkov <luka@openwrt.org>
Date: Wed, 30 Oct 2013 00:10:01 +0100
> Checking if MAC address is valid using is_valid_ether_addr() is already done in
> of_get_mac_address().
>
> Signed-off-by: Luka Perkov <luka@openwrt.org>
> Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Applied.
^ 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