* [PATCHv2 net] openvswitch: Fix mask generation for IPv6 labels.
From: Joe Stringer @ 2014-11-19 21:20 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA
When userspace doesn't provide a mask, OVS datapath generates a fully
unwildcarded mask for the flow. This is done by taking a copy of the
flow key, then iterating across its attributes, setting all values to
0xff. This works for most attributes, as the length of the netlink
attribute typically matches the length of the value. However, IPv6
labels only use the lower 20 bits of the field. This patch makes a
special case to handle this.
This fixes the following error seen when installing IPv6 flows without a mask:
openvswitch: netlink: Invalid IPv6 flow label value (value=ffffffff, max=fffff)
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
v2: OR lower 20 bits (upper 12 bits remain from earlier memdup)
---
net/openvswitch/flow_netlink.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index fa4ec2e..e530025 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -825,7 +825,7 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
return 0;
}
-static void nlattr_set(struct nlattr *attr, u8 val, bool is_attr_mask_key)
+static void mask_set_nlattr(struct nlattr *attr)
{
struct nlattr *nla;
int rem;
@@ -835,16 +835,18 @@ static void nlattr_set(struct nlattr *attr, u8 val, bool is_attr_mask_key)
/* We assume that ovs_key_lens[type] == -1 means that type is a
* nested attribute
*/
- if (is_attr_mask_key && ovs_key_lens[nla_type(nla)] == -1)
- nlattr_set(nla, val, false);
+ if (ovs_key_lens[nla_type(nla)] == -1)
+ nla_for_each_nested(nla, attr, rem)
+ memset(nla_data(nla), 0xff, nla_len(nla));
else
- memset(nla_data(nla), val, nla_len(nla));
- }
-}
+ memset(nla_data(nla), 0xff, nla_len(nla));
-static void mask_set_nlattr(struct nlattr *attr, u8 val)
-{
- nlattr_set(attr, val, true);
+ if (nla_type(nla) == OVS_KEY_ATTR_IPV6) {
+ struct ovs_key_ipv6 *ipv6_key = nla_data(nla);
+
+ ipv6_key->ipv6_label |= htonl(0x000FFFFF);
+ }
+ }
}
/**
@@ -926,7 +928,7 @@ int ovs_nla_get_match(struct sw_flow_match *match,
if (!newmask)
return -ENOMEM;
- mask_set_nlattr(newmask, 0xff);
+ mask_set_nlattr(newmask);
/* The userspace does not send tunnel attributes that are 0,
* but we should not wildcard them nonetheless.
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply related
* Re: Question about Patch Submissions
From: rapier @ 2014-11-19 21:29 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Cong Wang, netdev
In-Reply-To: <CAADnVQ+1_Ct3hhmZrCqpxcT=-XS4OXrpRbrHHHyOKQOuoDc06g@mail.gmail.com>
On 11/18/14, 8:38 PM, Alexei Starovoitov wrote:
> On Tue, Nov 18, 2014 at 12:39 PM, rapier <rapier@psc.edu> wrote:
>> break this down for presentation. We can make sure each section of the patch
>> applies cleanly but the entirety of the patch set would have to be applied
>> for it to compile.
>
> all patches need to compile cleanly on all architectures,
> otherwise bisect will be broken.
The issue is that this is large patch set. The diff comes in at around
2200 lines and adds new functionality to the kernel by implementing RFC
4898. What I'm really trying to figure out is how to split up the diff
files so I'm not dropping all 2200 lines into a single email message.
I'm assuming that's not really how people want to see submissions but if
I am incorrect please let me know.
There are two major components that we should be able to submit as
distinct patches so that should reduce the line count per submission.
Chris Rapier
^ permalink raw reply
* Re: [RFC] situation with csum_and_copy_... API
From: Al Viro @ 2014-11-19 21:30 UTC (permalink / raw)
To: David Miller; +Cc: torvalds, netdev, linux-kernel
In-Reply-To: <20141119.161744.1661940121298888832.davem@davemloft.net>
On Wed, Nov 19, 2014 at 04:17:44PM -0500, David Miller wrote:
> > Seeing a "__get_user()" and just being able to glance up in the same
> > function and seeing the "access_ok()" is just a good safety net. And
> > means that people don't have to waste time thinking about or looking
> > for where the hell the security net really is.
>
> Fair enough.
OK, with 3/5 dropped 4/5 get a trivial conflict (removal of function in 4/5
vs. change in it in 3/5). With that dealt with, the sucker is in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-davem
Shortlog:
Al Viro (4):
separate kernel- and userland-side msghdr
{compat_,}verify_iovec(): switch to generic copying of iovecs
fold verify_iovec() into copy_msghdr_from_user()
bury skb_copy_to_page()
Diffstat:
arch/arm/kernel/sys_oabi-compat.c | 4 +--
include/linux/socket.h | 17 +++++++---
include/linux/syscalls.h | 6 ++--
include/net/compat.h | 5 ++-
include/net/sock.h | 23 -------------
net/compat.c | 83 +++++++++++++++-------------------------------
net/core/iovec.c | 47 --------------------------
net/socket.c | 140 +++++++++++++++++++++++++++++++++++++-----------------------------------------
8 files changed, 114 insertions(+), 211 deletions(-)
I'll post more for review after I finally get some sleep - up for bloody 27
hours by now ;-/
^ permalink raw reply
* RE: [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode
From: Tantilov, Emil S @ 2014-11-19 21:50 UTC (permalink / raw)
To: Vlad Yasevich, netdev@vger.kernel.org
Cc: Kirsher, Jeffrey T, Keller, Jacob E, Skidmore, Donald C
In-Reply-To: <546CF258.9050205@gmail.com>
>-----Original Message-----
>From: Vlad Yasevich [mailto:vyasevich@gmail.com]
>Sent: Wednesday, November 19, 2014 11:41 AM
>To: Tantilov, Emil S; netdev@vger.kernel.org
>Cc: Kirsher, Jeffrey T; Keller, Jacob E; Skidmore, Donald C
>Subject: Re: [PATCH] ixgbe: Correctly disable vlan filter in
>promiscuous mode
>
>On 11/18/2014 03:24 PM, Tantilov, Emil S wrote:
>>> -----Original Message-----
>>> From: netdev-owner@vger.kernel.org [mailto:netdev-
>>> owner@vger.kernel.org] On Behalf Of Vladislav Yasevich
>>> Sent: Tuesday, November 18, 2014 11:28 AM
>>> To: netdev@vger.kernel.org
>>> Cc: Vladislav Yasevich; Kirsher, Jeffrey T; Keller, Jacob E
>>> Subject: [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode
>>>
>>> IXGBE adapater seems to require that vlan filtering be enabled if VMDQ
>>> or SRIOV are enabled. When those functions are disabled,
>>> vlan filtering may be disabled in promiscuous mode.
>>>
>>> Prior to commit a9b8943ee129e11045862d6d6e25c5b63c95403c
>>> ixgbe: remove vlan_filter_disable and enable functions
>>>
>>> the logic was correct. However, after the commit the logic
>>> got reversed and vlan filtered in now turned on when VMDQ/SRIOV
>>> is disabled.
>>>
>>> This patch changes the condition to enable hw vlan filtered
>>> when VMDQ or SRIOV is enabled.
>>>
>>> Fixes: a9b8943ee129e11045862d6d6e25c5b63c95403c (ixgbe:
>>> remove
>>> vlan_filter_disable and enable functions)
>>> CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>> CC: Jacob Keller <jacob.e.keller@intel.com>
>>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>>> ---
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> index d2df4e3..3f81c7a 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> @@ -3936,8 +3936,8 @@ void ixgbe_set_rx_mode(struct
>>> net_device *netdev)
>>> * if SR-IOV and VMDQ are disabled - otherwise ensure
>>> * that hardware VLAN filters remain enabled.
>>> */
>>> - if (!(adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
>>> - IXGBE_FLAG_SRIOV_ENABLED)))
>>> + if (adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
>>> + IXGBE_FLAG_SRIOV_ENABLED))
>>> vlnctrl |= (IXGBE_VLNCTRL_VFE |
>>> IXGBE_VLNCTRL_CFIEN);
>>> } else {
>>> if (netdev->flags & IFF_ALLMULTI) {
>>
>> The current logic is correct and it's like this on purpose
>> as it should be obvious by the comment preceding this check.
>
>Actually the comment right now does not match what the code
>is doing.
>
>The comment states:
> /* Only disable hardware filter vlans in promiscuous mode
> * if SR-IOV and VMDQ are disabled - otherwise ensure
> * that hardware VLAN filters remain enabled.
> */
>
>However, the code currently will _enable_ vlan filtering if VMDQ/SRIOV
>is _disabled_ in promiscuous mode.
Actually you're right. Sorry - I misread the patch initially.
This is indeed a bug in the code.
Acked-by: Emil Tantilov <emil.s.tantilov@intel.com>
Jeff should pick it up.
Thanks,
Emil
^ permalink raw reply
* Re: [PATCHv2 net] openvswitch: Fix mask generation for IPv6 labels.
From: Joe Stringer @ 2014-11-19 21:52 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, pshelar, dev
In-Reply-To: <1416432001-35678-1-git-send-email-joestringer@nicira.com>
On Wednesday, November 19, 2014 13:20:01 Joe Stringer wrote:
> When userspace doesn't provide a mask, OVS datapath generates a fully
> unwildcarded mask for the flow. This is done by taking a copy of the
> flow key, then iterating across its attributes, setting all values to
> 0xff. This works for most attributes, as the length of the netlink
> attribute typically matches the length of the value. However, IPv6
> labels only use the lower 20 bits of the field. This patch makes a
> special case to handle this.
>
> This fixes the following error seen when installing IPv6 flows without a
> mask:
>
> openvswitch: netlink: Invalid IPv6 flow label value (value=ffffffff,
> max=fffff)
Based on offlist discussion with Pravin, I'll send a patch for an alternative
approach.
^ permalink raw reply
* Re: [RFC] situation with csum_and_copy_... API
From: David Miller @ 2014-11-19 21:53 UTC (permalink / raw)
To: viro; +Cc: torvalds, netdev, linux-kernel
In-Reply-To: <20141119213006.GE7996@ZenIV.linux.org.uk>
From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Wed, 19 Nov 2014 21:30:07 +0000
> On Wed, Nov 19, 2014 at 04:17:44PM -0500, David Miller wrote:
>> > Seeing a "__get_user()" and just being able to glance up in the same
>> > function and seeing the "access_ok()" is just a good safety net. And
>> > means that people don't have to waste time thinking about or looking
>> > for where the hell the security net really is.
>>
>> Fair enough.
>
> OK, with 3/5 dropped 4/5 get a trivial conflict (removal of function in 4/5
> vs. change in it in 3/5). With that dealt with, the sucker is in
> git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-davem
>
> Shortlog:
> Al Viro (4):
> separate kernel- and userland-side msghdr
> {compat_,}verify_iovec(): switch to generic copying of iovecs
> fold verify_iovec() into copy_msghdr_from_user()
> bury skb_copy_to_page()
Pulled, thanks Al.
^ permalink raw reply
* Re: [PATCH 1/1] mISDN: Deletion of unnecessary checks before the function call "vfree"
From: David Miller @ 2014-11-19 21:54 UTC (permalink / raw)
To: elfring; +Cc: isdn, netdev, linux-kernel, kernel-janitors, julia.lawall
In-Reply-To: <546CF5A6.3030305@users.sourceforge.net>
From: SF Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 19 Nov 2014 20:55:18 +0100
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 19 Nov 2014 20:48:26 +0100
>
> The vfree() function performs also input parameter validation. Thus the test
> around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Applied, thanks.
^ permalink raw reply
* [PATCHv3 net] openvswitch: Don't validate IPv6 label masks.
From: Joe Stringer @ 2014-11-19 21:54 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA
When userspace doesn't provide a mask, OVS datapath generates a fully
unwildcarded mask for the flow by copying the flow and setting all bits
in all fields. For IPv6 label, this creates a mask that matches on the
upper 12 bits, causing the following error:
openvswitch: netlink: Invalid IPv6 flow label value (value=ffffffff, max=fffff)
This patch ignores the label validation check for masks, avoiding this
error.
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
v3: Alternative approach.
Was "openvswitch: Fix mask generation for IPv6 labels."
v2: OR lower 20 bits (upper 12 bits remain from earlier memdup)
---
net/openvswitch/flow_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index fa4ec2e..089b195 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -690,7 +690,7 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
return -EINVAL;
}
- if (ipv6_key->ipv6_label & htonl(0xFFF00000)) {
+ if (!is_mask && ipv6_key->ipv6_label & htonl(0xFFF00000)) {
OVS_NLERR("IPv6 flow label %x is out of range (max=%x).\n",
ntohl(ipv6_key->ipv6_label), (1 << 20) - 1);
return -EINVAL;
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply related
* Re: [PATCH v2] ipv6: delete protocol and unregister rtnetlink when cleanup
From: David Miller @ 2014-11-19 21:55 UTC (permalink / raw)
To: duanj.fnst; +Cc: cwang, netdev, eric.dumazet
In-Reply-To: <546BF3EB.1080603@cn.fujitsu.com>
From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Date: Wed, 19 Nov 2014 09:35:39 +0800
> pim6_protocol was added when initiation, but it not deleted.
> Similarly, unregister RTNL_FAMILY_IP6MR rtnetlink.
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> ---
> v2: add missing rtnl_unregister()
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] bpf: fix arraymap NULL deref and missing overflow and zero size checks
From: David Miller @ 2014-11-19 21:55 UTC (permalink / raw)
To: ast; +Cc: fengguang.wu, hannes, dborkman, netdev, linux-kernel
In-Reply-To: <1416360736-9531-1-git-send-email-ast@plumgrid.com>
From: Alexei Starovoitov <ast@plumgrid.com>
Date: Tue, 18 Nov 2014 17:32:16 -0800
> - fix NULL pointer dereference:
> kernel/bpf/arraymap.c:41 array_map_alloc() error: potential null dereference 'array'. (kzalloc returns null)
> kernel/bpf/arraymap.c:41 array_map_alloc() error: we previously assumed 'array' could be null (see line 40)
>
> - integer overflow check was missing in arraymap
> (hashmap checks for overflow via kmalloc_array())
>
> - arraymap can round_up(value_size, 8) to zero. check was missing.
>
> - hashmap was missing zero size check as well, since roundup_pow_of_two() can
> truncate into zero
>
> - found a typo in the arraymap comment and unnecessary empty line
>
> Fix all of these issues and make both overflow checks explicit U32 in size.
>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> ---
> This silly NULL deref bug and missing overflow check was an oversight when
> I refactored the code from two allocations (kmalloc for struct bpf_array and
> kcalloc for array of elements) in the first implementation of arraymap
> into one allocation which is this code.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode
From: Jeff Kirsher @ 2014-11-19 21:56 UTC (permalink / raw)
To: Tantilov, Emil S
Cc: Vlad Yasevich, netdev@vger.kernel.org, Keller, Jacob E,
Skidmore, Donald C
In-Reply-To: <87618083B2453E4A8714035B62D67992501A1011@FMSMSX105.amr.corp.intel.com>
[-- Attachment #1: Type: text/plain, Size: 3588 bytes --]
On Wed, 2014-11-19 at 13:50 -0800, Tantilov, Emil S wrote:
> >-----Original Message-----
> >From: Vlad Yasevich [mailto:vyasevich@gmail.com]
> >Sent: Wednesday, November 19, 2014 11:41 AM
> >To: Tantilov, Emil S; netdev@vger.kernel.org
> >Cc: Kirsher, Jeffrey T; Keller, Jacob E; Skidmore, Donald C
> >Subject: Re: [PATCH] ixgbe: Correctly disable vlan filter in
> >promiscuous mode
> >
> >On 11/18/2014 03:24 PM, Tantilov, Emil S wrote:
> >>> -----Original Message-----
> >>> From: netdev-owner@vger.kernel.org [mailto:netdev-
> >>> owner@vger.kernel.org] On Behalf Of Vladislav Yasevich
> >>> Sent: Tuesday, November 18, 2014 11:28 AM
> >>> To: netdev@vger.kernel.org
> >>> Cc: Vladislav Yasevich; Kirsher, Jeffrey T; Keller, Jacob E
> >>> Subject: [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode
> >>>
> >>> IXGBE adapater seems to require that vlan filtering be enabled if VMDQ
> >>> or SRIOV are enabled. When those functions are disabled,
> >>> vlan filtering may be disabled in promiscuous mode.
> >>>
> >>> Prior to commit a9b8943ee129e11045862d6d6e25c5b63c95403c
> >>> ixgbe: remove vlan_filter_disable and enable functions
> >>>
> >>> the logic was correct. However, after the commit the logic
> >>> got reversed and vlan filtered in now turned on when VMDQ/SRIOV
> >>> is disabled.
> >>>
> >>> This patch changes the condition to enable hw vlan filtered
> >>> when VMDQ or SRIOV is enabled.
> >>>
> >>> Fixes: a9b8943ee129e11045862d6d6e25c5b63c95403c (ixgbe:
> >>> remove
> >>> vlan_filter_disable and enable functions)
> >>> CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >>> CC: Jacob Keller <jacob.e.keller@intel.com>
> >>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> >>> ---
> >>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >>> index d2df4e3..3f81c7a 100644
> >>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >>> @@ -3936,8 +3936,8 @@ void ixgbe_set_rx_mode(struct
> >>> net_device *netdev)
> >>> * if SR-IOV and VMDQ are disabled - otherwise ensure
> >>> * that hardware VLAN filters remain enabled.
> >>> */
> >>> - if (!(adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
> >>> - IXGBE_FLAG_SRIOV_ENABLED)))
> >>> + if (adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
> >>> + IXGBE_FLAG_SRIOV_ENABLED))
> >>> vlnctrl |= (IXGBE_VLNCTRL_VFE |
> >>> IXGBE_VLNCTRL_CFIEN);
> >>> } else {
> >>> if (netdev->flags & IFF_ALLMULTI) {
> >>
> >> The current logic is correct and it's like this on purpose
> >> as it should be obvious by the comment preceding this check.
> >
> >Actually the comment right now does not match what the code
> >is doing.
> >
> >The comment states:
> > /* Only disable hardware filter vlans in promiscuous mode
> > * if SR-IOV and VMDQ are disabled - otherwise ensure
> > * that hardware VLAN filters remain enabled.
> > */
> >
> >However, the code currently will _enable_ vlan filtering if VMDQ/SRIOV
> >is _disabled_ in promiscuous mode.
>
> Actually you're right. Sorry - I misread the patch initially.
> This is indeed a bug in the code.
>
> Acked-by: Emil Tantilov <emil.s.tantilov@intel.com>
>
> Jeff should pick it up.
Consider it done, I have added Vlad's patch to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH net] cxgb4i : Don't block unload/cxgb4 unload when remote closes TCP connection
From: David Miller @ 2014-11-19 21:59 UTC (permalink / raw)
To: anish; +Cc: netdev, linux-scsi, hch, jbottomley, kxie, manojmalviya
In-Reply-To: <1416366591-9699-1-git-send-email-anish@chelsio.com>
From: Anish Bhatt <anish@chelsio.com>
Date: Tue, 18 Nov 2014 19:09:51 -0800
> cxgb4i was returning wrong error and not releasing module reference if remote
> end abruptly closed TCP connection. This prevents the cxgb4 network module from
> being unloaded, further affecting other network drivers dependent on cxgb4
>
> Sending to net as this affects all cxgb4 based network drivers.
>
> Signed-off-by: Anish Bhatt <anish@chelsio.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v2] enic: support skb->xmit_more
From: David Miller @ 2014-11-19 22:02 UTC (permalink / raw)
To: _govind; +Cc: netdev, ssujith, benve, eric.dumazet
In-Reply-To: <1416382172-28775-1-git-send-email-_govind@gmx.com>
From: Govindarajulu Varadarajan <_govind@gmx.com>
Date: Wed, 19 Nov 2014 12:59:32 +0530
> Check and update posted_index only when skb->xmit_more is 0 or tx queue is full.
>
> v2:
> use txq_map instead of skb_get_queue_mapping(skb)
>
> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] sky2: use new netdev_rss_key_fill() helper
From: David Miller @ 2014-11-19 22:04 UTC (permalink / raw)
To: ipm; +Cc: netdev, mlindner, stephen, edumazet
In-Reply-To: <1416388011-10491-1-git-send-email-ipm@chirality.org.uk>
From: Ian Morris <ipm@chirality.org.uk>
Date: Wed, 19 Nov 2014 09:06:51 +0000
> Switch to a random RSS key rather than a fixed one.
> Using netdev_rss_key_fill helper also ensures that all ports share
> a common key.
>
> See also commit 960fb622f85180f36d3aff82af53e2be3db2f888.
>
> Signed-off-by: Ian Morris <ipm@chirality.org.uk>
Applied, thank you.
^ permalink raw reply
* Re: [PATCHv3 net] openvswitch: Don't validate IPv6 label masks.
From: Pravin Shelar @ 2014-11-19 22:05 UTC (permalink / raw)
To: Joe Stringer; +Cc: netdev, LKML, dev@openvswitch.org
In-Reply-To: <1416434089-47062-1-git-send-email-joestringer@nicira.com>
On Wed, Nov 19, 2014 at 1:54 PM, Joe Stringer <joestringer@nicira.com> wrote:
> When userspace doesn't provide a mask, OVS datapath generates a fully
> unwildcarded mask for the flow by copying the flow and setting all bits
> in all fields. For IPv6 label, this creates a mask that matches on the
> upper 12 bits, causing the following error:
>
> openvswitch: netlink: Invalid IPv6 flow label value (value=ffffffff, max=fffff)
>
> This patch ignores the label validation check for masks, avoiding this
> error.
>
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
Thanks for the fix.
Acked-by: Pravin B Shelar <pshelar@nicira.com>
> ---
> v3: Alternative approach.
> Was "openvswitch: Fix mask generation for IPv6 labels."
> v2: OR lower 20 bits (upper 12 bits remain from earlier memdup)
> ---
> net/openvswitch/flow_netlink.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index fa4ec2e..089b195 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -690,7 +690,7 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
> return -EINVAL;
> }
>
> - if (ipv6_key->ipv6_label & htonl(0xFFF00000)) {
> + if (!is_mask && ipv6_key->ipv6_label & htonl(0xFFF00000)) {
> OVS_NLERR("IPv6 flow label %x is out of range (max=%x).\n",
> ntohl(ipv6_key->ipv6_label), (1 << 20) - 1);
> return -EINVAL;
> --
> 1.7.10.4
>
^ permalink raw reply
* [PATCH v4] ipx: fix locking regression in ipx_sendmsg and ipx_recvmsg
From: Jiri Bohac @ 2014-11-19 22:05 UTC (permalink / raw)
To: David Miller; +Cc: jbohac, arnd, acme, netdev
In-Reply-To: <20141119.154400.1045032776950540216.davem@davemloft.net>
This fixes an old regression introduced by commit
b0d0d915 (ipx: remove the BKL).
When a recvmsg syscall blocks waiting for new data, no data can be sent on the
same socket with sendmsg because ipx_recvmsg() sleeps with the socket locked.
This breaks mars-nwe (NetWare emulator):
- the ncpserv process reads the request using recvmsg
- ncpserv forks and spawns nwconn
- ncpserv calls a (blocking) recvmsg and waits for new requests
- nwconn deadlocks in sendmsg on the same socket
Commit b0d0d915 has simply replaced BKL locking with
lock_sock/release_sock. Unlike now, BKL got unlocked while
sleeping, so a blocking recvmsg did not block a concurrent
sendmsg.
Only keep the socket locked while actually working with the socket data and
release it prior to calling skb_recv_datagram().
Signed-off-by: Jiri Bohac <jbohac@suse.cz>
diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index a0c7536..d0725d9 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1764,6 +1764,7 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
struct ipxhdr *ipx = NULL;
struct sk_buff *skb;
int copied, rc;
+ bool locked = true;
lock_sock(sk);
/* put the autobinding in */
@@ -1790,6 +1791,8 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
if (sock_flag(sk, SOCK_ZAPPED))
goto out;
+ release_sock(sk);
+ locked = false;
skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
flags & MSG_DONTWAIT, &rc);
if (!skb) {
@@ -1825,7 +1828,8 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
out_free:
skb_free_datagram(sk, skb);
out:
- release_sock(sk);
+ if (locked)
+ release_sock(sk);
return rc;
}
--
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ
^ permalink raw reply related
* Re: [PATCH v4] ipx: fix locking regression in ipx_sendmsg and ipx_recvmsg
From: Arnd Bergmann @ 2014-11-19 22:12 UTC (permalink / raw)
To: Jiri Bohac; +Cc: David Miller, acme, netdev
In-Reply-To: <20141119220549.GA26133@midget.suse.cz>
On Wednesday 19 November 2014 23:05:49 Jiri Bohac wrote:
> This fixes an old regression introduced by commit
> b0d0d915 (ipx: remove the BKL).
>
> When a recvmsg syscall blocks waiting for new data, no data can be sent on the
> same socket with sendmsg because ipx_recvmsg() sleeps with the socket locked.
>
> This breaks mars-nwe (NetWare emulator):
> - the ncpserv process reads the request using recvmsg
> - ncpserv forks and spawns nwconn
> - ncpserv calls a (blocking) recvmsg and waits for new requests
> - nwconn deadlocks in sendmsg on the same socket
>
> Commit b0d0d915 has simply replaced BKL locking with
> lock_sock/release_sock. Unlike now, BKL got unlocked while
> sleeping, so a blocking recvmsg did not block a concurrent
> sendmsg.
>
> Only keep the socket locked while actually working with the socket data and
> release it prior to calling skb_recv_datagram().
>
>
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply
* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Cong Wang @ 2014-11-19 22:26 UTC (permalink / raw)
To: Wengang; +Cc: Eric Dumazet, netdev
In-Reply-To: <546C4022.5010509@oracle.com>
On Tue, Nov 18, 2014 at 11:00 PM, Wengang <wen.gang.wang@oracle.com> wrote:
>
> Yes, that's true. So the simplest way is move ipoib_header_ops to vmlinux.
>
That is not an option. Perhaps you need RCU to protect the dev->header_ops
pointer.
^ permalink raw reply
* [PATCH RESEND] natsemi: fix timing issue for reading mac from eeprom via dp8381x
From: Roland Kletzing @ 2014-11-19 22:26 UTC (permalink / raw)
To: netdev; +Cc: David Miller
Natsemi driver does not read MAC correctly from eeprom, while natsemi-diag
from nictools-pci does. Apparently, it`s a timing issue in the kernel driver.
According to ftp://ftp.gwdg.de/pub/linux/misc/donald.becker/diag/natsemi-diag.c
, eeprom_delay(ee_addr) is defined as follows:
/* Delay between EEPROM clock transitions.
This flushes the write buffer to prevent quick double-writes.
*/
#define eeprom_delay(ee_addr) inl(ee_addr); inl(ee_addr)
while in the natsemi linux kernel driver, the delay is done this way :
#define eeprom_delay(ee_addr) readl(ee_addr)
, which results in the MAC being all zero`s on my systems (old Geode GX1 board
like being used in Compaq Evo T20 or Wyse WT3235LE Thin Clients).
So i simply added a second readl() to increase delay (instead of turning into
inl() as proposed before). This may look a little bit ugly, but it`s fixing the
problem for me.
I´m not sure how many natsemi users being left on this planet (probably few),
but i guess this change does not do any harm on platforms where the driver does
not behave buggy, so please consider adding it to mainline/stable/longterm.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=51791
Reported-by: Roland Kletzing <devzero@web.de>
Signed-off-by: Roland Kletzing <devzero@web.de>
---
drivers/net/ethernet/natsemi/natsemi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/natsemi/natsemi.c b/drivers/net/ethernet/natsemi/natsemi.c
index b83f7c0..96f0029 100644
--- a/drivers/net/ethernet/natsemi/natsemi.c
+++ b/drivers/net/ethernet/natsemi/natsemi.c
@@ -987,7 +987,7 @@ static int natsemi_probe1(struct pci_dev *pdev, const struct pci_device_id *ent)
The old method of using an ISA access as a delay, __SLOW_DOWN_IO__, is
deprecated.
*/
-#define eeprom_delay(ee_addr) readl(ee_addr)
+#define eeprom_delay(ee_addr) readl(ee_addr); readl(ee_addr)
#define EE_Write0 (EE_ChipSelect)
#define EE_Write1 (EE_ChipSelect | EE_DataIn)
--
^ permalink raw reply related
* Re: [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls
From: Julian Anastasov @ 2014-11-19 22:26 UTC (permalink / raw)
To: SF Markus Elfring
Cc: David S. Miller, Jozsef Kadlecsik, Pablo Neira Ayuso,
Patrick McHardy, Simon Horman, Wensong Zhang, netdev, lvs-devel,
netfilter-devel, coreteam, LKML, kernel-janitors, Coccinelle
In-Reply-To: <546BA253.6030100@users.sourceforge.net>
Hello,
On Tue, 18 Nov 2014, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 18 Nov 2014 20:37:05 +0100
>
> The functions free_percpu() and module_put() test whether their argument
> is NULL and then return immediately. Thus the test around the call is
> not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Pablo, the IPVS parts look ok to me,
Acked-by: Julian Anastasov <ja@ssi.bg>
> ---
> net/netfilter/ipvs/ip_vs_ctl.c | 3 +--
> net/netfilter/ipvs/ip_vs_pe.c | 3 +--
> net/netfilter/ipvs/ip_vs_sched.c | 3 +--
> net/netfilter/ipvs/ip_vs_sync.c | 3 +--
> net/netfilter/nf_tables_api.c | 3 +--
> 5 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index fd3f444..7c5e40a 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -465,8 +465,7 @@ __ip_vs_bind_svc(struct ip_vs_dest *dest, struct ip_vs_service *svc)
>
> static void ip_vs_service_free(struct ip_vs_service *svc)
> {
> - if (svc->stats.cpustats)
> - free_percpu(svc->stats.cpustats);
> + free_percpu(svc->stats.cpustats);
> kfree(svc);
> }
>
> diff --git a/net/netfilter/ipvs/ip_vs_pe.c b/net/netfilter/ipvs/ip_vs_pe.c
> index 1a82b29..0df17ca 100644
> --- a/net/netfilter/ipvs/ip_vs_pe.c
> +++ b/net/netfilter/ipvs/ip_vs_pe.c
> @@ -37,8 +37,7 @@ struct ip_vs_pe *__ip_vs_pe_getbyname(const char *pe_name)
> rcu_read_unlock();
> return pe;
> }
> - if (pe->module)
> - module_put(pe->module);
> + module_put(pe->module);
> }
> rcu_read_unlock();
>
> diff --git a/net/netfilter/ipvs/ip_vs_sched.c b/net/netfilter/ipvs/ip_vs_sched.c
> index 4dbcda6..199760c 100644
> --- a/net/netfilter/ipvs/ip_vs_sched.c
> +++ b/net/netfilter/ipvs/ip_vs_sched.c
> @@ -104,8 +104,7 @@ static struct ip_vs_scheduler *ip_vs_sched_getbyname(const char *sched_name)
> mutex_unlock(&ip_vs_sched_mutex);
> return sched;
> }
> - if (sched->module)
> - module_put(sched->module);
> + module_put(sched->module);
> }
>
> mutex_unlock(&ip_vs_sched_mutex);
> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index eadffb2..cafe28d 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -820,8 +820,7 @@ ip_vs_conn_fill_param_sync(struct net *net, int af, union ip_vs_sync_conn *sc,
>
> p->pe_data = kmemdup(pe_data, pe_data_len, GFP_ATOMIC);
> if (!p->pe_data) {
> - if (p->pe->module)
> - module_put(p->pe->module);
> + module_put(p->pe->module);
> return -ENOMEM;
> }
> p->pe_data_len = pe_data_len;
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index deeb95f..b115f54 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -3472,8 +3472,7 @@ static int nf_tables_abort(struct sk_buff *skb)
> break;
> case NFT_MSG_NEWCHAIN:
> if (nft_trans_chain_update(trans)) {
> - if (nft_trans_chain_stats(trans))
> - free_percpu(nft_trans_chain_stats(trans));
> + free_percpu(nft_trans_chain_stats(trans));
>
> nft_trans_destroy(trans);
> } else {
> --
> 2.1.3
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Cong Wang @ 2014-11-19 22:56 UTC (permalink / raw)
To: Wengang; +Cc: Eric Dumazet, netdev
In-Reply-To: <CAHA+R7OyA_V149bpE8qUgG9epU-5UG8cf=zOgh5ZXVB3ZFZHUw@mail.gmail.com>
On Wed, Nov 19, 2014 at 2:26 PM, Cong Wang <cwang@twopensource.com> wrote:
> On Tue, Nov 18, 2014 at 11:00 PM, Wengang <wen.gang.wang@oracle.com> wrote:
>>
>> Yes, that's true. So the simplest way is move ipoib_header_ops to vmlinux.
>>
>
> That is not an option. Perhaps you need RCU to protect the dev->header_ops
> pointer.
Or maybe take a refcount of that module.
BTW, why this is a problem only for bonding? Doesn't neigh have the
same bug? Or it takes the module refcount somewhere I don't find?
^ permalink raw reply
* RE: [net-next 07/15] ixgbevf: Change receive model to use double buffered page based receives
From: Tantilov, Emil S @ 2014-11-19 23:06 UTC (permalink / raw)
To: Alexander Duyck, Kirsher, Jeffrey T, davem@davemloft.net
Cc: netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
jogreene@redhat.com
In-Reply-To: <546CE067.9010903@redhat.com>
>-----Original Message-----
>From: Alexander Duyck [mailto:alexander.h.duyck@redhat.com]
>Sent: Wednesday, November 19, 2014 10:25 AM
>To: Kirsher, Jeffrey T; davem@davemloft.net
>Cc: Tantilov, Emil S; netdev@vger.kernel.org;
>nhorman@redhat.com; sassmann@redhat.com; jogreene@redhat.com
>Subject: Re: [net-next 07/15] ixgbevf: Change receive model to use double buffered page based receives
>
>There were a few things in this patch that should be addressed.
>
>Comments inline below.
>
>- Alex
>
>On 11/18/2014 08:10 PM, Jeff Kirsher wrote:
>> From: Emil Tantilov <emil.s.tantilov@intel.com>
>>
>> This patch changes the basic receive path for ixgbevf so that instead of
>> receiving the data into an skb it is received into a double buffered page.
>> The main change is that the receives will be done in pages only and then
>> pull the header out of the page and copy it into the sk_buff data.
>>
>> This has the advantages of reduced cache misses and improved performance on
>> IOMMU enabled systems.
>>
>> CC: Alexander Duyck <alexander.h.duyck@redhat.com>
>> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
>> Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 24 +-
>> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 452 +++++++++++++++-------
>> 2 files changed, 331 insertions(+), 145 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>> index 72a354b..2362001 100644
>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>> @@ -58,8 +58,9 @@ struct ixgbevf_tx_buffer {
>> };
>>
>> struct ixgbevf_rx_buffer {
>> - struct sk_buff *skb;
>> dma_addr_t dma;
>> + struct page *page;
>> + unsigned int page_offset;
>> };
>>
>> struct ixgbevf_stats {
>> @@ -91,9 +92,10 @@ struct ixgbevf_ring {
>> void *desc; /* descriptor ring memory */
>> dma_addr_t dma; /* phys. address of
>descriptor ring */
>> unsigned int size; /* length in bytes */
>> - unsigned int count; /* amount of descriptors */
>> - unsigned int next_to_use;
>> - unsigned int next_to_clean;
>> + u16 count; /* amount of descriptors */
>> + u16 next_to_use;
>> + u16 next_to_clean;
>> + u16 next_to_alloc;
>>
>> union {
>> struct ixgbevf_tx_buffer *tx_buffer_info;
>> @@ -109,12 +111,11 @@ struct ixgbevf_ring {
>>
>> u64 hw_csum_rx_error;
>> u8 __iomem *tail;
>> + struct sk_buff *skb;
>>
>> u16 reg_idx; /* holds the special value that gets the hardware register
>> * offset associated with this ring, which is different
>> * for DCB and RSS modes */
>> -
>> - u16 rx_buf_len;
>> int queue_index; /* needed for multiqueue queue management */
>> };
>>
>> @@ -133,12 +134,10 @@ struct ixgbevf_ring {
>>
>> /* Supported Rx Buffer Sizes */
>> #define IXGBEVF_RXBUFFER_256 256 /* Used for packet split */
>> -#define IXGBEVF_RXBUFFER_2K 2048
>> -#define IXGBEVF_RXBUFFER_4K 4096
>> -#define IXGBEVF_RXBUFFER_8K 8192
>> -#define IXGBEVF_RXBUFFER_10K 10240
>> +#define IXGBEVF_RXBUFFER_2048 2048
>>
>> #define IXGBEVF_RX_HDR_SIZE IXGBEVF_RXBUFFER_256
>> +#define IXGBEVF_RX_BUFSZ IXGBEVF_RXBUFFER_2048
>>
>> #define MAXIMUM_ETHERNET_VLAN_SIZE (VLAN_ETH_FRAME_LEN + ETH_FCS_LEN)
>>
>> @@ -430,11 +429,6 @@ enum ixbgevf_state_t {
>> __IXGBEVF_WORK_INIT,
>> };
>>
>> -struct ixgbevf_cb {
>> - struct sk_buff *prev;
>> -};
>> -#define IXGBE_CB(skb) ((struct ixgbevf_cb *)(skb)->cb)
>> -
>> enum ixgbevf_boards {
>> board_82599_vf,
>> board_X540_vf,
>> diff --git
>a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> index 20bebd2..2ca7c96 100644
>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> @@ -422,8 +422,7 @@ static void
>ixgbevf_process_skb_fields(struct ixgbevf_ring *rx_ring,
>> * that this is in fact a non-EOP buffer.
>> **/
>> static bool ixgbevf_is_non_eop(struct ixgbevf_ring *rx_ring,
>> - union ixgbe_adv_rx_desc *rx_desc,
>> - struct sk_buff *skb)
>> + union ixgbe_adv_rx_desc *rx_desc)
>> {
>> u32 ntc = rx_ring->next_to_clean + 1;
>>
>> @@ -439,37 +438,40 @@ static bool
>ixgbevf_is_non_eop(struct ixgbevf_ring *rx_ring,
>> return true;
>> }
>>
>> -static bool ixgbevf_alloc_mapped_skb(struct ixgbevf_ring *rx_ring,
>> - struct ixgbevf_rx_buffer *bi)
>> +static bool ixgbevf_alloc_mapped_page(struct ixgbevf_ring *rx_ring,
>> + struct ixgbevf_rx_buffer *bi)
>> {
>> - struct sk_buff *skb = bi->skb;
>> + struct page *page = bi->page;
>> dma_addr_t dma = bi->dma;
>>
>> - if (unlikely(skb))
>> + /* since we are recycling buffers we should seldom need to alloc */
>> + if (likely(page))
>> return true;
>>
>> - skb = netdev_alloc_skb_ip_align(rx_ring->netdev,
>> - rx_ring->rx_buf_len);
>> - if (unlikely(!skb)) {
>> - rx_ring->rx_stats.alloc_rx_buff_failed++;
>> + /* alloc new page for storage */
>> + page = dev_alloc_page();
>> + if (unlikely(!page)) {
>> + rx_ring->rx_stats.alloc_rx_page_failed++;
>> return false;
>> }
>>
>> - dma = dma_map_single(rx_ring->dev, skb->data,
>> - rx_ring->rx_buf_len, DMA_FROM_DEVICE);
>> + /* map page for use */
>> + dma = dma_map_page(rx_ring->dev, page, 0,
>> + PAGE_SIZE, DMA_FROM_DEVICE);
>>
>> /* if mapping failed free memory back to system since
>> * there isn't much point in holding memory we can't use
>> */
>> if (dma_mapping_error(rx_ring->dev, dma)) {
>> - dev_kfree_skb_any(skb);
>> + __free_page(page);
>>
>> rx_ring->rx_stats.alloc_rx_buff_failed++;
>> return false;
>> }
>>
>> - bi->skb = skb;
>> bi->dma = dma;
>> + bi->page = page;
>> + bi->page_offset = 0;
>>
>> return true;
>> }
>> @@ -495,13 +497,13 @@ static void
>ixgbevf_alloc_rx_buffers(struct ixgbevf_ring *rx_ring,
>> i -= rx_ring->count;
>>
>> do {
>> - if (!ixgbevf_alloc_mapped_skb(rx_ring, bi))
>> + if (!ixgbevf_alloc_mapped_page(rx_ring, bi))
>> break;
>>
>> /* Refresh the desc even if pkt_addr didn't change
>> * because each write-back erases this info.
>> */
>> - rx_desc->read.pkt_addr = cpu_to_le64(bi->dma);
>> + rx_desc->read.pkt_addr = cpu_to_le64(bi->dma + bi->page_offset);
>>
>> rx_desc++;
>> bi++;
>> @@ -524,6 +526,9 @@ static void
>ixgbevf_alloc_rx_buffers(struct ixgbevf_ring *rx_ring,
>> /* record the next descriptor to use */
>> rx_ring->next_to_use = i;
>>
>> + /* update next to alloc since we have filled the ring */
>> + rx_ring->next_to_alloc = i;
>> +
>> /* Force memory writes to complete before letting h/w
>> * know there are new descriptors to fetch. (Only
>> * applicable for weak-ordered memory model archs,
>> @@ -534,6 +539,257 @@ static void
>ixgbevf_alloc_rx_buffers(struct ixgbevf_ring *rx_ring,
>> }
>> }
>>
>> +/* ixgbevf_pull_tail - ixgbevf specific version of skb_pull_tail
>> + * @rx_ring: Rx descriptor ring packet is being transacted on
>> + * @skb: pointer to current skb being adjusted
>> + *
>> + * This function is an ixgbevf specific version of __pskb_pull_tail. The
>> + * main difference between this version and the original function is that
>> + * this function can make several assumptions about the state of things
>> + * that allow for significant optimizations versus the standard function.
>> + * As a result we can do things like drop a frag and maintain an accurate
>> + * truesize for the skb.
>> + */
>> +static void ixgbevf_pull_tail(struct ixgbevf_ring *rx_ring,
>> + struct sk_buff *skb)
>> +{
>> + struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[0];
>> + unsigned char *va;
>> + unsigned int pull_len;
>> +
>> + /* it is valid to use page_address instead of kmap since we are
>> + * working with pages allocated out of the lomem pool per
>> + * alloc_page(GFP_ATOMIC)
>> + */
>> + va = skb_frag_address(frag);
>> +
>> + /* we need the header to contain the greater of either ETH_HLEN or
>> + * 60 bytes if the skb->len is less than 60 for skb_pad.
>> + */
>> + pull_len = eth_get_headlen(va, IXGBEVF_RX_HDR_SIZE);
>> +
>> + /* align pull length to size of long to optimize memcpy performance */
>> + skb_copy_to_linear_data(skb, va, ALIGN(pull_len, sizeof(long)));
>> +
>> + /* update all of the pointers */
>> + skb_frag_size_sub(frag, pull_len);
>> + frag->page_offset += pull_len;
>> + skb->data_len -= pull_len;
>> + skb->tail += pull_len;
>> +}
>
>I really think we should look at making this into a generic function.
>Maybe I will submit something later today to get a common function
>placed in the code. Maybe something like eth_pull_tail.
Sounds good.
>
>> +/* ixgbevf_cleanup_headers - Correct corrupted or empty headers
>> + * @rx_ring: Rx descriptor ring packet is being transacted on
>> + * @rx_desc: pointer to the EOP Rx descriptor
>> + * @skb: pointer to current skb being fixed
>> + *
>> + * Check for corrupted packet headers caused by senders on the local L2
>> + * embedded NIC switch not setting up their Tx Descriptors right. These
>> + * should be very rare.
>> + *
>> + * Also address the case where we are pulling data in on pages only
>> + * and as such no data is present in the skb header.
>> + *
>> + * In addition if skb is not at least 60 bytes we need to pad it so that
>> + * it is large enough to qualify as a valid Ethernet frame.
>> + *
>> + * Returns true if an error was encountered and skb was freed.
>> + */
>> +static bool ixgbevf_cleanup_headers(struct ixgbevf_ring *rx_ring,
>> + union ixgbe_adv_rx_desc *rx_desc,
>> + struct sk_buff *skb)
>> +{
>> + /* verify that the packet does not have any known errors */
>> + if (unlikely(ixgbevf_test_staterr(rx_desc,
>> +
>IXGBE_RXDADV_ERR_FRAME_ERR_MASK))) {
>> + struct net_device *netdev = rx_ring->netdev;
>> +
>> + if (!(netdev->features & NETIF_F_RXALL)) {
>> + dev_kfree_skb_any(skb);
>> + return true;
>> + }
>> + }
>> +
>> + /* place header in linear portion of buffer */
>> + if (skb_is_nonlinear(skb))
>> + ixgbevf_pull_tail(rx_ring, skb);
>> +
>> + /* if skb_pad returns an error the skb was freed */
>> + if (unlikely(skb->len < 60)) {
>> + int pad_len = 60 - skb->len;
>> +
>> + if (skb_pad(skb, pad_len))
>> + return true;
>> + __skb_put(skb, pad_len);
>> + }
>> +
>> + return false;
>> +}
>
>The same goes for the padding bit here. Maybe something like eth_skb_pad.
OK
>> +/* ixgbevf_reuse_rx_page - page flip buffer and store it back on the ring
>> + * @rx_ring: Rx descriptor ring to store buffers on
>> + * @old_buff: donor buffer to have page reused
>> + *
>> + * Synchronizes page for reuse by the adapter
>> + */
>> +static void ixgbevf_reuse_rx_page(struct ixgbevf_ring *rx_ring,
>> + struct ixgbevf_rx_buffer *old_buff)
>> +{
>> + struct ixgbevf_rx_buffer *new_buff;
>> + u16 nta = rx_ring->next_to_alloc;
>> +
>> + new_buff = &rx_ring->rx_buffer_info[nta];
>> +
>> + /* update, and store next to alloc */
>> + nta++;
>> + rx_ring->next_to_alloc = (nta < rx_ring->count) ? nta : 0;
>> +
>> + /* transfer page from old buffer to new buffer */
>> + new_buff->page = old_buff->page;
>> + new_buff->dma = old_buff->dma;
>> + new_buff->page_offset = old_buff->page_offset;
>> +
>> + /* sync the buffer for use by the device */
>> + dma_sync_single_range_for_device(rx_ring->dev, new_buff->dma,
>> + new_buff->page_offset,
>> + IXGBEVF_RX_BUFSZ,
>> + DMA_FROM_DEVICE);
>> +}
>> +
>> +/* ixgbevf_add_rx_frag - Add contents of Rx buffer to sk_buff
>> + * @rx_ring: Rx descriptor ring to transact packets on
>> + * @rx_buffer: buffer containing page to add
>> + * @rx_desc: descriptor containing length of buffer written by hardware
>> + * @skb: sk_buff to place the data into
>> + *
>> + * This function will add the data contained in rx_buffer->page to the skb.
>> + * This is done either through a direct copy if the data in the buffer is
>> + * less than the skb header size, otherwise it will just attach the page as
>> + * a frag to the skb.
>> + *
>> + * The function will then update the page offset if necessary and return
>> + * true if the buffer can be reused by the adapter.
>> + */
>> +static bool ixgbevf_add_rx_frag(struct ixgbevf_ring *rx_ring,
>> + struct ixgbevf_rx_buffer *rx_buffer,
>> + union ixgbe_adv_rx_desc *rx_desc,
>> + struct sk_buff *skb)
>> +{
>> + struct page *page = rx_buffer->page;
>> + unsigned int size = le16_to_cpu(rx_desc-
>>wb.upper.length);
>> +#if (PAGE_SIZE < 8192)
>> + unsigned int truesize = IXGBEVF_RX_BUFSZ;
>> +#else
>> + unsigned int truesize = ALIGN(size, L1_CACHE_BYTES);
>> +#endif
>> +
>> + if ((size <= IXGBEVF_RX_HDR_SIZE) && !skb_is_nonlinear(skb)) {
>> + unsigned char *va = page_address(page) + rx_buffer->page_offset;
>> +
>> + memcpy(__skb_put(skb, size), va, ALIGN(size, sizeof(long)));
>> +
>> + /* we can reuse buffer as-is, just make sure it is local */
>> + if (likely(page_to_nid(page) == numa_node_id()))
>> + return true;
>> +
>> + /* this page cannot be reused so discard it */
>> + put_page(page);
>> + return false;
>> + }
>> +
>> + skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
>> + rx_buffer->page_offset, size, truesize);
>> +
>> + /* avoid re-using remote pages */
>> + if (unlikely(page_to_nid(page) != numa_node_id()))
>> + return false;
>> +
>
>This is missing the pfmemalloc fix that is already in igb,
>and that I submitted for ixgbe and fm10k.
Just because of timing. I can redo the patch with the pfmemalloc check added.
>
>> +#if (PAGE_SIZE < 8192)
>> + /* if we are only owner of page we can reuse it */
>> + if (unlikely(page_count(page) != 1))
>> + return false;
>> +
>> + /* flip page offset to other buffer */
>> + rx_buffer->page_offset ^= IXGBEVF_RX_BUFSZ;
>> +
>> + /* since we are the only owner of the page and we need to
>> + * increment it.
>> + */
>> + atomic_inc(&page->_count);
>> +#else
>> + /* move offset up to the next cache line */
>> + rx_buffer->page_offset += truesize;
>> +
>> + if (rx_buffer->page_offset > (PAGE_SIZE - IXGBEVF_RX_BUFSZ))
>> + return false;
>> +
>> + /* bump ref count on page before it is given to the stack */
>> + get_page(page);
>> +#endif
>> +
>> + return true;
>> +}
>
>The get_page and atomic_inc calls can be pulled out and
>placed after if #if/else logic. The preference is to use atomic_inc since
>you are using an order 0 page and don't need to check for PageTail().
I can do that,
>> +static struct sk_buff *ixgbevf_fetch_rx_buffer(struct ixgbevf_ring *rx_ring,
>> + union ixgbe_adv_rx_desc *rx_desc,
>> + struct sk_buff *skb)
>> +{
>> + struct ixgbevf_rx_buffer *rx_buffer;
>> + struct page *page;
>> +
>> + rx_buffer = &rx_ring->rx_buffer_info[rx_ring->next_to_clean];
>> + page = rx_buffer->page;
>> + prefetchw(page);
>> +
>> + if (likely(!skb)) {
>> + void *page_addr = page_address(page) +
>> + rx_buffer->page_offset;
>> +
>> + /* prefetch first cache line of first page */
>> + prefetch(page_addr);
>> +#if L1_CACHE_BYTES < 128
>> + prefetch(page_addr + L1_CACHE_BYTES);
>> +#endif
>> +
>> + /* allocate a skb to store the frags */
>> + skb = netdev_alloc_skb_ip_align(rx_ring->netdev,
>> + IXGBEVF_RX_HDR_SIZE);
>> + if (unlikely(!skb)) {
>> + rx_ring->rx_stats.alloc_rx_buff_failed++;
>> + return NULL;
>> + }
>> +
>> + /* we will be copying header into skb->data in
>> + * pskb_may_pull so it is in our interest to prefetch
>> + * it now to avoid a possible cache miss
>> + */
>> + prefetchw(skb->data);
>> + }
>> +
>> + /* we are reusing so sync this buffer for CPU use */
>> + dma_sync_single_range_for_cpu(rx_ring->dev,
>> + rx_buffer->dma,
>> + rx_buffer->page_offset,
>> + IXGBEVF_RX_BUFSZ,
>> + DMA_FROM_DEVICE);
>> +
>> + /* pull page into skb */
>> + if (ixgbevf_add_rx_frag(rx_ring, rx_buffer, rx_desc, skb)) {
>> + /* hand second half of page back to the ring */
>> + ixgbevf_reuse_rx_page(rx_ring, rx_buffer);
>> + } else {
>> + /* we are not reusing the buffer so unmap it */
>> + dma_unmap_page(rx_ring->dev, rx_buffer->dma,
>> + PAGE_SIZE, DMA_FROM_DEVICE);
>> + }
>> +
>> + /* clear contents of buffer_info */
>> + rx_buffer->dma = 0;
>> + rx_buffer->page = NULL;
>> +
>> + return skb;
>> +}
>> +
>> static inline void ixgbevf_irq_enable_queues(struct ixgbevf_adapter *adapter,
>> u32 qmask)
>> {
>
><snip>
>
>> @@ -3320,21 +3522,11 @@ static int ixgbevf_set_mac(struct net_device *netdev, void *p)
>> static int ixgbevf_change_mtu(struct net_device *netdev, int new_mtu)
>> {
>> struct ixgbevf_adapter *adapter = netdev_priv(netdev);
>> + struct ixgbe_hw *hw = &adapter->hw;
>> int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
>> - int max_possible_frame = MAXIMUM_ETHERNET_VLAN_SIZE;
>> -
>> - switch (adapter->hw.api_version) {
>> - case ixgbe_mbox_api_11:
>> - max_possible_frame = IXGBE_MAX_JUMBO_FRAME_SIZE;
>> - break;
>> - default:
>> - if (adapter->hw.mac.type == ixgbe_mac_X540_vf)
>> - max_possible_frame = IXGBE_MAX_JUMBO_FRAME_SIZE;
>> - break;
>> - }
>>
>> /* MTU < 68 is an error and causes problems on some kernels */
>> - if ((new_mtu < 68) || (max_frame > max_possible_frame))
>> + if ((new_mtu < 68) || (max_frame > IXGBE_MAX_JUMBO_FRAME_SIZE))
>> return -EINVAL;
>>
>> hw_dbg(&adapter->hw, "changing MTU from %d to %d\n",
>
>This is wrong. You are still limited by the PF so if it is a version
>1.0 mailbox on an 82599 you cannot enable jumbo frames. Yes you can
>support it but the PF won't let you do it.
>
>> @@ -3342,8 +3534,8 @@ static int ixgbevf_change_mtu(struct
>net_device *netdev, int new_mtu)
>> /* must set new MTU before calling down or up */
>> netdev->mtu = new_mtu;
>>
>> - if (netif_running(netdev))
>> - ixgbevf_reinit_locked(adapter);
>> + /* notify the PF of our intent to use this size of frame */
>> + ixgbevf_rlpml_set_vf(hw, max_frame);
>>
>> return 0;
>> }
>>
>
>This is the reason why the change is wrong. If the mailbox api is
>version 1.0 you cannot support jumbo frames so ixgbevf_rlmpl_set_vf will
>return an error via the mailbox indicating that the message >is not
>supported.
I'll have to see how this change was introduced, but I can drop it for now since it's not directly related to this change anyway.
Thanks,
Emil
^ permalink raw reply
* Re: [PATCH net-next] bridge: make proxy arp configurable
From: kyeyoonp @ 2014-11-19 23:11 UTC (permalink / raw)
To: David Miller; +Cc: shemming, kyeyoonp, netdev
In-Reply-To: <20141031.122133.1697556792716394439.davem@davemloft.net>
> From: Stephen Hemminger <shemming@brocade.com>
> Date: Thu, 30 Oct 2014 20:09:42 -0700
>
>> @@ -60,3 +60,19 @@ config BRIDGE_VLAN_FILTERING
>> Say N to exclude this support and reduce the binary size.
>>
>> If unsure, say Y.
>> +
>> +config BRIDGE_ARP_PROXY
>> + bool "ARP proxying"
>> + depends on BRIDGE
>> + depends on INET
>> + default y
>> + ---help---
>> + If you say Y here, then the Ethernet bridge to keep track of
>> + the hardware address to IP address mapping.
>> +
>> + It is most useful when used as a wireless AP.
>> +
>> + Say N to exclude this support and reduce the binary size.
>> +
>> + If unsure, say Y.
>> +
>
> Please do not ever add empty lines at the end of files, GIT warns
> about this when I try to apply your patch.
Hi Dave, I notice that this patch was never applied to "net-next". I
have one more patch contribution to make to the bridge code regarding
Proxy ARP IPv4. I would very much like this patch by Stephen to be
applied as well. I have no problem resubmitting this patch (with the
empty line removed). Do you want me to do that as part of the set?
Thanks!
- Kyeyoon
^ permalink raw reply
* [PATCH net-next] net: sctp: keep owned chunk in destructor_arg instead of skb->cb
From: Daniel Borkmann @ 2014-11-19 23:21 UTC (permalink / raw)
To: davem; +Cc: linux-sctp, netdev
It's just silly to hold the skb destructor argument around inside
skb->cb[] as we currently do in SCTP.
Though this has been around forever, I'm inclined to say that prior
to 4c3a5bdae293 ("sctp: Don't charge for data in sndbuf again when
transmitting packet") this may well have caused issues as doing so
violates the cb[] usage accross layers; before 4c3a5bdae293-times,
we have charged twice for data, and when destructor kicks in, cb[]
could have been overwritten already by someone else.
Nowadays, we're sort of cheating on data accounting in the sense
that due to commit 4c3a5bdae293, we orphan the skb already in the
SCTP output path, and use a different destructor only to make sure
the sk doesn't vanish on skb destruction time. Thus, cb[] is still
valid here as we operate within the SCTP layer. It's actually a big
candidate for future rework, imho.
Anyhow, lets keep the chunk in destructor_arg, as this is the actual
purpose for it so that in future, we don't run into trouble.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/sctp/socket.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 2120292..85e0b65 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -162,7 +162,7 @@ static inline void sctp_set_owner_w(struct sctp_chunk *chunk)
chunk->skb->destructor = sctp_wfree;
/* Save the chunk pointer in skb for sctp_wfree to use later. */
- *((struct sctp_chunk **)(chunk->skb->cb)) = chunk;
+ skb_shinfo(chunk->skb)->destructor_arg = chunk;
asoc->sndbuf_used += SCTP_DATA_SNDSIZE(chunk) +
sizeof(struct sk_buff) +
@@ -6870,14 +6870,10 @@ static void sctp_wake_up_waiters(struct sock *sk,
*/
static void sctp_wfree(struct sk_buff *skb)
{
- struct sctp_association *asoc;
- struct sctp_chunk *chunk;
- struct sock *sk;
+ struct sctp_chunk *chunk = skb_shinfo(skb)->destructor_arg;
+ struct sctp_association *asoc = chunk->asoc;
+ struct sock *sk = asoc->base.sk;
- /* Get the saved chunk pointer. */
- chunk = *((struct sctp_chunk **)(skb->cb));
- asoc = chunk->asoc;
- sk = asoc->base.sk;
asoc->sndbuf_used -= SCTP_DATA_SNDSIZE(chunk) +
sizeof(struct sk_buff) +
sizeof(struct sctp_chunk);
--
1.7.11.7
^ permalink raw reply related
* [net-next PATCH v2] ixgbe: Remove IXGBE_FLAG_IN_NETPOLL since it doesn't do anything
From: Alexander Duyck @ 2014-11-19 23:28 UTC (permalink / raw)
To: netdev; +Cc: donald.c.skidmore, jeffrey.t.kirsher
This patch removes some dead code from the cleanup path for ixgbe.
Setting and clearing the flag doesn't do anything since all we are doing is
setting the flag, scheduling napi, clearing the flag and then letting
netpoll do the polling cleanup. As such it doesn't make much sense to have
it there.
This patch also removes one minor white-space error.
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
v2: Fixed an unused variable warning for adapter in ixgbe_rx_skb
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 -
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 18 ++++--------------
2 files changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 86fa607..f7d46b3 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -610,7 +610,6 @@ struct ixgbe_adapter {
#define IXGBE_FLAG_RX_1BUF_CAPABLE (u32)(1 << 4)
#define IXGBE_FLAG_RX_PS_CAPABLE (u32)(1 << 5)
#define IXGBE_FLAG_RX_PS_ENABLED (u32)(1 << 6)
-#define IXGBE_FLAG_IN_NETPOLL (u32)(1 << 7)
#define IXGBE_FLAG_DCA_ENABLED (u32)(1 << 8)
#define IXGBE_FLAG_DCA_CAPABLE (u32)(1 << 9)
#define IXGBE_FLAG_IMIR_ENABLED (u32)(1 << 10)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 7c4d3b3..3dfec61 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1583,14 +1583,10 @@ static void ixgbe_process_skb_fields(struct ixgbe_ring *rx_ring,
static void ixgbe_rx_skb(struct ixgbe_q_vector *q_vector,
struct sk_buff *skb)
{
- struct ixgbe_adapter *adapter = q_vector->adapter;
-
if (ixgbe_qv_busy_polling(q_vector))
netif_receive_skb(skb);
- else if (!(adapter->flags & IXGBE_FLAG_IN_NETPOLL))
- napi_gro_receive(&q_vector->napi, skb);
else
- netif_rx(skb);
+ napi_gro_receive(&q_vector->napi, skb);
}
/**
@@ -6064,7 +6060,6 @@ static void ixgbe_check_hang_subtask(struct ixgbe_adapter *adapter)
/* Cause software interrupt to ensure rings are cleaned */
ixgbe_irq_rearm_queues(adapter, eics);
-
}
/**
@@ -7397,14 +7392,9 @@ static void ixgbe_netpoll(struct net_device *netdev)
if (test_bit(__IXGBE_DOWN, &adapter->state))
return;
- adapter->flags |= IXGBE_FLAG_IN_NETPOLL;
- if (adapter->flags & IXGBE_FLAG_MSIX_ENABLED) {
- for (i = 0; i < adapter->num_q_vectors; i++)
- ixgbe_msix_clean_rings(0, adapter->q_vector[i]);
- } else {
- ixgbe_intr(adapter->pdev->irq, netdev);
- }
- adapter->flags &= ~IXGBE_FLAG_IN_NETPOLL;
+ /* loop through and schedule all active queues */
+ for (i = 0; i < adapter->num_q_vectors; i++)
+ ixgbe_msix_clean_rings(0, adapter->q_vector[i]);
}
#endif
^ 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