Netdev List
 help / color / mirror / Atom feed
* Re: [net 0/3][pull request] Intel Wired LAN Driver Updates 2015-01-06
From: David Miller @ 2015-01-06 18:31 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <1420533864-13125-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue,  6 Jan 2015 00:44:21 -0800

> This series contains fixes to i40e only.
> 
> Jesse provides a fix for when the driver was polling with interrupts
> disabled the hardware would occasionally not write back descriptors.
> His fix causes the driver to detect this situation and force an interrupt
> to fire which will flush the stuck descriptor.
> 
> Anjali provides a couple of fixes, the first corrects an issue where
> the receive port checksum error counter was incrementing incorrectly with
> UDP encapsulated tunneled traffic.  The second fix resolves an issue where
> the driver was examining the outer protocol layer to set the inner protocol
> layer checksum offload.  In the case of TCP over IPv6 over an IPv4 based
> VXLAN, the inner checksum offloads would be set to look for IPv4/UDP
> instead of IPv6/TCP, so fixed the issue so that the driver will look at
> the proper layer for encapsulation offload settings.

Not pulled, waiting for a respin with the fix for patch #1, thanks.

^ permalink raw reply

* Re: [PATCH net-next v3 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key
From: Greg Rose @ 2015-01-06 18:30 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: Vlad Zolotarov, netdev, Avi Kivity, jeffrey.t.kirsher
In-Reply-To: <20150106180455.GB29721@cloudius-systems.com>

On Tue, Jan 6, 2015 at 10:04 AM, Gleb Natapov <gleb@cloudius-systems.com> wrote:
> On Tue, Jan 06, 2015 at 08:59:41AM -0800, Greg Rose wrote:
>> On Tue, Jan 6, 2015 at 2:58 AM, Vlad Zolotarov
>> <vladz@cloudius-systems.com> wrote:
>> >
>> >
>> > I agree with Gleb here: when we started with just thinking about the idea of
>> > this patch the possible security issue was the first thing that came into
>> > our minds.
>> > But eventually we couldn't come up with any security risk or attack example
>> > that is exclusively caused by the fact that VF knows the indirection table
>> > and/or RSS hash key of the PF.
>> >
>> > So, Greg, if we have missed anything and your have such an example could you
>> > share it here, please?
>>
>> I don't have any examples and that is not my area of expertise.  But
>> just because we can't think of a security risk or attack example
>> doesn't mean there isn't one.
>>
> Is RSS hash security feature at all? Against what kind of attack? It
> looks like some drivers (igb among them) use non random value for the key.

I don't believe RSS hashing itself is a security feature - I don't
know that sharing the RSS info with a VF is a security risk.  I'm just
asking that we preserve default behavior to avoid the possibility.

>
>> Just add a policy hook so that the system admin can decide whether
>> this information should be shared with the VFs and then we're covered
>> for cases of both known and unknown exploits, risks, etc.
>>
> Default off means that it will stay that way for most installations and
> information will not be available for "cloud" users. It is hard to get
> proper support on public cloud for less trivial issues than changing
> host HW configuration.

Someone in the host is configuring the VF HW to begin with.  Someone
had to create the VFs in the first place so I presume they could set
the policy for this feature as well at the same time.  To return to an
example I provided to Vlad - anti-spoof checking is on by default but
we allow system admins to turn it off so that other features, such as
bonding, can be used.  I just want to preserve current behavior while
allowing the feature you want to add to be available for those who
want it.

If Dave and the rest of community feel that there is no risk to these
patches and that they should be applied then I'll go away and shut up
about it.  But for now I'm just approaching this from a "better safe
than sorry" viewpoint.

Thanks,

- Greg

>
> --
>                         Gleb.

^ permalink raw reply

* Re: [PATCH net] r8152: support ndo_features_check
From: David Miller @ 2015-01-06 18:30 UTC (permalink / raw)
  To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-113-Taiwan-albertk@realtek.com>

From: Hayes Wang <hayeswang@realtek.com>
Date: Tue, 6 Jan 2015 17:41:58 +0800

> Support ndo_features_check to avoid:
>  - the transport offset is more than the hw limitation when using hw checksum.
>  - the skb->len of a GSO packet is more than the limitation.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>

Applied, thanks.

^ permalink raw reply

* Re: pull-request: mac80211 2015-01-06
From: David Miller @ 2015-01-06 18:30 UTC (permalink / raw)
  To: johannes-cdvu00un1VgdHxzADdlk8Q
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1420546548.1966.26.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>

From: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
Date: Tue, 06 Jan 2015 13:15:48 +0100

> Here's just a single fix - a revert of a patch that broke the
> p54 and cw2100 drivers (arguably due to bad assumptions there.)
> Since this affects kernels since 3.17, I decided to revert for
> now and we'll revisit this optimisation properly for -next.

Pulled, thanks Johannes.

> Also -- happy New Year, and sorry about the genl issue :(

No problem.

> I'm still a bit confused about whether you wanted the pull request with
> the text in the email or in the tag, or branch, or something - I
> interpreted what you said to Kalle differently to what you said to me.
> I've duplicated it for now :)

If you put it in the tag, that's fine, I'll just add my signoff to
it.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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-next] cxgb4: Add PCI device ID for new T5 adapter
From: David Miller @ 2015-01-06 18:27 UTC (permalink / raw)
  To: hariprasad; +Cc: netdev, leedom, nirranjan
In-Reply-To: <1420545706-13763-1-git-send-email-hariprasad@chelsio.com>

From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Tue,  6 Jan 2015 17:31:46 +0530

> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] arm_arch_timer: include clocksource.h directly
From: David Miller @ 2015-01-06 18:27 UTC (permalink / raw)
  To: richardcochran; +Cc: netdev, linux-kernel, linux
In-Reply-To: <1420550773-6752-1-git-send-email-richardcochran@gmail.com>

From: Richard Cochran <richardcochran@gmail.com>
Date: Tue,  6 Jan 2015 14:26:13 +0100

> This driver makes use of the clocksource code. Previously it had only
> included the proper header indirectly, but that chain was inadvertently
> broken by 74d23cc "time: move the timecounter/cyclecounter code into its
> own file."
> 
> This patch fixes the issue by including clocksource.h directly.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next v3 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key
From: Greg Rose @ 2015-01-06 18:22 UTC (permalink / raw)
  To: Vlad Zolotarov; +Cc: Gleb Natapov, netdev, Avi Kivity, jeffrey.t.kirsher
In-Reply-To: <54AC1BBA.3010206@cloudius-systems.com>

I accidentally replied just to Vlad - here is a reply to all.

On Tue, Jan 6, 2015 at 9:30 AM, Vlad Zolotarov
<vladz@cloudius-systems.com> wrote:
>
> On 01/06/15 18:59, Greg Rose wrote:
>>

[snip]


>>
>> I don't have any examples and that is not my area of expertise.  But
>> just because we can't think of a security risk or attack example
>> doesn't mean there isn't one.
>>
>> Just add a policy hook so that the system admin can decide whether
>> this information should be shared with the VFs and then we're covered
>> for cases of both known and unknown exploits, risks, etc.
>
>
> I absolutely disagree with u in regard of defining an RSS redirection table
> and RSS hash key as a security sensitive data. I don't know how u got to
> this conclusion.

I have not reached any such conclusion - let me reiterate:  I have no
idea.  It is not my area of expertise.  However, to take the lowest
risk route just add a policy hook so that a system admin can turn the
feature on through the PF driver (which is acknowledged as secure) if
they wish then there is no worry.

