* Re: [PATCH v3] net: Do not include padding in TCP GRO checksum
From: David Miller @ 2013-11-16 1:34 UTC (permalink / raw)
To: herbert; +Cc: alexander.h.duyck, netdev, edumazet, herbert
In-Reply-To: <20131116004738.GA1491@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sat, 16 Nov 2013 08:47:38 +0800
> Also, if the padding occurs on every single packet or a fairly
> large amount of packets then you should consider improving
> pskb_trim_rcsum to keep the CHECKSUM_COMPLETE value.
It should be as easy as this? (this is actually just a 3 line change,
it looks like more because I'm moving pskb_trim_rcsum() later in the
file to where skb_checksum() is in scope).
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 215b5ea..bec1cc7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2263,24 +2263,6 @@ static inline void skb_postpull_rcsum(struct sk_buff *skb,
unsigned char *skb_pull_rcsum(struct sk_buff *skb, unsigned int len);
-/**
- * pskb_trim_rcsum - trim received skb and update checksum
- * @skb: buffer to trim
- * @len: new length
- *
- * This is exactly the same as pskb_trim except that it ensures the
- * checksum of received packets are still valid after the operation.
- */
-
-static inline int pskb_trim_rcsum(struct sk_buff *skb, unsigned int len)
-{
- if (likely(len >= skb->len))
- return 0;
- if (skb->ip_summed == CHECKSUM_COMPLETE)
- skb->ip_summed = CHECKSUM_NONE;
- return __pskb_trim(skb, len);
-}
-
#define skb_queue_walk(queue, skb) \
for (skb = (queue)->next; \
skb != (struct sk_buff *)(queue); \
@@ -2378,6 +2360,27 @@ __wsum __skb_checksum(const struct sk_buff *skb, int offset, int len,
__wsum skb_checksum(const struct sk_buff *skb, int offset, int len,
__wsum csum);
+/**
+ * pskb_trim_rcsum - trim received skb and update checksum
+ * @skb: buffer to trim
+ * @len: new length
+ *
+ * This is exactly the same as pskb_trim except that it ensures the
+ * checksum of received packets are still valid after the operation.
+ */
+
+static inline int pskb_trim_rcsum(struct sk_buff *skb, unsigned int len)
+{
+ if (likely(len >= skb->len))
+ return 0;
+ if (skb->ip_summed == CHECKSUM_COMPLETE) {
+ __wsum adj = skb_checksum(skb, len, skb->len - len, 0);
+
+ skb->csum = csum_sub(skb->csum, adj);
+ }
+ return __pskb_trim(skb, len);
+}
+
static inline void *skb_header_pointer(const struct sk_buff *skb, int offset,
int len, void *buffer)
{
^ permalink raw reply related
* Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
From: Ben Hutchings @ 2013-11-16 1:38 UTC (permalink / raw)
To: David Laight
Cc: Alan Stern, Sarah Sharp, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B741A-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
On Wed, 2013-11-13 at 16:58 +0000, David Laight wrote:
[...]
> > > > > You can split a bulk TD on a 1k boundary and the target won't know the
> > > > > difference.
> > > >
> > > > The target won't know the difference, but the host will. Here's an
> > > > example: Suppose the driver submits two URBs, each for a data-in
> > > > transfer of 32 KB. You split each URB up into two 16-KB TDs; let's
> > > > call them A, B, C, and D (where A and B make up the first URB, and C
> > > > and D make up the second URB).
> > >
> > > I was thinking about OUT transfers, IN ones are unlikely to be badly
> > > fragmented.
> >
> > Maybe not for the network stack, but OUT and IN end up equally
> > fragmented for the storage stack.
>
> But the minimum fragment size is (probably) 4k.
> For the network stack an OUT transfer might have a lot (and I mean lots)
> of fragments (there may be constraints, and linearising the skb is a option).
[...]
The maximum number of fragments in the skb is going to be 17 (including
the 'head' area). (I'm ignoring NETIF_F_FRAGLIST which is not normally
supported by physical device drivers.)
I don't know how many fragments that can end up as, at the USB level.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net] bonding: don't change to 802.3ad mode while ARP monitor is running
From: David Miller @ 2013-11-16 1:40 UTC (permalink / raw)
To: dingtianhong; +Cc: fubar, andy, nikolay, vfalico, netdev
In-Reply-To: <5285F12A.6030307@huawei.com>
From: Ding Tianhong <dingtianhong@huawei.com>
Date: Fri, 15 Nov 2013 18:02:18 +0800
> Because the ARP monitor is not support for 802.3ad, but We still
^^^^^^^
"supported"
No need to capitalize "We".
> could change the mode to 802.3ad from ab mode while ARP monirot
^^^^^^^
"monitor"
> is running, it is incorrect.
>
> So add a check for 802.3ad in bonding_store_mode to fix the problem,
> and make a micro program to simplify the code.
What is a "micro program"?
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: Update Pegasus and RTL8150 driver repositories;
From: David Miller @ 2013-11-16 1:42 UTC (permalink / raw)
To: petkan; +Cc: linux-kernel, netdev
In-Reply-To: <20131115102344.GA8544@bender.nucleusys.com>
From: Petko Manolov <petkan@nucleusys.com>
Date: Fri, 15 Nov 2013 12:23:44 +0200
> From: Petko Manolov <petkan@nucleusys.com>
> Date: Fri, 15 Nov 2013 11:51:54 +0200
> Subject: [PATCH] Update Pegasus and RTL8150 repositories;
Do not include this metadata in your patch postings, it duplicates
the commit header line when people try to apply it using "git am"
> Update Pegasus and RTL8150 driver repositories;
>
> The diff is against latest 'net' repository;
>
> Signed-off-by: Petko Manolov <petkan@nucleusys.com>
Please submit networking patches to netdev@vger.kernel.org in the
future, I happened to catch this and applied it this time.
Thanks.
^ permalink raw reply
* Re: [PATCH v3] net: Do not include padding in TCP GRO checksum
From: Herbert Xu @ 2013-11-16 1:43 UTC (permalink / raw)
To: David Miller; +Cc: alexander.h.duyck, netdev, edumazet, herbert
In-Reply-To: <20131115.203427.1522242353491609372.davem@davemloft.net>
On Fri, Nov 15, 2013 at 08:34:27PM -0500, David Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Sat, 16 Nov 2013 08:47:38 +0800
>
> > Also, if the padding occurs on every single packet or a fairly
> > large amount of packets then you should consider improving
> > pskb_trim_rcsum to keep the CHECKSUM_COMPLETE value.
>
> It should be as easy as this? (this is actually just a 3 line change,
> it looks like more because I'm moving pskb_trim_rcsum() later in the
> file to where skb_checksum() is in scope).
Looks good to me. 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] genetlink: unify registration functions
From: David Miller @ 2013-11-16 1:52 UTC (permalink / raw)
To: johannes; +Cc: netdev, johannes.berg
In-Reply-To: <1384521548-13241-1-git-send-email-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 15 Nov 2013 14:19:08 +0100
> From: Johannes Berg <johannes.berg@intel.com>
>
> Now that the ops assignment is just two variables rather than a
> long list iteration etc., there's no reason to separately export
> __genl_register_family() and __genl_register_family_with_ops().
>
> Unify the two functions into __genl_register_family() and make
> genl_register_family_with_ops() call it after assigning the ops.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Looks great, applied.
As a future simplification perhaps we can even elide the array length
argument. Most pass ARRAY_SIZE(foo) but there is at least one case
(DLM) which passes a constant, which is error prone.
#define genl_register_family_with_ops(family, ops)
__genl_register_family_with_ops(family, ops, ARRAY_SIZE(ops))
something like that.
^ permalink raw reply
* Re: [PATCH v3] net: Do not include padding in TCP GRO checksum
From: Herbert Xu @ 2013-11-16 1:53 UTC (permalink / raw)
To: Alexander Duyck; +Cc: davem, netdev, edumazet, herbert
In-Reply-To: <20131116004738.GA1491@gondor.apana.org.au>
On Sat, Nov 16, 2013 at 08:47:38AM +0800, Herbert Xu wrote:
> On Fri, Nov 15, 2013 at 03:00:34PM -0800, Alexander Duyck wrote:
> > In some recent tests I found the TCP checksum was being treated as valid
> > for certain frames with padding on them. On closer inspection I found the
> > issue was that GRO was using the skb->len instead of the length recorded in
> > the IP/IPv6 header to determine the number of bytes to checksum. As such
> > padded frames that actually had invalid checksums generated by adding the
> > padding to the checksum were being incorrectly tagged as valid.
> >
> > This change corrects that by using the tot_len from IPv4 headers and the
> > payload_len from IPv6 headers to compute the total number of bytes to be
> > included in the checksum.
> >
> > To address the fact that skb->csum is invalid when a padded frame is
> > received I have updated the code to fall though to the CHECKSUM_NONE path
> > for CHECKSUM_COMPLETE frames that contain padding.
> >
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>
> Nack. This is needlessly complex. As I said before, please
> just do a pskb_trim_rcsum in inet_gro_receive and its IPv6
> counterpart and this should all just work.
Actually I take that back. You are right that the preference is to
flush and not trim, since we want to preserve the incoming packet in
its exact form.
So can you tell me a bit more about the padding? Is it added by the
NIC or did it come from the remote side?
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 0/9] genetlink: reduce ops size and complexity (v2)
From: David Miller @ 2013-11-16 1:53 UTC (permalink / raw)
To: johannes
Cc: netdev, linux-wimax, bsingharora, netfilter-devel,
alex.bluesman.smirnov, dbaryshkov
In-Reply-To: <1384521515.14295.2.camel@jlt4.sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 15 Nov 2013 14:18:35 +0100
> We could instead register an array of pointers to the groups:
>
> static const struct mcast_group *my_groups[] = {
> &my_foo_mcast_group,
> ...
> };
>
> and pass this to the family - that'd still be less space (one pointer
> for each group rather than two in a linked list) and still allow all
> groups and this array to be const, but it's not quite as big a
> saving ...
>
> Thoughts?
This idea sounds fine. I don't even thing the array indexing is
odd, especially if we can have named mnenomics for the indices
or similar.
^ permalink raw reply
* Re: [PATCH 0/9] genetlink: reduce ops size and complexity (v2)
From: David Miller @ 2013-11-16 1:54 UTC (permalink / raw)
To: johannes
Cc: netdev, linux-wimax, bsingharora, netfilter-devel,
alex.bluesman.smirnov, dbaryshkov
In-Reply-To: <1384521794.14295.5.camel@jlt4.sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 15 Nov 2013 14:23:14 +0100
> On Fri, 2013-11-15 at 14:18 +0100, Johannes Berg wrote:
>
>> I've been eyeing the multicast groups as well
>
> Of course, there are also *much* fewer mcast groups, so the saving isn't
> nearly as big. But we still have some oddball code to register them all,
> basically
>
> err = register_mc_group();
> if (err)
> goto unregister_family;
>
> (since we have to register the family first, afaict)
>
> sometimes a few of those back to back, and it'd be nicer on the users if
> that just went away and was
>
> family.mcast_groups = my_groups;
> family.n_mcast_groups = ARRAY_SIZE(my_groups);
> family.ops = my_ops;
> family.n_ops = ARRAY_SIZE(my_ops);
> return genl_register_family(&family);
>
> since nobody uses them dynamically anyway, afaict.
See my other email:
#define genl_register_family_with_ops_mcast(family, ops, groups) \
__genl_register_family(family, ops, ARRAY_SIZE(ops), groups, ARRAY_SIZE(groups))
you get the idea.
^ permalink raw reply
* Re: [PATCH 1/4] net/phy: Add VSC8234 support
From: David Miller @ 2013-11-16 1:58 UTC (permalink / raw)
To: Shruti; +Cc: netdev, afleming, galak
In-Reply-To: <1384556305-17133-1-git-send-email-Shruti@Freescale.com>
These patches didn't actually make it to the netdev list, so they
were not queued up in patchwork.
^ permalink raw reply
* Re: [PATCH] sh_eth: call phy_scan_fixups() after PHY reset
From: David Miller @ 2013-11-16 2:00 UTC (permalink / raw)
To: f.fainelli
Cc: sergei.shtylyov, nobuhiro.iwamatsu.yj, netdev, linux-sh,
laurent.pinchart+renesas
In-Reply-To: <CAGVrzcaKA7T7Cppg0g29wsUZBu4-6+AY+6teH1SWSiT=D7RQBQ@mail.gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 15 Nov 2013 16:04:52 -0800
> Quite a lot of PHYs actually require a reset through BMCR_RESET when
> resuming from S2/S3 sleep states and even if they do not, it does not
> hurt doing so, I would welcome a generic solution to do that which
> would not circumvent the libphy state machine and APIs.
> idea would be to:
There was recently a patch I had to reject which had the same problem,
it wanted to BMCR_RESET the PHY when the link goes down to avoid a
problem with the hardware that causes a hang.
I brought up the same objection you are here, namely that resetting
the chip puts the hardware and phylib out of sync wrt. what is
configured into the chip.
So the generic solution you suggest here could help that driver
achieve what it wants too.
^ permalink raw reply
* Re: [PATCH net] bonding: set primary_reselect in LB and AB mode
From: Ding Tianhong @ 2013-11-16 1:50 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Ding Tianhong, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
In-Reply-To: <4031.1384538565@death.nxdomain>
于 2013/11/16 2:02, Jay Vosburgh 写道:
> Ding Tianhong <dingtianhong@huawei.com> wrote:
>
>> The primary_reselect only reselection for the primary slave,
>> but the primary slave only support for ALB, TLB and AB mode,
>> so we sould set the primary_reselect for these mode.
>>
>> to fix this: Add a check for ALB, TLB and AB mode in
>> bonding_store_primary_reselect, avoid to select active slave
>> again in other modes.
> I don't believe that setting primary_reselect in a
> !USES_PRIMARY() mode has any negative effects. It doesn't do anything,
> but also doesn't break anything. Is there a case that setting
> primary_reselect causes misbehavior in a !USES_PRIMARY() mode?
I see it will call bond_select_active_slave and I will think more about it,
if I could not find any problem about it, will miss this patch.
thanks.
Regards
Ding
> Presuming that primary_reselect doesn't break things, this just
> adds an ordering limitation when configuring bonding (mode must be set
> prior to primary_reselect). I don't believe this change adds any value,
> and may break existing configuration scripts.
>
> -J
>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_sysfs.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>> index 47749c9..cac2291 100644
>> --- a/drivers/net/bonding/bond_sysfs.c
>> +++ b/drivers/net/bonding/bond_sysfs.c
>> @@ -1141,6 +1141,13 @@ static ssize_t bonding_store_primary_reselect(struct device *d,
>> if (!rtnl_trylock())
>> return restart_syscall();
>>
>> + if (!USES_PRIMARY(bond->params.mode)) {
>> + pr_info("%s: Unable to set primary reselect; %s is in mode %d\n",
>> + bond->dev->name, bond->dev->name, bond->params.mode);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> new_value = bond_parse_parm(buf, pri_reselect_tbl);
>> if (new_value < 0) {
>> pr_err("%s: Ignoring invalid primary_reselect value %.*s.\n",
>> --
>> 1.7.12
> ---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
>
> --
> 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] bonding: don't change to 802.3ad mode while ARP monitor is running
From: Ding Tianhong @ 2013-11-16 1:52 UTC (permalink / raw)
To: David Miller; +Cc: dingtianhong, fubar, andy, nikolay, vfalico, netdev
In-Reply-To: <20131115.204042.1434943107870815286.davem@davemloft.net>
于 2013/11/16 9:40, David Miller 写道:
> From: Ding Tianhong <dingtianhong@huawei.com>
> Date: Fri, 15 Nov 2013 18:02:18 +0800
>
>> Because the ARP monitor is not support for 802.3ad, but We still
> ^^^^^^^
>
> "supported"
>
> No need to capitalize "We".
>
>> could change the mode to 802.3ad from ab mode while ARP monirot
> ^^^^^^^
>
> "monitor"
>
>> is running, it is incorrect.
>>
>> So add a check for 802.3ad in bonding_store_mode to fix the problem,
>> and make a micro program to simplify the code.
> What is a "micro program"?
sorry, not review the changelog yet,I will resend it and it will not be
happend again.
Regards
Ding
> --
> 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 1/4] net/phy: Add VSC8234 support
From: Florian Fainelli @ 2013-11-16 2:04 UTC (permalink / raw)
To: David Miller; +Cc: Shruti, netdev, afleming, galak
In-Reply-To: <20131115.205843.1202324001274425714.davem@davemloft.net>
2013/11/15 David Miller <davem@davemloft.net>:
>
> These patches didn't actually make it to the netdev list, so they
> were not queued up in patchwork.
Are you sure about them not making it to netdev? marc.info also has
the 4 of them here:
http://marc.info/?l=linux-netdev&m=138455632228947&w=2
http://marc.info/?l=linux-netdev&m=138455632828949&w=2
http://marc.info/?l=linux-netdev&m=138455633228950&w=2
http://marc.info/?l=linux-netdev&m=138455633628952&w=2
--
Florian
^ permalink raw reply
* Re: [PATCH 1/2] pkt_sched: fq: warn users using defrate
From: David Miller @ 2013-11-16 2:06 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, maze, willemb, ycheng, ncardwell
In-Reply-To: <1384534646.28716.75.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 15 Nov 2013 08:57:26 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> Commit 7eec4174ff29 ("pkt_sched: fq: fix non TCP flows pacing")
> obsoleted TCA_FQ_FLOW_DEFAULT_RATE without notice for the users.
>
> Suggested by David Miller
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH 2/2] pkt_sched: fq: fix pacing for small frames
From: David Miller @ 2013-11-16 2:06 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, maze, willemb, ycheng, ncardwell
In-Reply-To: <1384534694.28716.77.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 15 Nov 2013 08:58:14 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> For performance reasons, sch_fq tried hard to not setup timers for every
> sent packet, using a quantum based heuristic : A delay is setup only if
> the flow exhausted its credit.
>
> Problem is that application limited flows can refill their credit
> for every queued packet, and they can evade pacing.
>
> This problem can also be triggered when TCP flows use small MSS values,
> as TSO auto sizing builds packets that are smaller than the default fq
> quantum (3028 bytes)
>
> This patch adds a 40 ms delay to guard flow credit refill.
>
> Fixes: afe4fd062416 ("pkt_sched: fq: Fair Queue packet scheduler")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied and queued up for -stable.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/4] net/phy: Add VSC8234 support
From: David Miller @ 2013-11-16 2:09 UTC (permalink / raw)
To: f.fainelli; +Cc: Shruti, netdev, afleming, galak
In-Reply-To: <CAGVrzcZmwQMcpk2MXDfq07NEnxYs+UF4016Z_CezMOKVix-9BQ@mail.gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 15 Nov 2013 18:04:36 -0800
> 2013/11/15 David Miller <davem@davemloft.net>:
>>
>> These patches didn't actually make it to the netdev list, so they
>> were not queued up in patchwork.
>
> Are you sure about them not making it to netdev? marc.info also has
> the 4 of them here:
>
> http://marc.info/?l=linux-netdev&m=138455632228947&w=2
> http://marc.info/?l=linux-netdev&m=138455632828949&w=2
> http://marc.info/?l=linux-netdev&m=138455633228950&w=2
> http://marc.info/?l=linux-netdev&m=138455633628952&w=2
More specifically I meant that they didn't end up in patchwork,
which is what actually matters for my workflow.
^ permalink raw reply
* Re: [PATCH v3] net: Do not include padding in TCP GRO checksum
From: David Miller @ 2013-11-16 2:11 UTC (permalink / raw)
To: herbert; +Cc: alexander.h.duyck, netdev, edumazet, herbert
In-Reply-To: <20131116014350.GA1895@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sat, 16 Nov 2013 09:43:50 +0800
> On Fri, Nov 15, 2013 at 08:34:27PM -0500, David Miller wrote:
>> From: Herbert Xu <herbert@gondor.apana.org.au>
>> Date: Sat, 16 Nov 2013 08:47:38 +0800
>>
>> > Also, if the padding occurs on every single packet or a fairly
>> > large amount of packets then you should consider improving
>> > pskb_trim_rcsum to keep the CHECKSUM_COMPLETE value.
>>
>> It should be as easy as this? (this is actually just a 3 line change,
>> it looks like more because I'm moving pskb_trim_rcsum() later in the
>> file to where skb_checksum() is in scope).
>
> Looks good to me. Thanks!
Cool, I committed it as follows:
====================
[PATCH] net: Handle CHECKSUM_COMPLETE more adequately in pskb_trim_rcsum().
Currently pskb_trim_rcsum() just balks on CHECKSUM_COMPLETE packets
and remarks them as CHECKSUM_NONE, forcing a software checksum
validation later.
We have all of the mechanics available to fixup the skb->csum value,
even for complicated fragmented packets, via the helpers
skb_checksum() and csum_sub().
So just use them.
Based upon a suggestion by Herbert Xu.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/linux/skbuff.h | 39 +++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 215b5ea..bec1cc7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2263,24 +2263,6 @@ static inline void skb_postpull_rcsum(struct sk_buff *skb,
unsigned char *skb_pull_rcsum(struct sk_buff *skb, unsigned int len);
-/**
- * pskb_trim_rcsum - trim received skb and update checksum
- * @skb: buffer to trim
- * @len: new length
- *
- * This is exactly the same as pskb_trim except that it ensures the
- * checksum of received packets are still valid after the operation.
- */
-
-static inline int pskb_trim_rcsum(struct sk_buff *skb, unsigned int len)
-{
- if (likely(len >= skb->len))
- return 0;
- if (skb->ip_summed == CHECKSUM_COMPLETE)
- skb->ip_summed = CHECKSUM_NONE;
- return __pskb_trim(skb, len);
-}
-
#define skb_queue_walk(queue, skb) \
for (skb = (queue)->next; \
skb != (struct sk_buff *)(queue); \
@@ -2378,6 +2360,27 @@ __wsum __skb_checksum(const struct sk_buff *skb, int offset, int len,
__wsum skb_checksum(const struct sk_buff *skb, int offset, int len,
__wsum csum);
+/**
+ * pskb_trim_rcsum - trim received skb and update checksum
+ * @skb: buffer to trim
+ * @len: new length
+ *
+ * This is exactly the same as pskb_trim except that it ensures the
+ * checksum of received packets are still valid after the operation.
+ */
+
+static inline int pskb_trim_rcsum(struct sk_buff *skb, unsigned int len)
+{
+ if (likely(len >= skb->len))
+ return 0;
+ if (skb->ip_summed == CHECKSUM_COMPLETE) {
+ __wsum adj = skb_checksum(skb, len, skb->len - len, 0);
+
+ skb->csum = csum_sub(skb->csum, adj);
+ }
+ return __pskb_trim(skb, len);
+}
+
static inline void *skb_header_pointer(const struct sk_buff *skb, int offset,
int len, void *buffer)
{
--
1.7.11.7
^ permalink raw reply related
* [PATCH 1/2] net: ipv6: af_inet6: Fix warning when CONFIG_SYSCTL=n
From: Fabio Estevam @ 2013-11-16 2:52 UTC (permalink / raw)
To: davem; +Cc: netdev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
When CONFIG_SYSCTL=n the following build warning happens:
net/ipv6/af_inet6.c:710:13: warning: 'ipv6_packet_cleanup' defined but not used [-Wunused-function]
ipv6_packet_cleanup() is only used when CONFIG_SYSCTL=y, so protect its
definition with an'ifdef CONFIG_SYSCTL'.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
net/ipv6/af_inet6.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index ff75313..e4ad65c 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -707,10 +707,12 @@ static int __init ipv6_packet_init(void)
return 0;
}
+#ifdef CONFIG_SYSCTL
static void ipv6_packet_cleanup(void)
{
dev_remove_pack(&ipv6_packet_type);
}
+#endif
static int __net_init ipv6_init_mibs(struct net *net)
{
--
1.8.1.2
^ permalink raw reply related
* [PATCH 2/2] net: ipv6: ndisc: Fix warning when CONFIG_SYSCTL=n
From: Fabio Estevam @ 2013-11-16 2:52 UTC (permalink / raw)
To: davem; +Cc: netdev, Fabio Estevam
In-Reply-To: <1384570328-21666-1-git-send-email-festevam@gmail.com>
From: Fabio Estevam <fabio.estevam@freescale.com>
When CONFIG_SYSCTL=n the following build warning happens:
net/ipv6/ndisc.c:1730:1: warning: label 'out' defined but not used [-Wunused-label]
The 'out' label is only used when CONFIG_SYSCTL=y, so move it inside the
'ifdef CONFIG_SYSCTL' block.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
net/ipv6/ndisc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index f8a55ff..3512177 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1726,8 +1726,8 @@ int __init ndisc_init(void)
&ndisc_ifinfo_sysctl_change);
if (err)
goto out_unregister_pernet;
-#endif
out:
+#endif
return err;
#ifdef CONFIG_SYSCTL
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH v3] net: Do not include padding in TCP GRO checksum
From: Alexander Duyck @ 2013-11-16 4:10 UTC (permalink / raw)
To: Herbert Xu, Alexander Duyck; +Cc: davem, netdev, edumazet, herbert
In-Reply-To: <20131116015301.GA1999@gondor.apana.org.au>
On 11/15/2013 05:53 PM, Herbert Xu wrote:
> On Sat, Nov 16, 2013 at 08:47:38AM +0800, Herbert Xu wrote:
>> On Fri, Nov 15, 2013 at 03:00:34PM -0800, Alexander Duyck wrote:
>>> In some recent tests I found the TCP checksum was being treated as valid
>>> for certain frames with padding on them. On closer inspection I found the
>>> issue was that GRO was using the skb->len instead of the length recorded in
>>> the IP/IPv6 header to determine the number of bytes to checksum. As such
>>> padded frames that actually had invalid checksums generated by adding the
>>> padding to the checksum were being incorrectly tagged as valid.
>>>
>>> This change corrects that by using the tot_len from IPv4 headers and the
>>> payload_len from IPv6 headers to compute the total number of bytes to be
>>> included in the checksum.
>>>
>>> To address the fact that skb->csum is invalid when a padded frame is
>>> received I have updated the code to fall though to the CHECKSUM_NONE path
>>> for CHECKSUM_COMPLETE frames that contain padding.
>>>
>>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> Nack. This is needlessly complex. As I said before, please
>> just do a pskb_trim_rcsum in inet_gro_receive and its IPv6
>> counterpart and this should all just work.
> Actually I take that back. You are right that the preference is to
> flush and not trim, since we want to preserve the incoming packet in
> its exact form.
>
> So can you tell me a bit more about the padding? Is it added by the
> NIC or did it come from the remote side?
>
> Thanks,
The case I am addressing is padding added by the remote side.
Specifically in my case I was seeing TCP frames without options that
were padded up to 60 bytes from a netperf TCP_RR test. I messed up the
padding/checksum logic so it was making the same mistake in the Tx
checksum logic in the driver that I caught here in GRO. As a result I
was seeing checksum errors errors in wireshark, but noticed they were
being accepted by the stack as valid.
The driver has been fixed, and isn't anything that has been pushed
upstream yet so no harm there. I figured while I was at it I should
probably fix the GRO logic so that it doesn't mis-identify those types
of frames as being valid since that is something that may lead to
similar driver errors going undetected.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH net] bonding: set primary_reselect in LB and AB mode
From: Ding Tianhong @ 2013-11-16 4:07 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Ding Tianhong, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
In-Reply-To: <4031.1384538565@death.nxdomain>
于 2013/11/16 2:02, Jay Vosburgh 写道:
> Ding Tianhong <dingtianhong@huawei.com> wrote:
>
>> The primary_reselect only reselection for the primary slave,
>> but the primary slave only support for ALB, TLB and AB mode,
>> so we sould set the primary_reselect for these mode.
>>
>> to fix this: Add a check for ALB, TLB and AB mode in
>> bonding_store_primary_reselect, avoid to select active slave
>> again in other modes.
> I don't believe that setting primary_reselect in a
> !USES_PRIMARY() mode has any negative effects. It doesn't do anything,
> but also doesn't break anything. Is there a case that setting
> primary_reselect causes misbehavior in a !USES_PRIMARY() mode?
>
> Presuming that primary_reselect doesn't break things, this just
> adds an ordering limitation when configuring bonding (mode must be set
> prior to primary_reselect). I don't believe this change adds any value,
> and may break existing configuration scripts.
>
> -J
agree, I could not find any problem here, it is not bugfix, so miss it.
Regards
Ding
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_sysfs.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>> index 47749c9..cac2291 100644
>> --- a/drivers/net/bonding/bond_sysfs.c
>> +++ b/drivers/net/bonding/bond_sysfs.c
>> @@ -1141,6 +1141,13 @@ static ssize_t bonding_store_primary_reselect(struct device *d,
>> if (!rtnl_trylock())
>> return restart_syscall();
>>
>> + if (!USES_PRIMARY(bond->params.mode)) {
>> + pr_info("%s: Unable to set primary reselect; %s is in mode %d\n",
>> + bond->dev->name, bond->dev->name, bond->params.mode);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> new_value = bond_parse_parm(buf, pri_reselect_tbl);
>> if (new_value < 0) {
>> pr_err("%s: Ignoring invalid primary_reselect value %.*s.\n",
>> --
>> 1.7.12
> ---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
>
> --
> 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 tip/core/rcu 11/14] bonding/bond_main: Apply ACCESS_ONCE() to avoid sparse false positive
From: Ding Tianhong @ 2013-11-16 4:32 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
josh, niv, tglx, peterz, rostedt, dhowells, edumazet, darren,
fweisbec, sbw, Stephen Hemminger, David S. Miller, bridge, netdev
In-Reply-To: <1384562417-817-11-git-send-email-paulmck@linux.vnet.ibm.com>
于 2013/11/16 8:40, Paul E. McKenney 写道:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>
> The sparse checking for rcu_assign_pointer() was recently upgraded
> to reject non-__kernel address spaces. This also rejects __rcu,
> which is almost always the right thing to do. However, the uses in
> bond_change_active_slave() and __bond_release_one() are legitimate:
> They are assigning a pointer to an element from an RCU-protected list
> (or a NULL pointer), and all elements of this list are already visible
> to caller.
>
> This commit therefore silences these false positives either by laundering
> the pointers using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
> Triplett, or by using RCU_INIT_POINTER() for NULL pointer assignments.
I think it is fit for net-next.
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: bridge@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> ---
> drivers/net/bonding/bond_main.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 72df399c4ab3..bbd7fd3e46fe 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -890,7 +890,8 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
> if (new_active)
> bond_set_slave_active_flags(new_active);
> } else {
> - rcu_assign_pointer(bond->curr_active_slave, new_active);
> + /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
> + ACCESS_ONCE(bond->curr_active_slave) = new_active;
> }
>
> if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
> @@ -1801,7 +1802,7 @@ static int __bond_release_one(struct net_device *bond_dev,
> }
>
> if (all) {
> - rcu_assign_pointer(bond->curr_active_slave, NULL);
> + RCU_INIT_POINTER(bond->curr_active_slave, NULL);
> } else if (oldcurrent == slave) {
> /*
> * Note that we hold RTNL over this sequence, so there
^ permalink raw reply
* [PATCH] socket: don't return uninitialized addresses on concurrent socket shutdown
From: Hannes Frederic Sowa @ 2013-11-16 5:43 UTC (permalink / raw)
To: mpb; +Cc: netdev
In-Reply-To: <CAApbN=K8j+XQG+pQF2fxBSrDEPcjD242e9uP72azD9XWBzHcGQ@mail.gmail.com>
If a blocking read waits on a socket which gets concurrently shut down we
return 0 as error and so indicate success to the socket functions which
thus copy an uninitialized stack allocated address back to the user.
Fix this by clearing the 128 byte size (on x86-64) address first.
This patch fixes the problem for recvfrom, recvmsg and recvmmsg.
Reported-by: mpb <mpb.mail@gmail.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/socket.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/socket.c b/net/socket.c
index c226ace..44499db 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1834,6 +1834,7 @@ SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, ubuf, size_t, size,
if (!sock)
goto out;
+ memset(&address, 0, sizeof(address));
msg.msg_control = NULL;
msg.msg_controllen = 0;
msg.msg_iovlen = 1;
@@ -2228,6 +2229,8 @@ static int ___sys_recvmsg(struct socket *sock, struct msghdr __user *msg,
uaddr = (__force void __user *)msg_sys->msg_name;
uaddr_len = COMPAT_NAMELEN(msg);
+ if (uaddr != NULL)
+ memset(&addr, 0, sizeof(addr));
if (MSG_CMSG_COMPAT & flags) {
err = verify_compat_iovec(msg_sys, iov, &addr, VERIFY_WRITE);
} else
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2] socket: don't return uninitialized addresses on concurrent socket shutdown
From: Hannes Frederic Sowa @ 2013-11-16 5:48 UTC (permalink / raw)
To: mpb, netdev
In-Reply-To: <20131116054344.GG26901@order.stressinduktion.org>
If a blocking read waits on a socket which gets concurrently shut down we
return 0 as error and so indicate success to the socket functions which
thus copy an uninitialized stack allocated address back to the user.
Fix this by clearing the 128 byte size (on x86-64) address first.
This patch fixes the problem for recvfrom, recvmsg and recvmmsg.
Reported-by: mpb <mpb.mail@gmail.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
Sorry, grabbed the wrong patch file.
net/socket.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/socket.c b/net/socket.c
index c226ace..a44f29c 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1834,6 +1834,8 @@ SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, ubuf, size_t, size,
if (!sock)
goto out;
+ if (addr != NULL)
+ memset(&address, 0, sizeof(address));
msg.msg_control = NULL;
msg.msg_controllen = 0;
msg.msg_iovlen = 1;
@@ -2228,6 +2230,8 @@ static int ___sys_recvmsg(struct socket *sock, struct msghdr __user *msg,
uaddr = (__force void __user *)msg_sys->msg_name;
uaddr_len = COMPAT_NAMELEN(msg);
+ if (uaddr != NULL)
+ memset(&addr, 0, sizeof(addr));
if (MSG_CMSG_COMPAT & flags) {
err = verify_compat_iovec(msg_sys, iov, &addr, VERIFY_WRITE);
} else
--
1.8.3.1
^ 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