> However I don't want to argue about any longer. Let's move on.
>
> Let's clarify one thing about this "hook". Do u agree that it should cover
> only the cases when VF shares the mentioned above data with PF - namely for
> all devices but x550?

Look at how spoof checking is turned off/on for each VF using the "ip
link set" commands.  That's what I'm envisioning - some way to decide
on a per VF basis which VFs should be allowed to perform the query.

Thanks,

- Greg

^ permalink raw reply

* Re: [PATCH] b43legacy: Remove unused function
From: Rafał Miłecki @ 2015-01-06 18:19 UTC (permalink / raw)
  To: Rickard Strandqvist
  Cc: Larry Finger, Stefano Brivio, Network Development,
	linux-wireless@vger.kernel.org, Linux Kernel Mailing List,
	b43-dev
In-Reply-To: <1420567175-17323-1-git-send-email-rickard_strandqvist@spectrumdigital.se>

On 6 January 2015 at 18:59, Rickard Strandqvist
<rickard_strandqvist@spectrumdigital.se> wrote:
> Remove the function b43legacy_radio_set_tx_iq() that is not used anywhere.
>
> This was partially found by using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>

Acked-by: Rafał Miłecki <zajec5@gmail.com>

^ permalink raw reply

* Re: TCP connection issues against Amazon S3
From: Erik Grinaker @ 2015-01-06 18:17 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: linux-kernel, Yuchung Cheng, netdev
In-Reply-To: <1420564837.32621.30.camel@edumazet-glaptop2.roam.corp.google.com>


> On 06 Jan 2015, at 17:20, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2015-01-06 at 16:11 +0000, Erik Grinaker wrote:
>>> On 06 Jan 2015, at 16:04, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> On Tue, 2015-01-06 at 15:14 +0000, Erik Grinaker wrote:
>>>> (CCing Yuchung, as his name comes up in the relevant commits)
>>>> 
>>>> After upgrading from Ubuntu 12.04.5 to 14.04.1 we have begun seeing
>>>> intermittent TCP connection hangs for HTTP image requests against
>>>> Amazon S3. 3-5% of requests will suddenly stall in the middle of the
>>>> transfer before timing out. We see this problem across a range of
>>>> servers, in several data centres and networks, all located in Norway.
>>>> 
>>>> A packet dump [1] shows repeated ACK retransmits for some of the
>>>> requests. Using Ubuntu mainline kernels, we found the problem to have
>>>> been introduced between 3.11.10 and 3.12.0, possibly in
>>>> 0f7cc9a3c2bd89b15720dbf358e9b9e62af27126. The problem is also present
>>>> in 3.18.1. Disabling tcp_window_scaling seems to solve it, but has
>>>> obvious drawbacks for transfer speeds. Other sysctls do not seem to
>>>> affect it.
>>>> 
>>>> I am not sure if this is fundamentally a kernel bug or a network
>>>> issue, but we did not see this problem with older kernels.
>>>> 
>>>> [1] http://abstrakt.bengler.no/tcp-issues-s3.pcap.bz2
>>> 
>>> 
>>> CC netdev
>>> 
>>> This looks like the bug we fixed here :
>>> 
>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=39bb5e62867de82b269b07df900165029b928359
>> 
>> Has that patch gone into a release? Because the problem persists with 3.18.1.
> 
> Patch is in 3.18.1 yes.
> 
> So thats a separate issue. 
> 
> Can you confirm pcap was taken at receiver (195.159.221.106), not sender
> (54.231.136.74) , and on which host is running the 'buggy kernel' ?

Yes, pcap was taken on receiver (195.159.221.106).

> If the sender is broken, changing the kernel on receiver wont help.
> 
> BTW not using sack (on 54.231.132.98) is terrible for performance in
> lossy environments.

It may well be that the sender is broken; however, the sender is Amazon S3, so I do not have any control over it. And in any case, the problem goes away with 3.11.10 on receiver, but persists with 3.12.0 (or later) on receiver, so there must be some change in 3.12.0 which has caused this to trigger.

If you are confident that the problem is with Amazon, I can get in touch with their engineering department.

^ permalink raw reply

* Re: [PATCH] net: wireless: rtlwifi: btcoexist: halbtc8821a2ant:  Remove some unused functions
From: Kalle Valo @ 2015-01-06 18:14 UTC (permalink / raw)
  To: Rickard Strandqvist
  Cc: Larry Finger, Chaoming Li, Greg Kroah-Hartman, Fengguang Wu,
	linux-wireless, netdev, linux-kernel
In-Reply-To: <1420230361-1862-1-git-send-email-rickard_strandqvist@spectrumdigital.se>

Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se> writes:

> Removes some functions that are not used anywhere:
> ex_halbtc8821a2ant_periodical() ex_halbtc8821a2ant_halt_notify()
> ex_halbtc8821a2ant_bt_info_notify()
> ex_halbtc8821a2ant_special_packet_notify()
> ex_halbtc8821a2ant_connect_notify() ex_halbtc8821a2ant_scan_notify()
> ex_halbtc8821a2ant_lps_notify() ex_halbtc8821a2ant_ips_notify()
> ex_halbtc8821a2ant_display_coex_info() ex_halbtc8821a2ant_init_coex_dm()
> ex_halbtc8821a2ant_init_hwconfig()
>
> This was partially found by using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>

Rickard, I have dropped all your patches because I lost track which I
should apply and which not and I do not want to waste half an hour
figuring out. Please resend the ones which are still valid.

And to make it easier for everyone please group these wireless-drivers
cleanup patches into a larger patchset (like 10-15 patches max per
patchset). That way it's a lot easier for me tomanage them. Sending one
patch a day is not recommended, especially when we are talking trivial
cleanup patches like this.

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH net-next v3 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key
From: Gleb Natapov @ 2015-01-06 18:04 UTC (permalink / raw)
  To: Greg Rose; +Cc: Vlad Zolotarov, netdev, Avi Kivity, jeffrey.t.kirsher
In-Reply-To: <CALgkqUqh4uMxhnTA9yOm2m0Z3tOZ1E2+Z8Y68Kv5HJ6vP=Fk2A@mail.gmail.com>

On Tue, Jan 06, 2015 at 08:59:41AM -0800, Greg Rose wrote:
> On Tue, Jan 6, 2015 at 2:58 AM, Vlad Zolotarov
> <vladz@cloudius-systems.com> wrote:
> >
> > On 01/06/15 08:55, Gleb Natapov wrote:
> >>
> >> On Mon, Jan 05, 2015 at 03:54:52PM -0800, Greg Rose wrote:
> >>>
> >>> On Mon, Jan 5, 2015 at 6:15 AM, Vlad Zolotarov
> >>> <vladz@cloudius-systems.com> wrote:
> >>>>
> >>>> Add the ethtool ops to VF driver to allow querying the RSS indirection
> >>>> table
> >>>> and RSS Random Key.
> >>>>
> >>>>   - PF driver: Add new VF-PF channel commands.
> >>>>   - VF driver: Utilize these new commands and add the corresponding
> >>>>                ethtool callbacks.
> >>>>
> >>>> New in v3:
> >>>>     - Added a missing support for x550 devices.
> >>>>     - Mask the indirection table values according to PSRTYPE[n].RQPL.
> >>>>     - Minimized the number of added VF-PF commands.
> >>>>
> >>>> New in v2:
> >>>>     - Added a detailed description to patches 4 and 5.
> >>>>
> >>>> New in v1 (compared to RFC):
> >>>>     - Use "if-else" statement instead of a "switch-case" for a single
> >>>> option case.
> >>>>       More specifically: in cases where the newly added API version is
> >>>> the only one
> >>>>       allowed. We may consider using a "switch-case" back again when the
> >>>> list of
> >>>>       allowed API versions in these specific places grows up.
> >>>>
> >>>> Vlad Zolotarov (5):
> >>>>    ixgbe: Add a RETA query command to VF-PF channel API
> >>>>    ixgbevf: Add a RETA query code
> >>>>    ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
> >>>>    ixgbevf: Add RSS Key query code
> >>>>    ixgbevf: Add the appropriate ethtool ops to query RSS indirection
> >>>>      table and key
> >>>>
> >>>>   drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h      |  10 ++
> >>>>   drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c    |  91
> >>>> +++++++++++++++
> >>>>   drivers/net/ethernet/intel/ixgbevf/ethtool.c      |  43 +++++++
> >>>>   drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   4 +-
> >>>>   drivers/net/ethernet/intel/ixgbevf/mbx.h          |  10 ++
> >>>>   drivers/net/ethernet/intel/ixgbevf/vf.c           | 132
> >>>> ++++++++++++++++++++++
> >>>>   drivers/net/ethernet/intel/ixgbevf/vf.h           |   2 +
> >>>>   7 files changed, 291 insertions(+), 1 deletion(-)
> >>>
> >>> I've given this code a review and I don't see a way to
> >>> set a policy in the PF driver as to whether this request should be
> >>> allowed or not.  We cannot enable this query by default - it is a
> >>> security risk. To make this acceptable you need to do a
> >>> couple of things.
> >>>
> >> Can you please elaborate on the security risk this information poses?
> >> Is toeplitz hash function cryptographically strong enough so that VF
> >> cannot reconstruct the hash key from hash result provided in packet
> >> descriptor? The abstract of this paper [1] claims it is not, but I do
> >> not have access to the full article unfortunately hence the question.
> >>
> >> [1]
> >> http://ieeexplore.ieee.org/xpl/login.jsp?tp=&arnumber=5503170&url=http%3A%2F%2Fieeexplore.ieee.org%2Fxpls%2Fabs_all.jsp%3Farnumber%3D5503170
> >
> >
> > I agree with Gleb here: when we started with just thinking about the idea of
> > this patch the possible security issue was the first thing that came into
> > our minds.
> > But eventually we couldn't come up with any security risk or attack example
> > that is exclusively caused by the fact that VF knows the indirection table
> > and/or RSS hash key of the PF.
> >
> > So, Greg, if we have missed anything and your have such an example could you
> > share it here, please?
> 
> I don't have any examples and that is not my area of expertise.  But
> just because we can't think of a security risk or attack example
> doesn't mean there isn't one.
> 
Is RSS hash security feature at all? Against what kind of attack? It
looks like some drivers (igb among them) use non random value for the key.

> Just add a policy hook so that the system admin can decide whether
> this information should be shared with the VFs and then we're covered
> for cases of both known and unknown exploits, risks, etc.
> 
Default off means that it will stay that way for most installations and
information will not be available for "cloud" users. It is hard to get
proper support on public cloud for less trivial issues than changing
host HW configuration.

--
			Gleb.

^ permalink raw reply

* Re: route/max_size sysctl in ipv4
From: Eric Dumazet @ 2015-01-06 18:09 UTC (permalink / raw)
  To: Ani Sinha; +Cc: Pádraig Brady, David Miller, netdev@vger.kernel.org
In-Reply-To: <CAOxq_8O-YME=m9NnCQ7hPWjr6jWqcqUkgeqM=ZY6VW+icNsO2g@mail.gmail.com>

On Tue, 2015-01-06 at 09:11 -0800, Ani Sinha wrote:

> Why can't se simply change the documentation to reflect the fact that
> this sysctl is no longer in operation?

It is still in operation for IPv6

Looks like you propose to update the documentation.

This is great !

Why don't you send an official patch ?

^ permalink raw reply

* Re: [PATCH] b43legacy: Remove unused function
From: Larry Finger @ 2015-01-06 18:08 UTC (permalink / raw)
  To: Rickard Strandqvist
  Cc: Kalle Valo, linux-wireless, b43-dev, netdev, linux-kernel
In-Reply-To: <1420567175-17323-1-git-send-email-rickard_strandqvist@spectrumdigital.se>

On 01/06/2015 11:59 AM, Rickard Strandqvist wrote:
> Remove the function b43legacy_radio_set_tx_iq() that is not used anywhere.
>
> This was partially found by using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
>   drivers/net/wireless/b43legacy/radio.c |   19 -------------------
>   drivers/net/wireless/b43legacy/radio.h |    1 -
>   2 files changed, 20 deletions(-)

Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Larry

>
> diff --git a/drivers/net/wireless/b43legacy/radio.c b/drivers/net/wireless/b43legacy/radio.c
> index 8961776..9501420 100644
> --- a/drivers/net/wireless/b43legacy/radio.c
> +++ b/drivers/net/wireless/b43legacy/radio.c
> @@ -1743,25 +1743,6 @@ u16 freq_r3A_value(u16 frequency)
>   	return value;
>   }
>
> -void b43legacy_radio_set_tx_iq(struct b43legacy_wldev *dev)
> -{
> -	static const u8 data_high[5] = { 0x00, 0x40, 0x80, 0x90, 0xD0 };
> -	static const u8 data_low[5]  = { 0x00, 0x01, 0x05, 0x06, 0x0A };
> -	u16 tmp = b43legacy_radio_read16(dev, 0x001E);
> -	int i;
> -	int j;
> -
> -	for (i = 0; i < 5; i++) {
> -		for (j = 0; j < 5; j++) {
> -			if (tmp == (data_high[i] | data_low[j])) {
> -				b43legacy_phy_write(dev, 0x0069, (i - j) << 8 |
> -						    0x00C0);
> -				return;
> -			}
> -		}
> -	}
> -}
> -
>   int b43legacy_radio_selectchannel(struct b43legacy_wldev *dev,
>   				  u8 channel,
>   				  int synthetic_pu_workaround)
> diff --git a/drivers/net/wireless/b43legacy/radio.h b/drivers/net/wireless/b43legacy/radio.h
> index bccb3d7..dd2976d 100644
> --- a/drivers/net/wireless/b43legacy/radio.h
> +++ b/drivers/net/wireless/b43legacy/radio.h
> @@ -92,7 +92,6 @@ void b43legacy_nrssi_hw_write(struct b43legacy_wldev *dev, u16 offset, s16 val);
>   void b43legacy_nrssi_hw_update(struct b43legacy_wldev *dev, u16 val);
>   void b43legacy_nrssi_mem_update(struct b43legacy_wldev *dev);
>
> -void b43legacy_radio_set_tx_iq(struct b43legacy_wldev *dev);
>   u16 b43legacy_radio_calibrationvalue(struct b43legacy_wldev *dev);
>
>   #endif /* B43legacy_RADIO_H_ */
>

^ permalink raw reply

* Re: [PATCH] net: wireless: ipw2x00: ipw2200.c:  Remove unused function
From: Kalle Valo @ 2015-01-06 18:04 UTC (permalink / raw)
  To: Rickard Strandqvist
  Cc: Stanislav Yakovlev, John W. Linville, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <1419078599-31118-1-git-send-email-rickard_strandqvist@spectrumdigital.se>

Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se> writes:

> Remove the function ipw_alive() that is not used anywhere.
>
> This was partially found by using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>

Please improve the patch title and make it more unique like:

ipw2200: remove unused ipw_alive()

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH] net: wireless: b43legacy: radio.c: Remove unused function
From: Rickard Strandqvist @ 2015-01-06 18:01 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Sedat Dilek, Larry Finger, Stefano Brivio, Network Development,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Kernel Mailing List, b43-dev
In-Reply-To: <CACna6rxF1OKRZSpaL=+XzSV2kCpMH5LuB-B+eWV7zF8MJ-9tog-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

2015-01-05 7:34 GMT+01:00 Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> On 3 January 2015 at 13:28, Rickard Strandqvist
> <rickard_strandqvist-IW2WV5XWFqGZkjO+N0TKoMugMpMbD5Xr@public.gmane.org> wrote:
>> 2015-01-02 22:34 GMT+01:00 Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>>>
>>> 1) I gave you Ack for the changes
>>> 2) You could drop "net: wireless: " or better use something Sedat proposed
>>
>> Nice, yes I miss the Ack :)
>>
>> I just got one more complaining about my subject-line in "net: wireless: "
>> I use some sed call for this, so it's easy to fix. I will now remove that
>> part hereinafter.
>
> Will you re-send this patch with a proper subject then, please?
>
> --
> Rafał


Hi Rafał

Ok, resent the patch.

Just a thought about this naming.
Now I have changed in radio.c and radio.h, what happens when I send
another patch for other files in the same directory, the caption will
then be identical.


Kind regards
Rickard Strandqvist
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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

* [PATCH] b43legacy: Remove unused function
From: Rickard Strandqvist @ 2015-01-06 17:59 UTC (permalink / raw)
  To: Larry Finger, Stefano Brivio
  Cc: Rickard Strandqvist, Kalle Valo,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Remove the function b43legacy_radio_set_tx_iq() that is not used anywhere.

This was partially found by using a static code analysis program called cppcheck.

Signed-off-by: Rickard Strandqvist <rickard_strandqvist-IW2WV5XWFqGZkjO+N0TKoMugMpMbD5Xr@public.gmane.org>
---
 drivers/net/wireless/b43legacy/radio.c |   19 -------------------
 drivers/net/wireless/b43legacy/radio.h |    1 -
 2 files changed, 20 deletions(-)

diff --git a/drivers/net/wireless/b43legacy/radio.c b/drivers/net/wireless/b43legacy/radio.c
index 8961776..9501420 100644
--- a/drivers/net/wireless/b43legacy/radio.c
+++ b/drivers/net/wireless/b43legacy/radio.c
@@ -1743,25 +1743,6 @@ u16 freq_r3A_value(u16 frequency)
 	return value;
 }
 
-void b43legacy_radio_set_tx_iq(struct b43legacy_wldev *dev)
-{
-	static const u8 data_high[5] = { 0x00, 0x40, 0x80, 0x90, 0xD0 };
-	static const u8 data_low[5]  = { 0x00, 0x01, 0x05, 0x06, 0x0A };
-	u16 tmp = b43legacy_radio_read16(dev, 0x001E);
-	int i;
-	int j;
-
-	for (i = 0; i < 5; i++) {
-		for (j = 0; j < 5; j++) {
-			if (tmp == (data_high[i] | data_low[j])) {
-				b43legacy_phy_write(dev, 0x0069, (i - j) << 8 |
-						    0x00C0);
-				return;
-			}
-		}
-	}
-}

^ permalink raw reply related

* Re: [PATCH net-next 1/3] net: add IPv4 routing FIB support for swdev
From: Scott Feldman @ 2015-01-06 17:51 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: Netdev, Jiří Pírko, john fastabend, Thomas Graf,
	Jamal Hadi Salim, Andy Gospodarek, Roopa Prabhu
In-Reply-To: <1420552709.32369.50.camel@stressinduktion.org>

On Tue, Jan 6, 2015 at 5:58 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Hi Scott,
>
> On Do, 2015-01-01 at 19:29 -0800, sfeldma@gmail.com wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> To offload IPv4 L3 routing functions to swdev device, the swdev device driver
>> implements two new ndo ops (ndo_switch_fib_ipv4_add/del).  The ops are called
>> by the core IPv4 FIB code when installing/removing FIB entries to/from the
>> kernel FIB.  On install, the driver should return 0 if FIB entry (route) can be
>> installed to device for offloading, -EOPNOTSUPP if route cannot be installed
>> due to device limitations, and other negative error code on failure to install
>> route to device.  On failure error code, the route is not installed to device,
>> and not installed in kernel FIB, and the return code is propagated back to the
>> user-space caller (via netlink).  An -EOPNOTSUPP error code is skipped for the
>> device but installed in the kernel FIB.
>>
>> The FIB entry (route) nexthop list is used to find the swdev device port to
>> anchor the ndo op call.  The route's fib_dev (the first nexthop's dev) is used
>> find the swdev port by recursively traversing the fib_dev's lower_dev list
>> until a swdev port is found.  The ndo op is called on this swdev port.
>>
>> Since the FIB entry is "naked" when push from the kernel, the driver/device
>> is responsible for resolving the route's nexthops to neighbor MAC addresses.
>> This can be done by the driver by monitoring NETEVENT_NEIGH_UPDATE
>> netevent notifier to watch for ARP activity.  Once a nexthop is resolved to
>> neighbor MAC address, it can be installed to the device and the device will
>> do the L3 routing offload in HW, for that nexthop.
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>>  include/linux/netdevice.h |   22 +++++++++++
>>  include/net/switchdev.h   |   18 +++++++++
>>  net/ipv4/fib_trie.c       |   17 ++++++++-
>>  net/switchdev/switchdev.c |   89 +++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 145 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 679e6e9..b66d22b 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -767,6 +767,8 @@ struct netdev_phys_item_id {
>>  typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>>                                      struct sk_buff *skb);
>>
>> +struct fib_info;
>> +
>>  /*
>>   * This structure defines the management hooks for network devices.
>>   * The following hooks can be defined; unless noted otherwise, they are
>> @@ -1030,6 +1032,14 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>>   * int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 state);
>>   *   Called to notify switch device port of bridge port STP
>>   *   state change.
>> + * int (*ndo_sw_parent_fib_ipv4_add)(struct net_device *dev, __be32 dst,
>> + *                                int dst_len, struct fib_info *fi,
>> + *                                u8 tos, u8 type, u32 tb_id);
>> + *   Called to add IPv4 route to switch device.
>> + * int (*ndo_sw_parent_fib_ipv4_del)(struct net_device *dev, __be32 dst,
>> + *                                int dst_len, struct fib_info *fi,
>> + *                                u8 tos, u8 type, u32 tb_id);
>> + *   Called to delete IPv4 route from switch device.
>>   */
>>  struct net_device_ops {
>>       int                     (*ndo_init)(struct net_device *dev);
>> @@ -1189,6 +1199,18 @@ struct net_device_ops {
>>                                                           struct netdev_phys_item_id *psid);
>>       int                     (*ndo_switch_port_stp_update)(struct net_device *dev,
>>                                                             u8 state);
>> +     int                     (*ndo_switch_fib_ipv4_add)(struct net_device *dev,
>> +                                                        __be32 dst,
>> +                                                        int dst_len,
>> +                                                        struct fib_info *fi,
>> +                                                        u8 tos, u8 type,
>> +                                                        u32 tb_id);
>> +     int                     (*ndo_switch_fib_ipv4_del)(struct net_device *dev,
>> +                                                        __be32 dst,
>> +                                                        int dst_len,
>> +                                                        struct fib_info *fi,
>> +                                                        u8 tos, u8 type,
>> +                                                        u32 tb_id);
>>  #endif
>>  };
>
> At this point I would like to start the discussion about handling of the
> table ids/vrfs (again :) ): as I can see it, this version just passes
> table ids down to the driver layer and the rocker driver filters them by
> local/main table? This seems to be mostly fine for a first version but
> does not feel like it will integrate well with the rest of the linux
> networking ecosystem.
>
> Will hardware have the capabilities to do programmable matches like "ip
> rule" is currently capable to do? Should we plan for that? Do we want to
> support hardware which does support multiple tables/VRFs?

Good questions, thanks for bringing these up.

>
> I would like to present a first suggestion:
> My take on this would be strive towards an integration with ip-rule, so
> we add tables which will be offloaded to hardware. This happens only in
> situations where those tables will be the first match for incoming
> packets specified with an in-interface filter which has the capability
> to do the offloading (for example). The determination if the table is
> capable for hardware offloading should be done automatically, so if
> later hardware will be capable of doing ip rule like matches, we can
> just expand the check which flags the tables accordingly.

Sounds like a good suggestion to me.  We need to think about what the
swdev API looks like to the switch device driver.  Could you take a
stab at defining what integration with ip-rule looks like, code-wise,
at the swdev API layer?

With the rocker device we're prototyping with, the standard LPM on IP
dst is the normal L3 routing table structure.  Within that, table
priorities could be handled, so routes in one table take precedence
over routes in another table.  If we want to do policy routing, then
we'd need to use the ACL table in rocker to match on other fields
besides just IP dst.

> Anyway, if hardware supports multiple tables or VRFs, it is better to
> manage pass in a pointer where drivers can embed private data for
> management, I think.
>
> Thanks,
> Hannes
>
>
>

^ permalink raw reply

* Re: [net-next PATCH v1 08/11] net: rocker: add get flow API operation
From: John Fastabend @ 2015-01-06 17:50 UTC (permalink / raw)
  To: Scott Feldman
  Cc: Thomas Graf, Jiří Pírko, Jamal Hadi Salim,
	simon.horman@netronome.com, Netdev, David S. Miller,
	Andy Gospodarek
In-Reply-To: <CAE4R7bDUiLGQnEHs1jemqjrQkzObN5jCkAa3LdWr90KGysrm5w@mail.gmail.com>

On 01/06/2015 08:57 AM, Scott Feldman wrote:
> On Tue, Jan 6, 2015 at 6:59 AM, John Fastabend <john.fastabend@gmail.com> wrote:
>> On 01/05/2015 11:40 PM, Scott Feldman wrote:
>>>
>>> On Wed, Dec 31, 2014 at 11:48 AM, John Fastabend
>>> <john.fastabend@gmail.com> wrote:
>>>>
>>>> Add operations to get flows. I wouldn't mind cleaning this code
>>>> up a bit but my first attempt to do this used macros which shortered
>>>> the code up but when I was done I decided it just made the code
>>>> unreadable and unmaintainable.
>>>>
>>>> I might think about it a bit more but this implementation albeit
>>>> a bit long and repeatative is easier to understand IMO.
>>>
>>>
>>> Dang, you put a lot of work into this one.
>>>
>>> Something doesn't feel right though.  In this case, rocker driver just
>>> happened to have cached all the flow/group stuff in hash tables in
>>> software, so you don't need to query thru to the device to extract the
>>> if_flow info.  What doesn't feel right is all the work need in the
>>> driver.  For each and every driver.  get_flows needs to go above
>>> driver, somehow.
>>
>>
>> Another option is to have a software cache in the flow_table.c I
>> was trying to avoid caching as I really don't expect 'get' operations
>> to be fast path and going to hardware seems good enough for me.
>> Other than its a bit annoying to write the mapping code.
>
> Caching in flow_table.c seems best to me as drivers/devices don't need
> to be involved and the cache can server multiple users of the API.
> Are there cases where the device could get flow table entries
> installed/deleted outside the API?  For example, if the device was
> learning MAC addresses, and did automatic table insertions.  We worked
> around that case with the recent L2 swdev support by pushing learned
> MAC addrs up to bridge's FDB so software and hardware tables stay
> synced.
>

OK I guess I'm convinced. I'll go ahead and cache the flow entries in
software. I'll work this into v2.

-- 
John Fastabend         Intel Corporation

^ permalink raw reply

* Re: [net-next PATCH v1 04/11] rocker: add pipeline model for rocker switch
From: John Fastabend @ 2015-01-06 17:49 UTC (permalink / raw)
  To: Scott Feldman
  Cc: Thomas Graf, Jiří Pírko, Jamal Hadi Salim,
	simon.horman@netronome.com, Netdev, David S. Miller,
	Andy Gospodarek
In-Reply-To: <CAE4R7bAHY8=TJ_hZ5X72bB5fca22zg2L8FgKJdRdRZGDWpSvcA@mail.gmail.com>

On 01/06/2015 09:16 AM, Scott Feldman wrote:
> On Tue, Jan 6, 2015 at 9:00 AM, John Fastabend <john.fastabend@gmail.com> wrote:
>> On 01/05/2015 11:01 PM, Scott Feldman wrote:
>>>> +
>>>> +struct net_flow_jump_table parse_ethernet[3] = {
>>>> +       {
>>>> +               .field = {
>>>> +                  .header = HEADER_ETHERNET,
>>>> +                  .field = HEADER_ETHERNET_ETHERTYPE,
>>>> +                  .type = NET_FLOW_FIELD_REF_ATTR_TYPE_U16,
>>>> +                  .value_u16 = 0x0800,
>
> ETH_P_IP, etc
>
>>>
>>>
>>> How is htons/ntohs conversions happening here?
>>
>>
>> my current stance is to leave everything in host order in the model
>> and let the drivers do conversions as needed. For example some drivers
>> want the vlan vid in host order others network order. I think its
>> more readable above then with hton*() throughout.
>
> Hmmm...I would argue adding htons/htonl makes it more readable in the
> sense that it's a reminder that this is a field in a network header,
> to be used for matching against packet headers, which use
> network-ordering.  Store the field in the order best for comparison
> with the raw pkt data.  Drivers may still need to do some conversion
> if the field is programmed in hardware in a diff order.
>

Easy enough here, but then when we set_flows what do we use
network-ordering or host? If it can be network-order in some
cases and host-order in others its hard to resolve pragmatically.
Humans at a CLI can most likely get it right for well known fields
such as VLAN IDs but for less common fields (maybe proprietary)
or management software it gets tricky.

I guess we could add a flag to indicate byte ordering.

-- 
John Fastabend         Intel Corporation

^ permalink raw reply

* [PATCH] net: ethernet: cpsw: ignore VLAN ID 1
From: Felipe Balbi @ 2015-01-06 17:43 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Linux OMAP Mailing List, Felipe Balbi, stable,
	Mugunthan V N

CPSW completely hangs if we add, and later remove,
VLAN ID #1. What happens is that after removing
VLAN ID #1, no packets will be received by CPSW
rendering network unusable.

In order to "fix" the issue, we're returning -EINVAL
if anybody tries to add VLAN ID #1. While at that,
also filter out any ID > 4095 because we only have
12 bits for VLAN IDs.

Fixes: 3b72c2f (drivers: net:ethernet: cpsw: add support for VLAN)
Cc: <stable@vger.kernel.org> # v3.9+
Cc: Mugunthan V N <mugunthanvnm@ti.com>
Tested-by: Schuyler Patton <spatton@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index e61ee8351272..028bb7f3de65 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1669,6 +1669,13 @@ static int cpsw_ndo_vlan_rx_add_vid(struct net_device *ndev,
 	if (vid == priv->data.default_vlan)
 		return 0;
 
+	/* NOTICE: CPSW does not support VID 1. We should
+	 * also filter out VID > 4095 as we only have 12
+	 * bits for VID entries
+	 */
+	if (vid == 1 || vid >= VLAN_N_VID)
+		return -EINVAL;
+
 	dev_info(priv->dev, "Adding vlanid %d to vlan filter\n", vid);
 	return cpsw_add_vlan_ale_entry(priv, vid);
 }
@@ -1682,6 +1689,13 @@ static int cpsw_ndo_vlan_rx_kill_vid(struct net_device *ndev,
 	if (vid == priv->data.default_vlan)
 		return 0;
 
+	/* NOTICE: CPSW does not support VID 1. We should
+	 * also filter out VID > 4095 as we only have 12
+	 * bits for VID entries
+	 */
+	if (vid == 1 || vid >= VLAN_N_VID)
+		return -EINVAL;
+
 	dev_info(priv->dev, "removing vlanid %d from vlan filter\n", vid);
 	ret = cpsw_ale_del_vlan(priv->ale, vid, 0);
 	if (ret != 0)
-- 
2.2.0

^ permalink raw reply related

* Re: [PATCH net-next v2 0/8] net: extend ethtool link mode bitmaps to 48 bits
From: David Decotigny @ 2015-01-06 17:36 UTC (permalink / raw)
  To: Amir Vadai
  Cc: Florian Fainelli, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
	Saeed Mahameed, David S. Miller, Jason Wang, Michael S. Tsirkin,
	Herbert Xu, Al Viro, Ben Hutchings, Masatake YAMATO, Xi Wang,
	Neil Horman, WANG Cong, Flavio Leitner, Tom Gundersen, Jiri Pirko,
	Vlad Yasevich, Eric W. Biederman, Venkata Duvvuru,
	Govindarajulu Varadarajan
In-Reply-To: <54ABE991.3040107@mellanox.com>

Interesting. It seems that the band-aid I was proposing is already
obsolete. We could still use the remaining reserved 16 bits to encode
5 more bits per mask (that is: 53 bits / mask total). But if I
understand you, it would allow us to survive only a few months longer,
as opposed to a few weeks.

One short-term alternative solution I can imagine is the following:
/* For example bitmap-based for variable length: */
struct ethtool_link_mode {
  __u32 cmd;
  __u8 autoneg :1;
  __u8 duplex :2;
 __u16 supported_nbits;
  __u16 advertising_nbits;
  __u16 lp_advertising_nbits;
  __u32 reserved[4];
  __u8 masks[0];
};
/* Or simpler, statically limited to 64b / mask, but easier to migrate
to for driver authors: */
struct ethtool_link_mode {
  __u32 cmd;
  __u8 autoneg :1;
  __u8 duplex :2;
   __u64 supported;
  __u64 advertising;
  __u64 lp_advertising;
  __u32 reserved[4];
};
#define ETHTOOL_GLINK_MODE 0x0000004a
#define ETHTOOL_SLINK_MODE 0x0000004b
struct ethtool_ops {
...
   int (*get_link_mode)(struct net_device *, struct ethtool_link_mode *);
   int (*set_link_mode)(struct net_device *, struct ethtool_link_mode *);
};

The same thing required for EEE.

I am not sure about moving the autoneg and duplex fields into the new
struct. Especially the "duplex" field.

Then the idea would be to update the ethtool user-space tool to try
get/set_link mode when reporting/changing the autoneg/advertising
settings.

Both will require significant effort from the driver authors.
Especially if the variable-length bitmap approach is preferred:
 - most drivers currently use simple bitwise arithmetic in their code,
and that goes far beyond get/set_settings, it is sometimes part of the
core driver logic. They will have to migrate to the bitmap API if they
want to use the larger bitmaps (note: no change needed if they are
happy with <= 32b / mask)
 - we would have to progressively deprecate the use of #define
ADVERTISED_1000baseT_Full in favor of an enum of the bit indices.

Any feedback welcome. In the meantime, I am going to propose a v3 of
current option with 53 bits / mask. I can also propose a prototype of
the scheme described above, please let me know.

On Tue, Jan 6, 2015 at 5:56 AM, Amir Vadai <amirv@mellanox.com> wrote:
> On 1/6/2015 4:54 AM, David Decotigny wrote:
>> This patch series increases the width of the supported/advertising
>> ethtool masks from 32 bits to 48. This should allow to breathe for a
>> couple more years (or... months?).
> Hi,
>
> Nice work.
> What worries me is that it is going to be for weeks more than years or
> months...
>
> Mellanox is about to release next month a driver for a new NIC, with 3
> new speeds * few link modes for each + new link modes for 10G.
> It seems that we will need to consume almost all the new bits.
>
> Amir

^ permalink raw reply

* Re: [PATCH net-next v3 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key
From: Vlad Zolotarov @ 2015-01-06 17:30 UTC (permalink / raw)
  To: Greg Rose; +Cc: Gleb Natapov, netdev, Avi Kivity, jeffrey.t.kirsher
In-Reply-To: <CALgkqUqh4uMxhnTA9yOm2m0Z3tOZ1E2+Z8Y68Kv5HJ6vP=Fk2A@mail.gmail.com>


On 01/06/15 18:59, Greg Rose wrote:
> On Tue, Jan 6, 2015 at 2:58 AM, Vlad Zolotarov
> <vladz@cloudius-systems.com> wrote:
>> On 01/06/15 08:55, Gleb Natapov wrote:
>>> On Mon, Jan 05, 2015 at 03:54:52PM -0800, Greg Rose wrote:
>>>> On Mon, Jan 5, 2015 at 6:15 AM, Vlad Zolotarov
>>>> <vladz@cloudius-systems.com> wrote:
>>>>> Add the ethtool ops to VF driver to allow querying the RSS indirection
>>>>> table
>>>>> and RSS Random Key.
>>>>>
>>>>>    - PF driver: Add new VF-PF channel commands.
>>>>>    - VF driver: Utilize these new commands and add the corresponding
>>>>>                 ethtool callbacks.
>>>>>
>>>>> New in v3:
>>>>>      - Added a missing support for x550 devices.
>>>>>      - Mask the indirection table values according to PSRTYPE[n].RQPL.
>>>>>      - Minimized the number of added VF-PF commands.
>>>>>
>>>>> New in v2:
>>>>>      - Added a detailed description to patches 4 and 5.
>>>>>
>>>>> New in v1 (compared to RFC):
>>>>>      - Use "if-else" statement instead of a "switch-case" for a single
>>>>> option case.
>>>>>        More specifically: in cases where the newly added API version is
>>>>> the only one
>>>>>        allowed. We may consider using a "switch-case" back again when the
>>>>> list of
>>>>>        allowed API versions in these specific places grows up.
>>>>>
>>>>> Vlad Zolotarov (5):
>>>>>     ixgbe: Add a RETA query command to VF-PF channel API
>>>>>     ixgbevf: Add a RETA query code
>>>>>     ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
>>>>>     ixgbevf: Add RSS Key query code
>>>>>     ixgbevf: Add the appropriate ethtool ops to query RSS indirection
>>>>>       table and key
>>>>>
>>>>>    drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h      |  10 ++
>>>>>    drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c    |  91
>>>>> +++++++++++++++
>>>>>    drivers/net/ethernet/intel/ixgbevf/ethtool.c      |  43 +++++++
>>>>>    drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   4 +-
>>>>>    drivers/net/ethernet/intel/ixgbevf/mbx.h          |  10 ++
>>>>>    drivers/net/ethernet/intel/ixgbevf/vf.c           | 132
>>>>> ++++++++++++++++++++++
>>>>>    drivers/net/ethernet/intel/ixgbevf/vf.h           |   2 +
>>>>>    7 files changed, 291 insertions(+), 1 deletion(-)
>>>> I've given this code a review and I don't see a way to
>>>> set a policy in the PF driver as to whether this request should be
>>>> allowed or not.  We cannot enable this query by default - it is a
>>>> security risk. To make this acceptable you need to do a
>>>> couple of things.
>>>>
>>> Can you please elaborate on the security risk this information poses?
>>> Is toeplitz hash function cryptographically strong enough so that VF
>>> cannot reconstruct the hash key from hash result provided in packet
>>> descriptor? The abstract of this paper [1] claims it is not, but I do
>>> not have access to the full article unfortunately hence the question.
>>>
>>> [1]
>>> http://ieeexplore.ieee.org/xpl/login.jsp?tp=&arnumber=5503170&url=http%3A%2F%2Fieeexplore.ieee.org%2Fxpls%2Fabs_all.jsp%3Farnumber%3D5503170
>>
>> I agree with Gleb here: when we started with just thinking about the idea of
>> this patch the possible security issue was the first thing that came into
>> our minds.
>> But eventually we couldn't come up with any security risk or attack example
>> that is exclusively caused by the fact that VF knows the indirection table
>> and/or RSS hash key of the PF.
>>
>> So, Greg, if we have missed anything and your have such an example could you
>> share it here, please?
> I don't have any examples and that is not my area of expertise.  But
> just because we can't think of a security risk or attack example
> doesn't mean there isn't one.
>
> Just add a policy hook so that the system admin can decide whether
> this information should be shared with the VFs and then we're covered
> for cases of both known and unknown exploits, risks, etc.

I absolutely disagree with u in regard of defining an RSS redirection 
table and RSS hash key as a security sensitive data. I don't know how u 
got to this conclusion.
However I don't want to argue about any longer. Let's move on.

Let's clarify one thing about this "hook". Do u agree that it should 
cover only the cases when VF shares the mentioned above data with PF - 
namely for all devices but x550?

thanks,
vlad

>
> Thanks,
>
> - Greg
>
>> Thanks,
>> vlad
>>
>>> --
>>>                          Gleb.
>>

^ permalink raw reply

* Re: TCP connection issues against Amazon S3
From: Eric Dumazet @ 2015-01-06 17:20 UTC (permalink / raw)
  To: Erik Grinaker; +Cc: linux-kernel, Yuchung Cheng, netdev
In-Reply-To: <D5C36777-4461-4B8D-B6C4-38B4276F42DC@bengler.no>

On Tue, 2015-01-06 at 16:11 +0000, Erik Grinaker wrote:
> > On 06 Jan 2015, at 16:04, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Tue, 2015-01-06 at 15:14 +0000, Erik Grinaker wrote:
> >> (CCing Yuchung, as his name comes up in the relevant commits)
> >> 
> >> After upgrading from Ubuntu 12.04.5 to 14.04.1 we have begun seeing
> >> intermittent TCP connection hangs for HTTP image requests against
> >> Amazon S3. 3-5% of requests will suddenly stall in the middle of the
> >> transfer before timing out. We see this problem across a range of
> >> servers, in several data centres and networks, all located in Norway.
> >> 
> >> A packet dump [1] shows repeated ACK retransmits for some of the
> >> requests. Using Ubuntu mainline kernels, we found the problem to have
> >> been introduced between 3.11.10 and 3.12.0, possibly in
> >> 0f7cc9a3c2bd89b15720dbf358e9b9e62af27126. The problem is also present
> >> in 3.18.1. Disabling tcp_window_scaling seems to solve it, but has
> >> obvious drawbacks for transfer speeds. Other sysctls do not seem to
> >> affect it.
> >> 
> >> I am not sure if this is fundamentally a kernel bug or a network
> >> issue, but we did not see this problem with older kernels.
> >> 
> >> [1] http://abstrakt.bengler.no/tcp-issues-s3.pcap.bz2
> > 
> > 
> > CC netdev
> > 
> > This looks like the bug we fixed here :
> > 
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=39bb5e62867de82b269b07df900165029b928359
> 
> Has that patch gone into a release? Because the problem persists with 3.18.1.

Patch is in 3.18.1 yes.

So thats a separate issue. 

Can you confirm pcap was taken at receiver (195.159.221.106), not sender
(54.231.136.74) , and on which host is running the 'buggy kernel' ?

If the sender is broken, changing the kernel on receiver wont help.

BTW not using sack (on 54.231.132.98) is terrible for performance in
lossy environments.

^ permalink raw reply

* [PATCH net-next] Driver: Vmxnet3: Make Rx ring 2 size configurable
From: Shrikrishna Khare @ 2015-01-06 17:20 UTC (permalink / raw)
  To: sbhatewara, pv-drivers, netdev, linux-kernel
  Cc: Shrikrishna Khare, Ramya Bolla

Rx ring 2 size can be configured by adjusting rx-jumbo parameter
of ethtool -G.

Signed-off-by: Ramya Bolla <bollar@vmware.com>
Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>
Signed-off-by: Shrikrishna Khare <skhare@vmware.com>
---
 drivers/net/vmxnet3/vmxnet3_defs.h    |    1 +
 drivers/net/vmxnet3/vmxnet3_drv.c     |    6 +++++-
 drivers/net/vmxnet3/vmxnet3_ethtool.c |   27 ++++++++++++++++++++-------
 drivers/net/vmxnet3/vmxnet3_int.h     |    6 ++++--
 4 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_defs.h b/drivers/net/vmxnet3/vmxnet3_defs.h
index 4d84912..25b6fa4 100644
--- a/drivers/net/vmxnet3/vmxnet3_defs.h
+++ b/drivers/net/vmxnet3/vmxnet3_defs.h
@@ -342,6 +342,7 @@ union Vmxnet3_GenericDesc {
 #define VMXNET3_TX_RING_MAX_SIZE   4096
 #define VMXNET3_TC_RING_MAX_SIZE   4096
 #define VMXNET3_RX_RING_MAX_SIZE   4096
+#define VMXNET3_RX_RING2_MAX_SIZE  2048
 #define VMXNET3_RC_RING_MAX_SIZE   8192
 
 /* a list of reasons for queue stop */
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index afd2953..7af1f5c 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2505,6 +2505,9 @@ vmxnet3_adjust_rx_ring_size(struct vmxnet3_adapter *adapter)
 	ring0_size = min_t(u32, ring0_size, VMXNET3_RX_RING_MAX_SIZE /
 			   sz * sz);
 	ring1_size = adapter->rx_queue[0].rx_ring[1].size;
+	ring1_size = (ring1_size + sz - 1) / sz * sz;
+	ring1_size = min_t(u32, ring1_size, VMXNET3_RX_RING2_MAX_SIZE /
+			   sz * sz);
 	comp_size = ring0_size + ring1_size;
 
 	for (i = 0; i < adapter->num_rx_queues; i++) {
@@ -2585,7 +2588,7 @@ vmxnet3_open(struct net_device *netdev)
 
 	err = vmxnet3_create_queues(adapter, adapter->tx_ring_size,
 				    adapter->rx_ring_size,
-				    VMXNET3_DEF_RX_RING_SIZE);
+				    adapter->rx_ring2_size);
 	if (err)
 		goto queue_err;
 
@@ -2964,6 +2967,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
 
 	adapter->tx_ring_size = VMXNET3_DEF_TX_RING_SIZE;
 	adapter->rx_ring_size = VMXNET3_DEF_RX_RING_SIZE;
+	adapter->rx_ring2_size = VMXNET3_DEF_RX_RING2_SIZE;
 
 	spin_lock_init(&adapter->cmd_lock);
 	adapter->adapter_pa = dma_map_single(&adapter->pdev->dev, adapter,
diff --git a/drivers/net/vmxnet3/vmxnet3_ethtool.c b/drivers/net/vmxnet3/vmxnet3_ethtool.c
index b7b5332..8a5a90e 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethtool.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethtool.c
@@ -447,12 +447,12 @@ vmxnet3_get_ringparam(struct net_device *netdev,
 	param->rx_max_pending = VMXNET3_RX_RING_MAX_SIZE;
 	param->tx_max_pending = VMXNET3_TX_RING_MAX_SIZE;
 	param->rx_mini_max_pending = 0;
-	param->rx_jumbo_max_pending = 0;
+	param->rx_jumbo_max_pending = VMXNET3_RX_RING2_MAX_SIZE;
 
 	param->rx_pending = adapter->rx_ring_size;
 	param->tx_pending = adapter->tx_ring_size;
 	param->rx_mini_pending = 0;
-	param->rx_jumbo_pending = 0;
+	param->rx_jumbo_pending = adapter->rx_ring2_size;
 }
 
 
@@ -461,7 +461,7 @@ vmxnet3_set_ringparam(struct net_device *netdev,
 		      struct ethtool_ringparam *param)
 {
 	struct vmxnet3_adapter *adapter = netdev_priv(netdev);
-	u32 new_tx_ring_size, new_rx_ring_size;
+	u32 new_tx_ring_size, new_rx_ring_size, new_rx_ring2_size;
 	u32 sz;
 	int err = 0;
 
@@ -473,6 +473,10 @@ vmxnet3_set_ringparam(struct net_device *netdev,
 						VMXNET3_RX_RING_MAX_SIZE)
 		return -EINVAL;
 
+	if (param->rx_jumbo_pending == 0 ||
+	    param->rx_jumbo_pending > VMXNET3_RX_RING2_MAX_SIZE)
+		return -EINVAL;
+
 	/* if adapter not yet initialized, do nothing */
 	if (adapter->rx_buf_per_pkt == 0) {
 		netdev_err(netdev, "adapter not completely initialized, "
@@ -500,8 +504,15 @@ vmxnet3_set_ringparam(struct net_device *netdev,
 							   sz) != 0)
 		return -EINVAL;
 
-	if (new_tx_ring_size == adapter->tx_queue[0].tx_ring.size &&
-	    new_rx_ring_size == adapter->rx_queue[0].rx_ring[0].size) {
+	/* ring2 has to be a multiple of VMXNET3_RING_SIZE_ALIGN */
+	new_rx_ring2_size = (param->rx_jumbo_pending + VMXNET3_RING_SIZE_MASK) &
+				~VMXNET3_RING_SIZE_MASK;
+	new_rx_ring2_size = min_t(u32, new_rx_ring2_size,
+				  VMXNET3_RX_RING2_MAX_SIZE);
+
+	if (new_tx_ring_size == adapter->tx_ring_size &&
+	    new_rx_ring_size == adapter->rx_ring_size &&
+	    new_rx_ring2_size == adapter->rx_ring2_size) {
 		return 0;
 	}
 
@@ -522,7 +533,7 @@ vmxnet3_set_ringparam(struct net_device *netdev,
 		vmxnet3_rq_destroy_all(adapter);
 
 		err = vmxnet3_create_queues(adapter, new_tx_ring_size,
-			new_rx_ring_size, VMXNET3_DEF_RX_RING_SIZE);
+			new_rx_ring_size, new_rx_ring2_size);
 
 		if (err) {
 			/* failed, most likely because of OOM, try default
@@ -530,11 +541,12 @@ vmxnet3_set_ringparam(struct net_device *netdev,
 			netdev_err(netdev, "failed to apply new sizes, "
 				   "try the default ones\n");
 			new_rx_ring_size = VMXNET3_DEF_RX_RING_SIZE;
+			new_rx_ring2_size = VMXNET3_DEF_RX_RING2_SIZE;
 			new_tx_ring_size = VMXNET3_DEF_TX_RING_SIZE;
 			err = vmxnet3_create_queues(adapter,
 						    new_tx_ring_size,
 						    new_rx_ring_size,
-						    VMXNET3_DEF_RX_RING_SIZE);
+						    new_rx_ring2_size);
 			if (err) {
 				netdev_err(netdev, "failed to create queues "
 					   "with default sizes. Closing it\n");
@@ -549,6 +561,7 @@ vmxnet3_set_ringparam(struct net_device *netdev,
 	}
 	adapter->tx_ring_size = new_tx_ring_size;
 	adapter->rx_ring_size = new_rx_ring_size;
+	adapter->rx_ring2_size = new_rx_ring2_size;
 
 out:
 	clear_bit(VMXNET3_STATE_BIT_RESETTING, &adapter->state);
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index 5f0199f..048f020 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -69,10 +69,10 @@
 /*
  * Version numbers
  */
-#define VMXNET3_DRIVER_VERSION_STRING   "1.2.1.0-k"
+#define VMXNET3_DRIVER_VERSION_STRING   "1.3.1.0-k"
 
 /* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
-#define VMXNET3_DRIVER_VERSION_NUM      0x01020100
+#define VMXNET3_DRIVER_VERSION_NUM      0x01030100
 
 #if defined(CONFIG_PCI_MSI)
 	/* RSS only makes sense if MSI-X is supported. */
@@ -352,6 +352,7 @@ struct vmxnet3_adapter {
 	/* Ring sizes */
 	u32 tx_ring_size;
 	u32 rx_ring_size;
+	u32 rx_ring2_size;
 
 	struct work_struct work;
 
@@ -384,6 +385,7 @@ struct vmxnet3_adapter {
 /* must be a multiple of VMXNET3_RING_SIZE_ALIGN */
 #define VMXNET3_DEF_TX_RING_SIZE    512
 #define VMXNET3_DEF_RX_RING_SIZE    256
+#define VMXNET3_DEF_RX_RING2_SIZE   128
 
 #define VMXNET3_MAX_ETH_HDR_SIZE    22
 #define VMXNET3_MAX_SKB_BUF_SIZE    (3*1024)
-- 
1.7.4.1

^ permalink raw reply related

* Re: [net-next PATCH v1 04/11] rocker: add pipeline model for rocker switch
From: Scott Feldman @ 2015-01-06 17:16 UTC (permalink / raw)
  To: John Fastabend
  Cc: Thomas Graf, Jiří Pírko, Jamal Hadi Salim,
	simon.horman@netronome.com, Netdev, David S. Miller,
	Andy Gospodarek
In-Reply-To: <54AC149A.6040401@gmail.com>

On Tue, Jan 6, 2015 at 9:00 AM, John Fastabend <john.fastabend@gmail.com> wrote:
> On 01/05/2015 11:01 PM, Scott Feldman wrote:
>>> +
>>> +struct net_flow_jump_table parse_ethernet[3] = {
>>> +       {
>>> +               .field = {
>>> +                  .header = HEADER_ETHERNET,
>>> +                  .field = HEADER_ETHERNET_ETHERTYPE,
>>> +                  .type = NET_FLOW_FIELD_REF_ATTR_TYPE_U16,
>>> +                  .value_u16 = 0x0800,

ETH_P_IP, etc

>>
>>
>> How is htons/ntohs conversions happening here?
>
>
> my current stance is to leave everything in host order in the model
> and let the drivers do conversions as needed. For example some drivers
> want the vlan vid in host order others network order. I think its
> more readable above then with hton*() throughout.

Hmmm...I would argue adding htons/htonl makes it more readable in the
sense that it's a reminder that this is a field in a network header,
to be used for matching against packet headers, which use
network-ordering.  Store the field in the order best for comparison
with the raw pkt data.  Drivers may still need to do some conversion
if the field is programmed in hardware in a diff order.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox