Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v1 0/7] net: extend ethtool link mode bitmaps to 48 bits
From: Ben Hutchings @ 2015-01-05  2:30 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: David Decotigny, Amir Vadai, Florian Fainelli, Linux NetDev,
	Linux Kernel Mailing List, linux-api, David Decotigny,
	David S. Miller, Jason Wang, Michael S. Tsirkin, Herbert Xu,
	Al Viro, Masatake YAMATO, Xi Wang, Neil Horman, WANG Cong,
	Flavio Leitner, Tom Gundersen, Jiri Pirko, Vlad Yasevich,
	Eric W. Biederman, Saeed Mahameed, Venkata Duvvuru, Govindaraj
In-Reply-To: <CAHo-Ooxi8V_58b456vePY9_7ChuBe0jxEVHvGkGvRBpAR744yA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1240 bytes --]

On Mon, 2015-01-05 at 01:34 +0100, Maciej Żenczykowski wrote:
> >> I can send updates to other drivers, even though it's rather pointless
> >> to update 1G drivers at this point for example. Please let me know,
> >> but I'd prefer to do this in follow-up patches outside this first
> >> patch series.
> > [...]
> >
> > They should be changed to ensure they reject setting any of the high
> > advertising flags, but it's not urgent.
> 
> if old drivers advertised a get/set_bits function while new drivers
> advertised a get/set_new_bits function,
> you could not updated any old drivers, and simply take care of
> rejecting invalid bits in core, by calling set_new_bits if provided,
> if not, rejecting bad bits and calling set_bits if no bad bits were
> set.

We've never checked that the reserved fields are zero before, and I
think there are still drivers that don't fully validate the existing 32
bits.  So while I think drivers should fully validate the advertising
flags, userland generally can't assume they do.  And therefore I don't
think it's worth adding complexity to the ethtool core that only partly
fixes this.

Ben.

-- 
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: [PATCH/RFC flow-net-next 02/10] net: flow: Add features to tables
From: Simon Horman @ 2015-01-05  2:18 UTC (permalink / raw)
  To: Cong Wang; +Cc: John Fastabend, netdev
In-Reply-To: <CAHA+R7NmfUpYcKfW_Gck2HaANH3OQk-9BAYZCXnBtGO1eV13xg@mail.gmail.com>

On Mon, Dec 29, 2014 at 03:03:39PM -0800, Cong Wang wrote:
> On Sun, Dec 28, 2014 at 6:15 PM, Simon Horman
> <simon.horman@netronome.com> wrote:
> > +static struct net_flow_table *net_flow_table_get_table(struct net_device *dev,
> > +                                                      int table_uid)
> > +{
> > +       struct net_flow_table **tables;
> > +       int i;
> > +
> > +       tables = net_flow_get_tables(dev);
> > +       if (IS_ERR(tables))
> > +               return ERR_PTR(PTR_ERR(tables));
> 
> Seriously? :)

I was looking for a way to handle the type of tables being different
to the return type of net_flow_table_get_table().

Would you recommend a cast?

		return (struct net_flow_table *)tables;

^ permalink raw reply

* Re: [PATCH/RFC flow-net-next 04/10] net: flow: Add counters to flows
From: Simon Horman @ 2015-01-05  2:10 UTC (permalink / raw)
  To: Arad, Ronen; +Cc: Fastabend, John R, netdev@vger.kernel.org
In-Reply-To: <E4CD12F19ABA0C4D8729E087A761DC3505DD0E24@ORSMSX101.amr.corp.intel.com>

On Mon, Dec 29, 2014 at 07:31:41AM +0000, Arad, Ronen wrote:
> 
> 
> >-----Original Message-----
> >From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> >Behalf Of Simon Horman
> >Sent: Monday, December 29, 2014 4:16 AM
> >To: Fastabend, John R; netdev@vger.kernel.org
> >Cc: Simon Horman
> >Subject: [PATCH/RFC flow-net-next 04/10] net: flow: Add counters to flows
> >
> >It may be useful for hardware flow table support for counters to be exposed
> >via the flow API. One possible use case of this is for Open vSwitch to use
> >the flow API in conjunction with its existing datapath flow management
> >scheme which in a nutshell treats the datapath as a cache that times out
> >idle entries.
> >
> >This patch exposes optionally exposes three counters:
> >- Number of packets that have matched a flow
> >- Number of bytes of packets that have matched a flow
> >- The time in ms when the flow was last hit
> >
> >Inspired by the flow counters present in Open Flow.
> >
> >Signed-off-by: Simon Horman <simon.horman@netronome.com>
> >
> >---
> >
> >Compile tested only
> >---
> > include/uapi/linux/if_flow.h | 24 ++++++++++++++++++++++++
> > net/core/flow_table.c        | 27 +++++++++++++++++++++++++++
> > 2 files changed, 51 insertions(+)
> >
> >diff --git a/include/uapi/linux/if_flow.h b/include/uapi/linux/if_flow.h
> >index 28da45b..18214ea 100644
> >--- a/include/uapi/linux/if_flow.h
> >+++ b/include/uapi/linux/if_flow.h
> >@@ -127,6 +127,9 @@
> >  *	     [NET_FLOW_ATTR_PRIORITY]
> >  *	     [NET_FLOW_ATTR_IDLE_TIMEOUT]
> >  *	     [NET_FLOW_ATTR_HARD_TIMEOUT]
> >+ *	     [NET_FLOW_ATTR_BYTE_COUNT]
> >+ *	     [NET_FLOW_ATTR_PACKET_COUNT]
> >+ *	     [NET_FLOW_ATTR_LAST_USED]
> >  *	     [NET_FLOW_ATTR_MATCHES]
> >  *	        [NET_FLOW_FIELD_REF]
> >  *	        [NET_FLOW_FIELD_REF]
> >@@ -153,6 +156,9 @@
> >  *     [NET_FLOW_ATTR_PRIORITY]
> >  *     [NET_FLOW_ATTR_IDLE_TIMEOUT]
> >  *     [NET_FLOW_ATTR_HARD_TIMEOUT]
> >+ *     [NET_FLOW_ATTR_BYTE_COUNT]
> >+ *     [NET_FLOW_ATTR_PACKET_COUNT]
> >+ *     [NET_FLOW_ATTR_LAST_USED]
> >  *     [NET_FLOW_MATCHES]
> >  *       [NET_FLOW_FIELD_REF]
> >  *       [NET_FLOW_FIELD_REF]
> >@@ -365,6 +371,9 @@ enum {
> >  * @idle_timeout idle timeout of flow in seconds. Zero for no timeout.
> >  * @hard_timeout timeout of flow regardless of use in seconds.
> >  *               Zero for no timeout.
> >+ * @byte_count bytes recieved
> >+ * @byte_count packets recieved
> >+ * @last_used time of most recent use (msec since system initialisation)
> >  *
> >  * Flows must match all entries in match set.
> >  */
> >@@ -374,6 +383,9 @@ struct net_flow_flow {
> > 	int priority;
> > 	__u32 idle_timeout;
> > 	__u32 hard_timeout;
> >+	__u64 byte_count;
> >+	__u64 packet_count;
> >+	__u64 last_used;
> > 	struct net_flow_field_ref *matches;
> > 	struct net_flow_action *actions;
> > };
> >@@ -414,6 +426,9 @@ enum {
> > 	NET_FLOW_ATTR_ACTIONS,
> > 	NET_FLOW_ATTR_IDLE_TIMEOUT,
> > 	NET_FLOW_ATTR_HARD_TIMEOUT,
> >+	NET_FLOW_ATTR_BYTE_COUNT,
> >+	NET_FLOW_ATTR_PACKET_COUNT,
> >+	NET_FLOW_ATTR_LAST_USED,
> > 	__NET_FLOW_ATTR_MAX,
> > };
> > #define NET_FLOW_ATTR_MAX (__NET_FLOW_ATTR_MAX - 1)
> >@@ -465,6 +480,15 @@ enum {
> >
> > 	/* Table supports idle timeout of flows */
> > 	NET_FLOW_TABLE_F_HARD_TIMEOUT		= (1 << 1),
> >+
> >+	/* Table supports byte counter for flows */
> >+	NET_FLOW_TABLE_F_BYTE_COUNT		= (1 << 2),
> >+
> >+	/* Table supports packet counter for flows */
> >+	NET_FLOW_TABLE_F_PACKET_COUNT		= (1 << 3),
> >+
> >+	/* Table supports last used counter for flows */
> >+	NET_FLOW_TABLE_F_PACKET_LAST_USED	= (1 << 4),
> > };
> >
> > #if 0
> >diff --git a/net/core/flow_table.c b/net/core/flow_table.c
> >index 89ba9bc..070e646 100644
> >--- a/net/core/flow_table.c
> >+++ b/net/core/flow_table.c
> >@@ -54,6 +54,9 @@ struct nla_policy net_flow_flow_policy[NET_FLOW_ATTR_MAX +
> >1] = {
> > 	[NET_FLOW_ATTR_PRIORITY]	= { .type = NLA_U32 },
> > 	[NET_FLOW_ATTR_IDLE_TIMEOUT]	= { .type = NLA_U32 },
> > 	[NET_FLOW_ATTR_HARD_TIMEOUT]	= { .type = NLA_U32 },
> >+	[NET_FLOW_ATTR_BYTE_COUNT]	= { .type = NLA_U64 },
> >+	[NET_FLOW_ATTR_PACKET_COUNT]	= { .type = NLA_U64 },
> >+	[NET_FLOW_ATTR_LAST_USED]	= { .type = NLA_U64 },
> > 	[NET_FLOW_ATTR_MATCHES]	= { .type = NLA_NESTED },
> > 	[NET_FLOW_ATTR_ACTIONS]	= { .type = NLA_NESTED },
> > };
> >@@ -206,6 +209,16 @@ int net_flow_put_flow(struct sk_buff *skb, struct
> >net_flow_flow *flow)
> > 	    nla_put_u32(skb, NET_FLOW_ATTR_HARD_TIMEOUT, flow->hard_timeout))
> > 		goto flows_put_failure;
> >
> >+	if (flow->byte_count &&
> >+	    nla_put_u32(skb, NET_FLOW_ATTR_BYTE_COUNT, flow->byte_count))
> >+		goto flows_put_failure;
> >+	if (flow->packet_count &&
> >+	    nla_put_u32(skb, NET_FLOW_ATTR_PACKET_COUNT, flow->packet_count))
> >+		goto flows_put_failure;
> >+	if (flow->last_used &&
> >+	    nla_put_u32(skb, NET_FLOW_ATTR_LAST_USED, flow->last_used))
> >+		goto flows_put_failure;
> >+
> The flow byte_count, packet_count, and last_used fields are defined as __u64 and related netlink attributes are of type NLA_U64 but nla_put_u32() is used to add them to the netlink msg.

Thanks, I will fix that up by using nla_put_u64().

> > 	matches = nla_nest_start(skb, NET_FLOW_ATTR_MATCHES);
> > 	if (!matches)
> > 		goto flows_put_failure;
> >@@ -536,6 +549,13 @@ static int net_flow_get_flow(struct net_flow_flow *flow,
> >struct nlattr *attr)
> > 	if (f[NET_FLOW_ATTR_HARD_TIMEOUT])
> > 		flow->hard_timeout = nla_get_u32(f[NET_FLOW_ATTR_HARD_TIMEOUT]);
> >
> >+	if (f[NET_FLOW_ATTR_BYTE_COUNT])
> >+		flow->byte_count = nla_get_u64(f[NET_FLOW_ATTR_BYTE_COUNT]);
> >+	if (f[NET_FLOW_ATTR_PACKET_COUNT])
> >+		flow->packet_count = nla_get_u64(f[NET_FLOW_ATTR_PACKET_COUNT]);
> >+	if (f[NET_FLOW_ATTR_LAST_USED])
> >+		flow->last_used = nla_get_u64(f[NET_FLOW_ATTR_LAST_USED]);
> >+
> > 	flow->matches = NULL;
> > 	flow->actions = NULL;
> >
> >@@ -1386,6 +1406,13 @@ static int net_flow_table_cmd_flows(struct sk_buff
> >*recv_skb,
> > 		if (this.hard_timeout)
> > 			used_features |= NET_FLOW_TABLE_F_HARD_TIMEOUT;
> >
> >+		if (this.byte_count)
> >+			used_features |= NET_FLOW_TABLE_F_BYTE_COUNT;
> >+		if (this.packet_count)
> >+			used_features |= NET_FLOW_TABLE_F_PACKET_COUNT;
> >+		if (this.last_used)
> >+			used_features |= NET_FLOW_TABLE_F_PACKET_LAST_USED;
> >+
> > 		err = net_flow_table_check_features(dev, this.table_id,
> > 						    used_features);
> > 		if (err)
> >--
> >2.1.3
> >
> >--
> >To unsubscribe from this list: send the line "unsubscribe netdev" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] net: flow: Allow actions and matches to be NULL in net_flow_put_flow()
From: Simon Horman @ 2015-01-05  2:06 UTC (permalink / raw)
  To: John Fastabend; +Cc: John Fastabend, netdev
In-Reply-To: <54A195C7.3010009@gmail.com>

On Mon, Dec 29, 2014 at 09:56:23AM -0800, John Fastabend wrote:
> On 12/28/2014 06:20 PM, Simon Horman wrote:
> >This makes the handing of the absence of actions or matches
> >symmetric with net_flow_get_flow().
> >
> >Signed-off-by: Simon Horman <simon.horman@netronome.com>
> >---
> 
> Again thanks I actually just hit this with my rocker implementation
> so very timely fix. Same story here I'll roll it into the patch and
> submit it later today.

Thanks. I have hit a few more minor problems which I have fixes for.
What is the best way to handle them? Rebase them on v1 of your patchset
and post them for review if they are still relevant?

^ permalink raw reply

* Re: [PATCH] net: flow: Guard against accessing non-existent attributes
From: Simon Horman @ 2015-01-05  2:04 UTC (permalink / raw)
  To: John Fastabend; +Cc: John Fastabend, netdev
In-Reply-To: <54A1951A.2070209@gmail.com>

On Mon, Dec 29, 2014 at 09:53:30AM -0800, John Fastabend wrote:
> On 12/28/2014 06:17 PM, Simon Horman wrote:
> >Signed-off-by: Simon Horman <simon.horman@netronome.com>
> >---
> >  net/core/flow_table.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> 
> Great thanks. I rolled it into the patches I should hopefully have
> ready to submit later today.
> 
> By the way I got this working on rocker now so we can test it easier,
> I'll post the driver updates as well.

Great, excellent news.

^ permalink raw reply

* Re: [PATCH] net: wireless: rtlwifi: btcoexist: halbtc8821a2ant: Remove some unused functions
From: Larry Finger @ 2015-01-05  1:45 UTC (permalink / raw)
  To: Rickard Strandqvist
  Cc: Chaoming Li, Kalle Valo, Greg Kroah-Hartman, Fengguang Wu,
	linux-wireless@vger.kernel.org, Network Development,
	Linux Kernel Mailing List
In-Reply-To: <CAKXHbyOYO29uKA127qyYrCfpgQ2zoqBS9rAJME513oZ8JVdOug@mail.gmail.com>

On 01/03/2015 06:03 AM, Rickard Strandqvist wrote:
> 2015-01-03 7:05 GMT+01:00 Larry Finger <Larry.Finger@lwfinger.net>:
>
>> On 01/02/2015 02:26 PM, Rickard Strandqvist wrote:
>>
>>> 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>
>>>
>>
>> I know that you have been told that including "net: wireless:" in the
>> subject line is discouraged. Please do not do this again. The staging
>> directory is different as GregKH uses "staging:" in the subject to route
>> patches, but wireless does not.
>>
>> As to the patch, NACK for the simple reason that I am currently working on
>> a number of changes to btcoexist. Some of these routines may end up being
>> removed, but others will not. Having your patch remove them, and one of
>> mine adding them back just constitutes a lot of churning of the source. In
>> addition, it greatly increases the probability of the source trees becoming
>> unsynchronized and getting merge conflicts.
>>
>> Larry
>>
>>
>
> Hi Larry
>
> I do not recognize that there has been no diskution on the subject of "net:
> wireless:"
> I use some sed call for this, so it's easy to fix. You want the "net:
> wireless:" part completely erased then?
> I check in Documentation/ but find any clear info for this.
>
> Sorry for the patch, is there any way for me to see that this is something
> that is being worked on?

These patches are the result of private communications between the Realtek 
engineers and me. The only publication is in the "rock" branch of the git repo 
at http://github.com/lwfinger/rtlwifi_new.git.

I do not understand the "use of some 'sed' call". If you edit the source and add 
it to the git repo with the add and commit operations, then format-patch gives 
you exactly what you need, and send-email does that operation.

Larry

^ permalink raw reply

* Re: [PATCH net-next v1 0/7] net: extend ethtool link mode bitmaps to 48 bits
From: Maciej Żenczykowski @ 2015-01-05  0:34 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: David Decotigny, Amir Vadai, Florian Fainelli, Linux NetDev,
	Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA,
	David Decotigny, David S. Miller, Jason Wang, Michael S. Tsirkin,
	Herbert Xu, Al Viro, Masatake YAMATO, Xi Wang, Neil Horman,
	WANG Cong, Flavio Leitner, Tom Gundersen, Jiri Pirko,
	Vlad Yasevich, Eric W. Biederman, Saeed Mahameed, Venkata Duvvuru,
	Govindaraj
In-Reply-To: <1420408540.5686.140.camel-/+tVBieCtBitmTQ+vhA3Yw@public.gmane.org>

>> I can send updates to other drivers, even though it's rather pointless
>> to update 1G drivers at this point for example. Please let me know,
>> but I'd prefer to do this in follow-up patches outside this first
>> patch series.
> [...]
>
> They should be changed to ensure they reject setting any of the high
> advertising flags, but it's not urgent.

if old drivers advertised a get/set_bits function while new drivers
advertised a get/set_new_bits function,
you could not updated any old drivers, and simply take care of
rejecting invalid bits in core, by calling set_new_bits if provided,
if not, rejecting bad bits and calling set_bits if no bad bits were
set.

^ permalink raw reply

* Re: [PATCH 2/2] Fix copy-paste bug: assign from src struct not dest
From: Giel van Schijndel @ 2015-01-04 23:02 UTC (permalink / raw)
  To: linux-kernel
In-Reply-To: <1420394427-19509-2-git-send-email-me@mortis.eu>

[-- Attachment #1: Type: text/plain, Size: 210 bytes --]

On Sun, Jan 04, 2015 at 19:00:23 +0100, Giel van Schijndel wrote:
> ---

Forgot to:
Signed-off-by: Giel van Schijndel <me@mortis.eu>

-- 
Met vriendelijke groet,
With kind regards,
Giel van Schijndel

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH] Fix NUL (\0 or \x00) specification in string
From: Giel van Schijndel @ 2015-01-04 23:04 UTC (permalink / raw)
  To: linux-kernel; +Cc: Armin Schindler, Karsten Keil, open list:ISDN SUBSYSTEM
In-Reply-To: <1420394722-20197-1-git-send-email-me@mortis.eu>

[-- Attachment #1: Type: text/plain, Size: 522 bytes --]

On Sun, Jan 04, 2015 at 19:05:22 +0100, Giel van Schijndel wrote:
> In C one can either use '\0' or '\x00' (or '\000') to add a NUL byte to
> a string. '\0x00' isn't part of these and will in fact result in a
> single NUL followed by "x00". This fixes that.
> ---

Forgot to:
Signed-off-by: Giel van Schijndel <me@mortis.eu>

-- 
Met vriendelijke groet,
With kind regards,
Giel van Schijndel
--
"There are 2 types of people in this world - those who like Chuck
 Norris and those who are dead."
  -- Jus12

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH] Fix an infinite retry-loop
From: Giel van Schijndel @ 2015-01-04 23:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jitendra Kalsaria, Ron Mercer, supporter:QLOGIC QLA3XXX NE...,
	open list:QLOGIC QLA3XXX NE...
In-Reply-To: <1420394696-20099-1-git-send-email-me@mortis.eu>

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

On Sun, Jan 04, 2015 at 19:04:55 +0100, Giel van Schijndel wrote:
> This was clearly intended as a retry-10-times loop, but due to the
> absence of code incrementing the loop-counter it was practically a
> retry-forever loop.
> 
> Rewritten it as a for-loop as well to make the loop-counter increment
> (as well as its potential absence) easier to spot.
> ---

Forgot to:
Signed-off-by: Giel van Schijndel <me@mortis.eu>

-- 
Met vriendelijke groet,
With kind regards,
Giel van Schijndel
--
"A computer is a stupid machine with the ability to do incredibly smart
 things, while computer programmers are smart people with the ability to
 do incredibly stupid things. They are, in short, a perfect match.
  -- Bill Bryson

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] Align member-assigns in a structure-copy block
From: Giel van Schijndel @ 2015-01-04 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kalle Valo, Eliad Peller, John W. Linville, Arik Nemtsov,
	open list:TI WILINK WIRELES..., open list:NETWORKING DRIVERS
In-Reply-To: <1420394427-19509-1-git-send-email-me@mortis.eu>

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]

On Sun, Jan 04, 2015 at 19:00:22 +0100, Giel van Schijndel wrote:
> This highlights the differences (errors).
> ---

Forgot to:
Signed-off-by: Giel van Schijndel <me@mortis.eu>

-- 
Met vriendelijke groet,
With kind regards,
Giel van Schijndel
--
"Walking on water and developing software from a specification are easy
 if both are frozen."
  -- Edward V Berard

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v1 2/7] net: phy: extend link mode support to 48 bits
From: Ben Hutchings @ 2015-01-04 22:03 UTC (permalink / raw)
  To: David Decotigny
  Cc: Amir Vadai, Florian Fainelli, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, David Decotigny,
	David S. Miller, Jason Wang, Michael S. Tsirkin, Herbert Xu,
	Al Viro, Masatake YAMATO, Xi Wang, Neil Horman, WANG Cong,
	Flavio Leitner, Tom Gundersen, Jiri Pirko, Vlad Yasevich,
	Eric W. Biederman, Saeed Mahameed, Venkata Duvvuru,
	Govindarajulu Varadarajan
In-Reply-To: <1420405017-23278-3-git-send-email-ddecotig-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2212 bytes --]

On Sun, 2015-01-04 at 12:56 -0800, David Decotigny wrote:
> From: David Decotigny <decot-Ypc/8FJVVoBWk0Htik3J/w@public.gmane.org>
> 
> Signed-off-by: David Decotigny <decot-Ypc/8FJVVoBWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/net/phy/phy.c        | 29 ++++++++++++++---------------
>  drivers/net/phy/phy_device.c |  4 ++--
>  include/linux/phy.h          | 10 +++++-----
>  3 files changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 767cd11..e9c8499 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
[...]
> @@ -245,7 +246,7 @@ static inline unsigned int phy_find_valid(unsigned int idx, u32 features)
>   */
>  static void phy_sanitize_settings(struct phy_device *phydev)
>  {
> -	u32 features = phydev->supported;
> +	ethtool_link_mode_mask_t features = phydev->supported;
>  	unsigned int idx;
>  
>  	/* Sanitize settings based on PHY capabilities */
> @@ -279,13 +280,13 @@ int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd)
>  		return -EINVAL;
>  
>  	/* We make sure that we don't pass unsupported values in to the PHY */
> -	cmd->advertising &= phydev->supported;
> +	phydev->advertising = ethtool_cmd_advertising(cmd) & phydev->supported;

phydev->advertising should not be changed until after the following
validation.  Use a local variable for this.

>  	/* Verify the settings we care about. */
>  	if (cmd->autoneg != AUTONEG_ENABLE && cmd->autoneg != AUTONEG_DISABLE)
>  		return -EINVAL;
>  
> -	if (cmd->autoneg == AUTONEG_ENABLE && cmd->advertising == 0)
> +	if (cmd->autoneg == AUTONEG_ENABLE && phydev->advertising == 0)
>  		return -EINVAL;
>  
>  	if (cmd->autoneg == AUTONEG_DISABLE &&
> @@ -300,8 +301,6 @@ int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd)
>  
>  	phydev->speed = speed;
>  
> -	phydev->advertising = cmd->advertising;
> -
>  	if (AUTONEG_ENABLE == cmd->autoneg)
>  		phydev->advertising |= ADVERTISED_Autoneg;
>  	else
[...]

The assignment to phydev->advertising should probably remain here.

Ben.

-- 
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v1 1/7] net: ethtool: extend link mode support to 48 bits
From: Ben Hutchings @ 2015-01-04 22:01 UTC (permalink / raw)
  To: David Decotigny
  Cc: Amir Vadai, Florian Fainelli, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, David Decotigny,
	David S. Miller, Jason Wang, Michael S. Tsirkin, Herbert Xu,
	Al Viro, Masatake YAMATO, Xi Wang, Neil Horman, WANG Cong,
	Flavio Leitner, Tom Gundersen, Jiri Pirko, Vlad Yasevich,
	Eric W. Biederman, Saeed Mahameed, Venkata Duvvuru,
	Govindarajulu Varadarajan
In-Reply-To: <1420405017-23278-2-git-send-email-ddecotig-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 3096 bytes --]

On Sun, 2015-01-04 at 12:56 -0800, David Decotigny wrote:
> From: David Decotigny <decot-Ypc/8FJVVoBWk0Htik3J/w@public.gmane.org>

This is mostly fine, with just a few minor issues.

> Signed-off-by: David Decotigny <decot-Ypc/8FJVVoBWk0Htik3J/w@public.gmane.org>
> ---
>  include/uapi/linux/ethtool.h | 130 ++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 110 insertions(+), 20 deletions(-)
> 
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 5f66d9c..61e7734 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
[...]
> @@ -123,6 +140,46 @@ static inline __u32 ethtool_cmd_speed(const struct ethtool_cmd *ep)
>  	return (ep->speed_hi << 16) | ep->speed;
>  }
>  
> +/**
> + * MAKE_ETHTOOL_LINK_MODE_ACCESSORS - create the link_mode accessors
> + * Macro to generate the %ethtool_cmd_supported_*,
> + * %ethtool_cmd_advertising_*, %ethtool_cmd_lp_advertising_*,
> + * %ethtool_eee_supported_*, %ethtool_eee_advertised_*,
> + * %ethtool_eee_lp_advertised_* families of functions.
> + *
> + * Macro args:

Delete the 'Macro args:' heading; that's implied by the @ prefixes.

> + *  @struct_name: either %ethtool_cmd or %ethtool_eee
> + *  @field_name: name of the fields in %struct_name to
> + *      access. supported/advertising/lp_advertising for ethtool_cmd,
> + *      supported/advertised/lp_advertised for ethtool_eee
> + *
> + * Generates the following static functions:
> + *  @ethtool_cmd_supported(const struct ethtool_cmd*): returns
> + *      the 64b value of %supported fields (the upper bits 63..48 are 0)
> + *  @ethtool_cmd_supported_set(struct ethtool_cmd*,
> + *      ethtool_link_mode_mask_t value): set the %supported fields to
> + *      given %value (only the lower 48 bits used, upper bits 63..48
> + *      ignored)

Delete the @ prefixes from these headings.

> + * Same doc for all the other functions.
> + */
> +#define MAKE_ETHTOOL_LINK_MODE_ACCESSORS(struct_name, field_name)	\

I think the name should begin with ETHTOOL but it's not a big deal.

[...]
> @@ -1192,6 +1273,9 @@ enum ethtool_sfeatures_retval_bits {
>  #define SPARC_ETH_GSET		ETHTOOL_GSET
>  #define SPARC_ETH_SSET		ETHTOOL_SSET
>  
> +/* Do not use the following macros directly to update
> + * ethtool_cmd::supported, ethtool_eee::supported. Please use
> + * ethtool_(cmd|eee)_supported(|_set) instead */

The ending */ belongs on a new line.

>  #define SUPPORTED_10baseT_Half		(1 << 0)
>  #define SUPPORTED_10baseT_Full		(1 << 1)
>  #define SUPPORTED_100baseT_Half		(1 << 2)
> @@ -1223,7 +1307,12 @@ enum ethtool_sfeatures_retval_bits {
>  #define SUPPORTED_56000baseCR4_Full	(1 << 28)
>  #define SUPPORTED_56000baseSR4_Full	(1 << 29)
>  #define SUPPORTED_56000baseLR4_Full	(1 << 30)
> +/* TODO: for bit indices >= 31, make sure to shift 1ULL instead of 1 */
[...]

I don't think that comment is necessary; the compiler will surely warn
if someone forgets.

Ben.

-- 
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v1 0/7] net: extend ethtool link mode bitmaps to 48 bits
From: Ben Hutchings @ 2015-01-04 21:55 UTC (permalink / raw)
  To: David Decotigny
  Cc: Amir Vadai, Florian Fainelli, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, David Decotigny,
	David S. Miller, Jason Wang, Michael S. Tsirkin, Herbert Xu,
	Al Viro, Masatake YAMATO, Xi Wang, Neil Horman, WANG Cong,
	Flavio Leitner, Tom Gundersen, Jiri Pirko, Vlad Yasevich,
	Eric W. Biederman, Saeed Mahameed, Venkata Duvvuru,
	Govindarajulu Varadarajan
In-Reply-To: <1420405017-23278-1-git-send-email-ddecotig-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

On Sun, 2015-01-04 at 12:56 -0800, David Decotigny wrote:
[...]
> I can send updates to other drivers, even though it's rather pointless
> to update 1G drivers at this point for example. Please let me know,
> but I'd prefer to do this in follow-up patches outside this first
> patch series.
[...]

They should be changed to ensure they reject setting any of the high
advertising flags, but it's not urgent.

Ben.

-- 
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: TCP out of memory - possible bug [3.18.0-rc3] / sched?
From: Markus Trippelsdorf @ 2015-01-04 21:39 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Tomasz Mloduchowski, netdev
In-Reply-To: <1420405287.32621.8.camel@edumazet-glaptop2.roam.corp.google.com>

On 2015.01.04 at 13:01 -0800, Eric Dumazet wrote:
> On Sun, 2015-01-04 at 09:42 +0100, Markus Trippelsdorf wrote:
> 
> > Any news on this issue?
> > 
> > I stumbled across the same problem today with 3.19.0-rc2:
> 
> What make you think it was fixed in 3.19.0-rc2 ?

Nothing.

> Have you tried a bisection ?

It was a one time event for me. So a bisection is out of the question.

But Tomasz wrote that he can reproduce the issue reliably.

Have you tried a bisection, Tomasz?

-- 
Markus

^ permalink raw reply

* Re: TCP out of memory - possible bug [3.18.0-rc3] / sched?
From: Eric Dumazet @ 2015-01-04 21:01 UTC (permalink / raw)
  To: Markus Trippelsdorf; +Cc: Tomasz Mloduchowski, netdev
In-Reply-To: <20150104084240.GA1710@x4>

On Sun, 2015-01-04 at 09:42 +0100, Markus Trippelsdorf wrote:

> Any news on this issue?
> 
> I stumbled across the same problem today with 3.19.0-rc2:

What make you think it was fixed in 3.19.0-rc2 ?

Have you tried a bisection ?

^ permalink raw reply

* [PATCH net-next v1 6/7] net: tun: extend link mode support to 48 bits
From: David Decotigny @ 2015-01-04 20:56 UTC (permalink / raw)
  To: Amir Vadai, Florian Fainelli, netdev, linux-kernel, linux-api
  Cc: David Decotigny, 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, Saeed Mahameed, Venkata Duvvuru,
	Govindarajulu Varadarajan
In-Reply-To: <1420405017-23278-1-git-send-email-ddecotig@gmail.com>

From: David Decotigny <decot@googlers.com>

Signed-off-by: David Decotigny <decot@googlers.com>
---
 drivers/net/tun.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index c0df872..88f9078 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2257,8 +2257,8 @@ static struct miscdevice tun_miscdev = {
 
 static int tun_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
-	cmd->supported		= 0;
-	cmd->advertising	= 0;
+	ethtool_cmd_supported_set(cmd, 0);
+	ethtool_cmd_advertising_set(cmd, 0);
 	ethtool_cmd_speed_set(cmd, SPEED_10);
 	cmd->duplex		= DUPLEX_FULL;
 	cmd->port		= PORT_TP;
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH net-next v1 7/7] net: mlx4_en: extend link mode support to 48 bits
From: David Decotigny @ 2015-01-04 20:56 UTC (permalink / raw)
  To: Amir Vadai, Florian Fainelli, netdev, linux-kernel, linux-api
  Cc: David Decotigny, 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, Saeed Mahameed, Venkata Duvvuru,
	Govindarajulu Varadarajan
In-Reply-To: <1420405017-23278-1-git-send-email-ddecotig@gmail.com>

From: David Decotigny <decot@googlers.com>

Signed-off-by: David Decotigny <decot@googlers.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 67 +++++++++++++++----------
 1 file changed, 40 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index 90e0f04..395ab72 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -388,7 +388,8 @@ static u32 mlx4_en_autoneg_get(struct net_device *dev)
 	return autoneg;
 }
 
-static u32 ptys_get_supported_port(struct mlx4_ptys_reg *ptys_reg)
+static ethtool_link_mode_mask_t
+ptys_get_supported_port(struct mlx4_ptys_reg *ptys_reg)
 {
 	u32 eth_proto = be32_to_cpu(ptys_reg->eth_proto_cap);
 
@@ -465,7 +466,7 @@ enum ethtool_report {
 };
 
 /* Translates mlx4 link mode to equivalent ethtool Link modes/speed */
-static u32 ptys2ethtool_map[MLX4_LINK_MODES_SZ][3] = {
+static u64 ptys2ethtool_map[MLX4_LINK_MODES_SZ][3] = {
 	[MLX4_100BASE_TX] = {
 		SUPPORTED_100baseT_Full,
 		ADVERTISED_100baseT_Full,
@@ -558,10 +559,11 @@ static u32 ptys2ethtool_map[MLX4_LINK_MODES_SZ][3] = {
 		},
 };
 
-static u32 ptys2ethtool_link_modes(u32 eth_proto, enum ethtool_report report)
+static ethtool_link_mode_mask_t
+ptys2ethtool_link_modes(u32 eth_proto, enum ethtool_report report)
 {
 	int i;
-	u32 link_modes = 0;
+	ethtool_link_mode_mask_t link_modes = 0;
 
 	for (i = 0; i < MLX4_LINK_MODES_SZ; i++) {
 		if (eth_proto & MLX4_PROT_MASK(i))
@@ -570,7 +572,8 @@ static u32 ptys2ethtool_link_modes(u32 eth_proto, enum ethtool_report report)
 	return link_modes;
 }
 
-static u32 ethtool2ptys_link_modes(u32 link_modes, enum ethtool_report report)
+static u32 ethtool2ptys_link_modes(ethtool_link_mode_mask_t link_modes,
+				   enum ethtool_report report)
 {
 	int i;
 	u32 ptys_modes = 0;
@@ -601,6 +604,9 @@ static int ethtool_get_ptys_settings(struct net_device *dev,
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	struct mlx4_ptys_reg ptys_reg;
 	u32 eth_proto;
+	ethtool_link_mode_mask_t eth_supported;
+	ethtool_link_mode_mask_t eth_advertising;
+	ethtool_link_mode_mask_t eth_lp_advertising;
 	int ret;
 
 	memset(&ptys_reg, 0, sizeof(ptys_reg));
@@ -624,41 +630,44 @@ static int ethtool_get_ptys_settings(struct net_device *dev,
 	en_dbg(DRV, priv, "ptys_reg.eth_proto_lp_adv %x\n",
 	       be32_to_cpu(ptys_reg.eth_proto_lp_adv));
 
-	cmd->supported = 0;
-	cmd->advertising = 0;
+	eth_supported = 0;
+	eth_advertising = 0;
 
-	cmd->supported |= ptys_get_supported_port(&ptys_reg);
+	eth_supported |= ptys_get_supported_port(&ptys_reg);
 
 	eth_proto = be32_to_cpu(ptys_reg.eth_proto_cap);
-	cmd->supported |= ptys2ethtool_link_modes(eth_proto, SUPPORTED);
+	eth_supported |= ptys2ethtool_link_modes(eth_proto, SUPPORTED);
 
 	eth_proto = be32_to_cpu(ptys_reg.eth_proto_admin);
-	cmd->advertising |= ptys2ethtool_link_modes(eth_proto, ADVERTISED);
+	eth_advertising |= ptys2ethtool_link_modes(eth_proto, ADVERTISED);
 
-	cmd->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
-	cmd->advertising |= (priv->prof->tx_pause) ? ADVERTISED_Pause : 0;
+	eth_supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+	eth_advertising |= (priv->prof->tx_pause) ? ADVERTISED_Pause : 0;
 
-	cmd->advertising |= (priv->prof->tx_pause ^ priv->prof->rx_pause) ?
+	eth_advertising |= (priv->prof->tx_pause ^ priv->prof->rx_pause) ?
 		ADVERTISED_Asym_Pause : 0;
 
 	cmd->port = ptys_get_active_port(&ptys_reg);
-	cmd->transceiver = (SUPPORTED_TP & cmd->supported) ?
+	cmd->transceiver = (SUPPORTED_TP & eth_supported) ?
 		XCVR_EXTERNAL : XCVR_INTERNAL;
 
 	if (mlx4_en_autoneg_get(dev)) {
-		cmd->supported |= SUPPORTED_Autoneg;
-		cmd->advertising |= ADVERTISED_Autoneg;
+		eth_supported |= SUPPORTED_Autoneg;
+		eth_advertising |= ADVERTISED_Autoneg;
 	}
 
 	cmd->autoneg = (priv->port_state.flags & MLX4_EN_PORT_ANC) ?
 		AUTONEG_ENABLE : AUTONEG_DISABLE;
 
 	eth_proto = be32_to_cpu(ptys_reg.eth_proto_lp_adv);
-	cmd->lp_advertising = ptys2ethtool_link_modes(eth_proto, ADVERTISED);
+	eth_lp_advertising = ptys2ethtool_link_modes(eth_proto, ADVERTISED);
 
-	cmd->lp_advertising |= (priv->port_state.flags & MLX4_EN_PORT_ANC) ?
+	eth_lp_advertising |= (priv->port_state.flags & MLX4_EN_PORT_ANC) ?
 			ADVERTISED_Autoneg : 0;
 
+	ethtool_cmd_supported_set(cmd, eth_supported);
+	ethtool_cmd_advertising_set(cmd, eth_advertising);
+	ethtool_cmd_lp_advertising_set(cmd, eth_lp_advertising);
 	cmd->phy_address = 0;
 	cmd->mdio_support = 0;
 	cmd->maxtxpkt = 0;
@@ -673,27 +682,30 @@ static void ethtool_get_default_settings(struct net_device *dev,
 					 struct ethtool_cmd *cmd)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
+	ethtool_link_mode_mask_t eth_supported, eth_advertising;
 	int trans_type;
 
 	cmd->autoneg = AUTONEG_DISABLE;
-	cmd->supported = SUPPORTED_10000baseT_Full;
-	cmd->advertising = ADVERTISED_10000baseT_Full;
+	eth_supported = SUPPORTED_10000baseT_Full;
+	eth_advertising = ADVERTISED_10000baseT_Full;
 	trans_type = priv->port_state.transceiver;
 
 	if (trans_type > 0 && trans_type <= 0xC) {
 		cmd->port = PORT_FIBRE;
 		cmd->transceiver = XCVR_EXTERNAL;
-		cmd->supported |= SUPPORTED_FIBRE;
-		cmd->advertising |= ADVERTISED_FIBRE;
+		eth_supported |= SUPPORTED_FIBRE;
+		eth_advertising |= ADVERTISED_FIBRE;
 	} else if (trans_type == 0x80 || trans_type == 0) {
 		cmd->port = PORT_TP;
 		cmd->transceiver = XCVR_INTERNAL;
-		cmd->supported |= SUPPORTED_TP;
-		cmd->advertising |= ADVERTISED_TP;
+		eth_supported |= SUPPORTED_TP;
+		eth_advertising |= ADVERTISED_TP;
 	} else  {
 		cmd->port = -1;
 		cmd->transceiver = -1;
 	}
+	ethtool_cmd_supported_set(cmd, eth_supported);
+	ethtool_cmd_advertising_set(cmd, eth_advertising);
 }
 
 static int mlx4_en_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
@@ -747,13 +759,14 @@ static int mlx4_en_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	struct mlx4_ptys_reg ptys_reg;
 	__be32 proto_admin;
+	const ethtool_link_mode_mask_t eth_adv = ethtool_cmd_advertising(cmd);
 	int ret;
 
-	u32 ptys_adv = ethtool2ptys_link_modes(cmd->advertising, ADVERTISED);
+	u32 ptys_adv = ethtool2ptys_link_modes(eth_adv, ADVERTISED);
 	int speed = ethtool_cmd_speed(cmd);
 
-	en_dbg(DRV, priv, "Set Speed=%d adv=0x%x autoneg=%d duplex=%d\n",
-	       speed, cmd->advertising, cmd->autoneg, cmd->duplex);
+	en_dbg(DRV, priv, "Set Speed=%d adv=0x%llx autoneg=%d duplex=%d\n",
+	       speed, eth_adv, cmd->autoneg, cmd->duplex);
 
 	if (!(priv->mdev->dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_ETH_PROT_CTRL) ||
 	    (cmd->duplex == DUPLEX_HALF))
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH net-next v1 5/7] net: veth: extend link mode support to 48 bits
From: David Decotigny @ 2015-01-04 20:56 UTC (permalink / raw)
  To: Amir Vadai, Florian Fainelli, netdev, linux-kernel, linux-api
  Cc: David Decotigny, 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, Saeed Mahameed, Venkata Duvvuru,
	Govindarajulu Varadarajan
In-Reply-To: <1420405017-23278-1-git-send-email-ddecotig@gmail.com>

From: David Decotigny <decot@googlers.com>

Signed-off-by: David Decotigny <decot@googlers.com>
---
 drivers/net/veth.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 8ad5965..902e127 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -49,8 +49,8 @@ static struct {
 
 static int veth_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
-	cmd->supported		= 0;
-	cmd->advertising	= 0;
+	ethtool_cmd_supported_set(cmd, 0);
+	ethtool_cmd_advertising_set(cmd, 0);
 	ethtool_cmd_speed_set(cmd, SPEED_10000);
 	cmd->duplex		= DUPLEX_FULL;
 	cmd->port		= PORT_TP;
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH net-next v1 4/7] net: mdio: extend link mode support to 48 bits
From: David Decotigny @ 2015-01-04 20:56 UTC (permalink / raw)
  To: Amir Vadai, Florian Fainelli, netdev, linux-kernel, linux-api
  Cc: David Decotigny, 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, Saeed Mahameed, Venkata Duvvuru,
	Govindarajulu Varadarajan
In-Reply-To: <1420405017-23278-1-git-send-email-ddecotig@gmail.com>

From: David Decotigny <decot@googlers.com>

Signed-off-by: David Decotigny <decot@googlers.com>
---
 drivers/net/mdio.c   | 59 +++++++++++++++++++++++++++++-----------------------
 include/linux/mdio.h | 15 +++++++------
 2 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/drivers/net/mdio.c b/drivers/net/mdio.c
index 3e027ed..5cac2ac 100644
--- a/drivers/net/mdio.c
+++ b/drivers/net/mdio.c
@@ -148,9 +148,10 @@ int mdio45_nway_restart(const struct mdio_if_info *mdio)
 }
 EXPORT_SYMBOL(mdio45_nway_restart);
 
-static u32 mdio45_get_an(const struct mdio_if_info *mdio, u16 addr)
+static ethtool_link_mode_mask_t
+mdio45_get_an(const struct mdio_if_info *mdio, u16 addr)
 {
-	u32 result = 0;
+	ethtool_link_mode_mask_t result = 0;
 	int reg;
 
 	reg = mdio->mdio_read(mdio->dev, mdio->prtad, MDIO_MMD_AN, addr);
@@ -185,9 +186,11 @@ static u32 mdio45_get_an(const struct mdio_if_info *mdio, u16 addr)
  */
 void mdio45_ethtool_gset_npage(const struct mdio_if_info *mdio,
 			       struct ethtool_cmd *ecmd,
-			       u32 npage_adv, u32 npage_lpa)
+			       ethtool_link_mode_mask_t npage_adv,
+			       ethtool_link_mode_mask_t npage_lpa)
 {
 	int reg;
+	ethtool_link_mode_mask_t supported_link_modes, advertising_link_modes;
 	u32 speed;
 
 	BUILD_BUG_ON(MDIO_SUPPORTS_C22 != ETH_MDIO_SUPPORTS_C22);
@@ -206,64 +209,64 @@ void mdio45_ethtool_gset_npage(const struct mdio_if_info *mdio,
 	case MDIO_PMA_CTRL2_100BTX:
 	case MDIO_PMA_CTRL2_10BT:
 		ecmd->port = PORT_TP;
-		ecmd->supported = SUPPORTED_TP;
+		supported_link_modes = SUPPORTED_TP;
 		reg = mdio->mdio_read(mdio->dev, mdio->prtad, MDIO_MMD_PMAPMD,
 				      MDIO_SPEED);
 		if (reg & MDIO_SPEED_10G)
-			ecmd->supported |= SUPPORTED_10000baseT_Full;
+			supported_link_modes |= SUPPORTED_10000baseT_Full;
 		if (reg & MDIO_PMA_SPEED_1000)
-			ecmd->supported |= (SUPPORTED_1000baseT_Full |
+			supported_link_modes |= (SUPPORTED_1000baseT_Full |
 					    SUPPORTED_1000baseT_Half);
 		if (reg & MDIO_PMA_SPEED_100)
-			ecmd->supported |= (SUPPORTED_100baseT_Full |
+			supported_link_modes |= (SUPPORTED_100baseT_Full |
 					    SUPPORTED_100baseT_Half);
 		if (reg & MDIO_PMA_SPEED_10)
-			ecmd->supported |= (SUPPORTED_10baseT_Full |
+			supported_link_modes |= (SUPPORTED_10baseT_Full |
 					    SUPPORTED_10baseT_Half);
-		ecmd->advertising = ADVERTISED_TP;
+		advertising_link_modes = ADVERTISED_TP;
 		break;
 
 	case MDIO_PMA_CTRL2_10GBCX4:
 		ecmd->port = PORT_OTHER;
-		ecmd->supported = 0;
-		ecmd->advertising = 0;
+		supported_link_modes = 0;
+		advertising_link_modes = 0;
 		break;
 
 	case MDIO_PMA_CTRL2_10GBKX4:
 	case MDIO_PMA_CTRL2_10GBKR:
 	case MDIO_PMA_CTRL2_1000BKX:
 		ecmd->port = PORT_OTHER;
-		ecmd->supported = SUPPORTED_Backplane;
+		supported_link_modes = SUPPORTED_Backplane;
 		reg = mdio->mdio_read(mdio->dev, mdio->prtad, MDIO_MMD_PMAPMD,
 				      MDIO_PMA_EXTABLE);
 		if (reg & MDIO_PMA_EXTABLE_10GBKX4)
-			ecmd->supported |= SUPPORTED_10000baseKX4_Full;
+			supported_link_modes |= SUPPORTED_10000baseKX4_Full;
 		if (reg & MDIO_PMA_EXTABLE_10GBKR)
-			ecmd->supported |= SUPPORTED_10000baseKR_Full;
+			supported_link_modes |= SUPPORTED_10000baseKR_Full;
 		if (reg & MDIO_PMA_EXTABLE_1000BKX)
-			ecmd->supported |= SUPPORTED_1000baseKX_Full;
+			supported_link_modes |= SUPPORTED_1000baseKX_Full;
 		reg = mdio->mdio_read(mdio->dev, mdio->prtad, MDIO_MMD_PMAPMD,
 				      MDIO_PMA_10GBR_FECABLE);
 		if (reg & MDIO_PMA_10GBR_FECABLE_ABLE)
-			ecmd->supported |= SUPPORTED_10000baseR_FEC;
-		ecmd->advertising = ADVERTISED_Backplane;
+			supported_link_modes |= SUPPORTED_10000baseR_FEC;
+		advertising_link_modes = ADVERTISED_Backplane;
 		break;
 
 	/* All the other defined modes are flavours of optical */
 	default:
 		ecmd->port = PORT_FIBRE;
-		ecmd->supported = SUPPORTED_FIBRE;
-		ecmd->advertising = ADVERTISED_FIBRE;
+		supported_link_modes = SUPPORTED_FIBRE;
+		advertising_link_modes = ADVERTISED_FIBRE;
 		break;
 	}
 
 	if (mdio->mmds & MDIO_DEVS_AN) {
-		ecmd->supported |= SUPPORTED_Autoneg;
+		supported_link_modes |= SUPPORTED_Autoneg;
 		reg = mdio->mdio_read(mdio->dev, mdio->prtad, MDIO_MMD_AN,
 				      MDIO_CTRL1);
 		if (reg & MDIO_AN_CTRL1_ENABLE) {
 			ecmd->autoneg = AUTONEG_ENABLE;
-			ecmd->advertising |=
+			advertising_link_modes |=
 				ADVERTISED_Autoneg |
 				mdio45_get_an(mdio, MDIO_AN_ADVERTISE) |
 				npage_adv;
@@ -275,21 +278,22 @@ void mdio45_ethtool_gset_npage(const struct mdio_if_info *mdio,
 	}
 
 	if (ecmd->autoneg) {
-		u32 modes = 0;
+		ethtool_link_mode_mask_t modes = 0;
 		int an_stat = mdio->mdio_read(mdio->dev, mdio->prtad,
 					      MDIO_MMD_AN, MDIO_STAT1);
 
 		/* If AN is complete and successful, report best common
 		 * mode, otherwise report best advertised mode. */
 		if (an_stat & MDIO_AN_STAT1_COMPLETE) {
-			ecmd->lp_advertising =
+			ethtool_link_mode_mask_t lp_adv =
 				mdio45_get_an(mdio, MDIO_AN_LPA) | npage_lpa;
 			if (an_stat & MDIO_AN_STAT1_LPABLE)
-				ecmd->lp_advertising |= ADVERTISED_Autoneg;
-			modes = ecmd->advertising & ecmd->lp_advertising;
+				lp_adv |= ADVERTISED_Autoneg;
+			ethtool_cmd_lp_advertising_set(ecmd, lp_adv);
+			modes = advertising_link_modes & lp_adv;
 		}
 		if ((modes & ~ADVERTISED_Autoneg) == 0)
-			modes = ecmd->advertising;
+			modes = advertising_link_modes;
 
 		if (modes & (ADVERTISED_10000baseT_Full |
 			     ADVERTISED_10000baseKX4_Full |
@@ -338,6 +342,9 @@ void mdio45_ethtool_gset_npage(const struct mdio_if_info *mdio,
 			break;
 		}
 	}
+
+	ethtool_cmd_supported_set(ecmd, supported_link_modes);
+	ethtool_cmd_advertising_set(ecmd, advertising_link_modes);
 }
 EXPORT_SYMBOL(mdio45_ethtool_gset_npage);
 
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index b42963b..18c649a 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -69,7 +69,8 @@ extern int mdio45_links_ok(const struct mdio_if_info *mdio, u32 mmds);
 extern int mdio45_nway_restart(const struct mdio_if_info *mdio);
 extern void mdio45_ethtool_gset_npage(const struct mdio_if_info *mdio,
 				      struct ethtool_cmd *ecmd,
-				      u32 npage_adv, u32 npage_lpa);
+				      ethtool_link_mode_mask_t npage_adv,
+				      ethtool_link_mode_mask_t npage_lpa);
 
 /**
  * mdio45_ethtool_gset - get settings for ETHTOOL_GSET
@@ -97,9 +98,10 @@ extern int mdio_mii_ioctl(const struct mdio_if_info *mdio,
  * A small helper function that translates MMD EEE Capability (3.20) bits
  * to ethtool supported settings.
  */
-static inline u32 mmd_eee_cap_to_ethtool_sup_t(u16 eee_cap)
+static inline ethtool_link_mode_mask_t
+mmd_eee_cap_to_ethtool_sup_t(u16 eee_cap)
 {
-	u32 supported = 0;
+	ethtool_link_mode_mask_t supported = 0;
 
 	if (eee_cap & MDIO_EEE_100TX)
 		supported |= SUPPORTED_100baseT_Full;
@@ -125,9 +127,10 @@ static inline u32 mmd_eee_cap_to_ethtool_sup_t(u16 eee_cap)
  * and MMD EEE Link Partner Ability (7.61) bits to ethtool advertisement
  * settings.
  */
-static inline u32 mmd_eee_adv_to_ethtool_adv_t(u16 eee_adv)
+static inline ethtool_link_mode_mask_t
+mmd_eee_adv_to_ethtool_adv_t(u16 eee_adv)
 {
-	u32 adv = 0;
+	ethtool_link_mode_mask_t adv = 0;
 
 	if (eee_adv & MDIO_EEE_100TX)
 		adv |= ADVERTISED_100baseT_Full;
@@ -153,7 +156,7 @@ static inline u32 mmd_eee_adv_to_ethtool_adv_t(u16 eee_adv)
  * to EEE advertisements for the MMD EEE Advertisement (7.60) and
  * MMD EEE Link Partner Ability (7.61) registers.
  */
-static inline u16 ethtool_adv_to_mmd_eee_adv_t(u32 adv)
+static inline u16 ethtool_adv_to_mmd_eee_adv_t(ethtool_link_mode_mask_t adv)
 {
 	u16 reg = 0;
 
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH net-next v1 3/7] net: mii: extend link mode support to 48 bits
From: David Decotigny @ 2015-01-04 20:56 UTC (permalink / raw)
  To: Amir Vadai, Florian Fainelli, netdev, linux-kernel, linux-api
  Cc: David Decotigny, 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, Saeed Mahameed, Venkata Duvvuru,
	Govindarajulu Varadarajan
In-Reply-To: <1420405017-23278-1-git-send-email-ddecotig@gmail.com>

From: David Decotigny <decot@googlers.com>

Signed-off-by: David Decotigny <decot@googlers.com>
---
 drivers/net/mii.c   | 52 +++++++++++++++++++++++++++++-----------------------
 include/linux/mii.h | 31 ++++++++++++++++---------------
 2 files changed, 45 insertions(+), 38 deletions(-)

diff --git a/drivers/net/mii.c b/drivers/net/mii.c
index 4a99c39..2be59ba 100644
--- a/drivers/net/mii.c
+++ b/drivers/net/mii.c
@@ -33,7 +33,7 @@
 #include <linux/ethtool.h>
 #include <linux/mii.h>
 
-static u32 mii_get_an(struct mii_if_info *mii, u16 addr)
+static ethtool_link_mode_mask_t mii_get_an(struct mii_if_info *mii, u16 addr)
 {
 	int advert;
 
@@ -56,14 +56,15 @@ int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
 {
 	struct net_device *dev = mii->dev;
 	u16 bmcr, bmsr, ctrl1000 = 0, stat1000 = 0;
-	u32 nego;
+	ethtool_link_mode_mask_t supported_link_modes, advertising_link_modes;
+	ethtool_link_mode_mask_t nego;
 
-	ecmd->supported =
+	supported_link_modes =
 	    (SUPPORTED_10baseT_Half | SUPPORTED_10baseT_Full |
 	     SUPPORTED_100baseT_Half | SUPPORTED_100baseT_Full |
 	     SUPPORTED_Autoneg | SUPPORTED_TP | SUPPORTED_MII);
 	if (mii->supports_gmii)
-		ecmd->supported |= SUPPORTED_1000baseT_Half |
+		supported_link_modes |= SUPPORTED_1000baseT_Half |
 			SUPPORTED_1000baseT_Full;
 
 	/* only supports twisted-pair */
@@ -76,7 +77,7 @@ int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
 	ecmd->phy_address = mii->phy_id;
 	ecmd->mdio_support = ETH_MDIO_SUPPORTS_C22;
 
-	ecmd->advertising = ADVERTISED_TP | ADVERTISED_MII;
+	advertising_link_modes = ADVERTISED_TP | ADVERTISED_MII;
 
 	bmcr = mii->mdio_read(dev, mii->phy_id, MII_BMCR);
 	bmsr = mii->mdio_read(dev, mii->phy_id, MII_BMSR);
@@ -85,23 +86,25 @@ int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
 		stat1000 = mii->mdio_read(dev, mii->phy_id, MII_STAT1000);
 	}
 	if (bmcr & BMCR_ANENABLE) {
-		ecmd->advertising |= ADVERTISED_Autoneg;
+		ethtool_link_mode_mask_t lp_adv;
+
+		advertising_link_modes |= ADVERTISED_Autoneg;
 		ecmd->autoneg = AUTONEG_ENABLE;
 
-		ecmd->advertising |= mii_get_an(mii, MII_ADVERTISE);
+		advertising_link_modes |= mii_get_an(mii, MII_ADVERTISE);
 		if (mii->supports_gmii)
-			ecmd->advertising |=
+			advertising_link_modes |=
 					mii_ctrl1000_to_ethtool_adv_t(ctrl1000);
 
 		if (bmsr & BMSR_ANEGCOMPLETE) {
-			ecmd->lp_advertising = mii_get_an(mii, MII_LPA);
-			ecmd->lp_advertising |=
-					mii_stat1000_to_ethtool_lpa_t(stat1000);
+			lp_adv = mii_get_an(mii, MII_LPA);
+			lp_adv |= mii_stat1000_to_ethtool_lpa_t(stat1000);
 		} else {
-			ecmd->lp_advertising = 0;
+			lp_adv = 0;
 		}
 
-		nego = ecmd->advertising & ecmd->lp_advertising;
+		ethtool_cmd_lp_advertising_set(ecmd, lp_adv);
+		nego = advertising_link_modes & lp_adv;
 
 		if (nego & (ADVERTISED_1000baseT_Full |
 			    ADVERTISED_1000baseT_Half)) {
@@ -128,6 +131,8 @@ int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
 	}
 
 	mii->full_duplex = ecmd->duplex;
+	ethtool_cmd_supported_set(ecmd, supported_link_modes);
+	ethtool_cmd_advertising_set(ecmd, advertising_link_modes);
 
 	/* ignore maxtxpkt, maxrxpkt for now */
 
@@ -168,13 +173,15 @@ int mii_ethtool_sset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
 	if (ecmd->autoneg == AUTONEG_ENABLE) {
 		u32 bmcr, advert, tmp;
 		u32 advert2 = 0, tmp2 = 0;
-
-		if ((ecmd->advertising & (ADVERTISED_10baseT_Half |
-					  ADVERTISED_10baseT_Full |
-					  ADVERTISED_100baseT_Half |
-					  ADVERTISED_100baseT_Full |
-					  ADVERTISED_1000baseT_Half |
-					  ADVERTISED_1000baseT_Full)) == 0)
+		ethtool_link_mode_mask_t ethtool_adv;
+
+		ethtool_adv = ethtool_cmd_advertising(ecmd);
+		if ((ethtool_adv & (ADVERTISED_10baseT_Half |
+				    ADVERTISED_10baseT_Full |
+				    ADVERTISED_100baseT_Half |
+				    ADVERTISED_100baseT_Full |
+				    ADVERTISED_1000baseT_Half |
+				    ADVERTISED_1000baseT_Full)) == 0)
 			return -EINVAL;
 
 		/* advertise only what has been requested */
@@ -184,11 +191,10 @@ int mii_ethtool_sset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
 			advert2 = mii->mdio_read(dev, mii->phy_id, MII_CTRL1000);
 			tmp2 = advert2 & ~(ADVERTISE_1000HALF | ADVERTISE_1000FULL);
 		}
-		tmp |= ethtool_adv_to_mii_adv_t(ecmd->advertising);
+		tmp |= ethtool_adv_to_mii_adv_t(ethtool_adv);
 
 		if (mii->supports_gmii)
-			tmp2 |=
-			      ethtool_adv_to_mii_ctrl1000_t(ecmd->advertising);
+			tmp2 |= ethtool_adv_to_mii_ctrl1000_t(ethtool_adv);
 		if (advert != tmp) {
 			mii->mdio_write(dev, mii->phy_id, MII_ADVERTISE, tmp);
 			mii->advertising = tmp;
diff --git a/include/linux/mii.h b/include/linux/mii.h
index 47492c9..6f5336c 100644
--- a/include/linux/mii.h
+++ b/include/linux/mii.h
@@ -106,7 +106,7 @@ static inline unsigned int mii_duplex (unsigned int duplex_lock,
  * settings to phy autonegotiation advertisements for the
  * MII_ADVERTISE register.
  */
-static inline u32 ethtool_adv_to_mii_adv_t(u32 ethadv)
+static inline u32 ethtool_adv_to_mii_adv_t(ethtool_link_mode_mask_t ethadv)
 {
 	u32 result = 0;
 
@@ -133,9 +133,9 @@ static inline u32 ethtool_adv_to_mii_adv_t(u32 ethadv)
  * A small helper function that translates MII_ADVERTISE bits
  * to ethtool advertisement settings.
  */
-static inline u32 mii_adv_to_ethtool_adv_t(u32 adv)
+static inline ethtool_link_mode_mask_t mii_adv_to_ethtool_adv_t(u32 adv)
 {
-	u32 result = 0;
+	ethtool_link_mode_mask_t result = 0;
 
 	if (adv & ADVERTISE_10HALF)
 		result |= ADVERTISED_10baseT_Half;
@@ -161,7 +161,8 @@ static inline u32 mii_adv_to_ethtool_adv_t(u32 adv)
  * settings to phy autonegotiation advertisements for the
  * MII_CTRL1000 register when in 1000T mode.
  */
-static inline u32 ethtool_adv_to_mii_ctrl1000_t(u32 ethadv)
+static inline u32
+ethtool_adv_to_mii_ctrl1000_t(ethtool_link_mode_mask_t ethadv)
 {
 	u32 result = 0;
 
@@ -181,9 +182,9 @@ static inline u32 ethtool_adv_to_mii_ctrl1000_t(u32 ethadv)
  * bits, when in 1000Base-T mode, to ethtool
  * advertisement settings.
  */
-static inline u32 mii_ctrl1000_to_ethtool_adv_t(u32 adv)
+static inline ethtool_link_mode_mask_t mii_ctrl1000_to_ethtool_adv_t(u32 adv)
 {
-	u32 result = 0;
+	ethtool_link_mode_mask_t result = 0;
 
 	if (adv & ADVERTISE_1000HALF)
 		result |= ADVERTISED_1000baseT_Half;
@@ -201,9 +202,9 @@ static inline u32 mii_ctrl1000_to_ethtool_adv_t(u32 adv)
  * bits, when in 1000Base-T mode, to ethtool
  * LP advertisement settings.
  */
-static inline u32 mii_lpa_to_ethtool_lpa_t(u32 lpa)
+static inline ethtool_link_mode_mask_t mii_lpa_to_ethtool_lpa_t(u32 lpa)
 {
-	u32 result = 0;
+	ethtool_link_mode_mask_t result = 0;
 
 	if (lpa & LPA_LPACK)
 		result |= ADVERTISED_Autoneg;
@@ -219,9 +220,9 @@ static inline u32 mii_lpa_to_ethtool_lpa_t(u32 lpa)
  * bits, when in 1000Base-T mode, to ethtool
  * advertisement settings.
  */
-static inline u32 mii_stat1000_to_ethtool_lpa_t(u32 lpa)
+static inline ethtool_link_mode_mask_t mii_stat1000_to_ethtool_lpa_t(u32 lpa)
 {
-	u32 result = 0;
+	ethtool_link_mode_mask_t result = 0;
 
 	if (lpa & LPA_1000HALF)
 		result |= ADVERTISED_1000baseT_Half;
@@ -239,7 +240,7 @@ static inline u32 mii_stat1000_to_ethtool_lpa_t(u32 lpa)
  * settings to phy autonegotiation advertisements for the
  * MII_CTRL1000 register when in 1000Base-X mode.
  */
-static inline u32 ethtool_adv_to_mii_adv_x(u32 ethadv)
+static inline u32 ethtool_adv_to_mii_adv_x(ethtool_link_mode_mask_t ethadv)
 {
 	u32 result = 0;
 
@@ -263,9 +264,9 @@ static inline u32 ethtool_adv_to_mii_adv_x(u32 ethadv)
  * bits, when in 1000Base-X mode, to ethtool
  * advertisement settings.
  */
-static inline u32 mii_adv_to_ethtool_adv_x(u32 adv)
+static inline ethtool_link_mode_mask_t mii_adv_to_ethtool_adv_x(u32 adv)
 {
-	u32 result = 0;
+	ethtool_link_mode_mask_t result = 0;
 
 	if (adv & ADVERTISE_1000XHALF)
 		result |= ADVERTISED_1000baseT_Half;
@@ -287,9 +288,9 @@ static inline u32 mii_adv_to_ethtool_adv_x(u32 adv)
  * bits, when in 1000Base-X mode, to ethtool
  * LP advertisement settings.
  */
-static inline u32 mii_lpa_to_ethtool_lpa_x(u32 lpa)
+static inline ethtool_link_mode_mask_t mii_lpa_to_ethtool_lpa_x(u32 lpa)
 {
-	u32 result = 0;
+	ethtool_link_mode_mask_t result = 0;
 
 	if (lpa & LPA_LPACK)
 		result |= ADVERTISED_Autoneg;
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH net-next v1 2/7] net: phy: extend link mode support to 48 bits
From: David Decotigny @ 2015-01-04 20:56 UTC (permalink / raw)
  To: Amir Vadai, Florian Fainelli, netdev, linux-kernel, linux-api
  Cc: David Decotigny, 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, Saeed Mahameed, Venkata Duvvuru,
	Govindarajulu Varadarajan
In-Reply-To: <1420405017-23278-1-git-send-email-ddecotig@gmail.com>

From: David Decotigny <decot@googlers.com>

Signed-off-by: David Decotigny <decot@googlers.com>
---
 drivers/net/phy/phy.c        | 29 ++++++++++++++---------------
 drivers/net/phy/phy_device.c |  4 ++--
 include/linux/phy.h          | 10 +++++-----
 3 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 767cd11..e9c8499 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -132,7 +132,7 @@ static inline int phy_aneg_done(struct phy_device *phydev)
 struct phy_setting {
 	int speed;
 	int duplex;
-	u32 setting;
+	ethtool_link_mode_mask_t setting;
 };
 
 /* A mapping of all SUPPORTED settings to speed/duplex */
@@ -227,7 +227,8 @@ static inline unsigned int phy_find_setting(int speed, int duplex)
  *   the mask in features.  Returns the index of the last setting
  *   if nothing else matches.
  */
-static inline unsigned int phy_find_valid(unsigned int idx, u32 features)
+static inline unsigned int phy_find_valid(unsigned int idx,
+					  ethtool_link_mode_mask_t features)
 {
 	while (idx < MAX_NUM_SETTINGS && !(settings[idx].setting & features))
 		idx++;
@@ -245,7 +246,7 @@ static inline unsigned int phy_find_valid(unsigned int idx, u32 features)
  */
 static void phy_sanitize_settings(struct phy_device *phydev)
 {
-	u32 features = phydev->supported;
+	ethtool_link_mode_mask_t features = phydev->supported;
 	unsigned int idx;
 
 	/* Sanitize settings based on PHY capabilities */
@@ -279,13 +280,13 @@ int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd)
 		return -EINVAL;
 
 	/* We make sure that we don't pass unsupported values in to the PHY */
-	cmd->advertising &= phydev->supported;
+	phydev->advertising = ethtool_cmd_advertising(cmd) & phydev->supported;
 
 	/* Verify the settings we care about. */
 	if (cmd->autoneg != AUTONEG_ENABLE && cmd->autoneg != AUTONEG_DISABLE)
 		return -EINVAL;
 
-	if (cmd->autoneg == AUTONEG_ENABLE && cmd->advertising == 0)
+	if (cmd->autoneg == AUTONEG_ENABLE && phydev->advertising == 0)
 		return -EINVAL;
 
 	if (cmd->autoneg == AUTONEG_DISABLE &&
@@ -300,8 +301,6 @@ int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd)
 
 	phydev->speed = speed;
 
-	phydev->advertising = cmd->advertising;
-
 	if (AUTONEG_ENABLE == cmd->autoneg)
 		phydev->advertising |= ADVERTISED_Autoneg;
 	else
@@ -318,10 +317,10 @@ EXPORT_SYMBOL(phy_ethtool_sset);
 
 int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd)
 {
-	cmd->supported = phydev->supported;
+	ethtool_cmd_supported_set(cmd, phydev->supported);
 
-	cmd->advertising = phydev->advertising;
-	cmd->lp_advertising = phydev->lp_advertising;
+	ethtool_cmd_advertising_set(cmd, phydev->advertising);
+	ethtool_cmd_lp_advertising_set(cmd, phydev->lp_advertising);
 
 	ethtool_cmd_speed_set(cmd, phydev->speed);
 	cmd->duplex = phydev->duplex;
@@ -1040,7 +1039,7 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable)
 	    (phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
 	     phy_is_internal(phydev))) {
 		int eee_lp, eee_cap, eee_adv;
-		u32 lp, cap, adv;
+		ethtool_link_mode_mask_t cap, adv, lp;
 		int status;
 		unsigned int idx;
 
@@ -1132,21 +1131,21 @@ int phy_ethtool_get_eee(struct phy_device *phydev, struct ethtool_eee *data)
 				    MDIO_MMD_PCS, phydev->addr);
 	if (val < 0)
 		return val;
-	data->supported = mmd_eee_cap_to_ethtool_sup_t(val);
+	ethtool_eee_supported_set(data, mmd_eee_cap_to_ethtool_sup_t(val));
 
 	/* Get advertisement EEE */
 	val = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_ADV,
 				    MDIO_MMD_AN, phydev->addr);
 	if (val < 0)
 		return val;
-	data->advertised = mmd_eee_adv_to_ethtool_adv_t(val);
+	ethtool_eee_advertised_set(data, mmd_eee_adv_to_ethtool_adv_t(val));
 
 	/* Get LP advertisement EEE */
 	val = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_LPABLE,
 				    MDIO_MMD_AN, phydev->addr);
 	if (val < 0)
 		return val;
-	data->lp_advertised = mmd_eee_adv_to_ethtool_adv_t(val);
+	ethtool_eee_lp_advertised_set(data, mmd_eee_adv_to_ethtool_adv_t(val));
 
 	return 0;
 }
@@ -1161,7 +1160,7 @@ EXPORT_SYMBOL(phy_ethtool_get_eee);
  */
 int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
 {
-	int val = ethtool_adv_to_mmd_eee_adv_t(data->advertised);
+	int val = ethtool_adv_to_mmd_eee_adv_t(ethtool_eee_advertised(data));
 
 	phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN,
 			       phydev->addr, val);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 3fc91e8..4391dc7 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -734,7 +734,7 @@ EXPORT_SYMBOL(phy_resume);
  */
 static int genphy_config_advert(struct phy_device *phydev)
 {
-	u32 advertise;
+	ethtool_link_mode_mask_t advertise;
 	int oldadv, adv, bmsr;
 	int err, changed = 0;
 
@@ -1086,7 +1086,7 @@ EXPORT_SYMBOL(genphy_soft_reset);
 int genphy_config_init(struct phy_device *phydev)
 {
 	int val;
-	u32 features;
+	ethtool_link_mode_mask_t features;
 
 	features = (SUPPORTED_TP | SUPPORTED_MII
 			| SUPPORTED_AUI | SUPPORTED_FIBRE |
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 22af8f8..aabf808 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -390,10 +390,10 @@ struct phy_device {
 	u32 interrupts;
 
 	/* Union of PHY and Attached devices' supported modes */
-	/* See mii.h for more info */
-	u32 supported;
-	u32 advertising;
-	u32 lp_advertising;
+	/* See ethtool.h for more info */
+	ethtool_link_mode_mask_t supported;
+	ethtool_link_mode_mask_t advertising;
+	ethtool_link_mode_mask_t lp_advertising;
 
 	int autoneg;
 
@@ -447,7 +447,7 @@ struct phy_driver {
 	u32 phy_id;
 	char *name;
 	unsigned int phy_id_mask;
-	u32 features;
+	ethtool_link_mode_mask_t features;
 	u32 flags;
 	const void *driver_data;
 
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH net-next v1 1/7] net: ethtool: extend link mode support to 48 bits
From: David Decotigny @ 2015-01-04 20:56 UTC (permalink / raw)
  To: Amir Vadai, Florian Fainelli, netdev, linux-kernel, linux-api
  Cc: David Decotigny, 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, Saeed Mahameed, Venkata Duvvuru,
	Govindarajulu Varadarajan
In-Reply-To: <1420405017-23278-1-git-send-email-ddecotig@gmail.com>

From: David Decotigny <decot@googlers.com>

Signed-off-by: David Decotigny <decot@googlers.com>
---
 include/uapi/linux/ethtool.h | 130 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 110 insertions(+), 20 deletions(-)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 5f66d9c..61e7734 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -23,14 +23,16 @@
 /**
  * struct ethtool_cmd - link control and status
  * @cmd: Command number = %ETHTOOL_GSET or %ETHTOOL_SSET
- * @supported: Bitmask of %SUPPORTED_* flags for the link modes,
- *	physical connectors and other link features for which the
- *	interface supports autonegotiation or auto-detection.
- *	Read-only.
- * @advertising: Bitmask of %ADVERTISED_* flags for the link modes,
- *	physical connectors and other link features that are
- *	advertised through autonegotiation or enabled for
- *	auto-detection.
+ * @supported: Low bits of bitmask of %SUPPORTED_* flags for the link
+ *	modes, physical connectors and other link features for which
+ *	the interface supports autonegotiation or auto-detection.
+ *	Read-only. Please do not access this field directly, use the
+ *	%ethtool_cmd_supported_* family of functions instead.
+ * @advertising: Low bits of bitmask of %ADVERTISED_* flags for the
+ *	link modes, physical connectors and other link features that
+ *	are advertised through autonegotiation or enabled for
+ *	auto-detection. Please do not access this field directly, use
+ *	the %ethtool_cmd_advertising_* family of functions instead.
  * @speed: Low bits of the speed
  * @duplex: Duplex mode; one of %DUPLEX_*
  * @port: Physical connector type; one of %PORT_*
@@ -56,10 +58,22 @@
  *	yield %ETH_TP_MDI_INVALID and writes may be ignored or rejected.
  *	When written successfully, the link should be renegotiated if
  *	necessary.
- * @lp_advertising: Bitmask of %ADVERTISED_* flags for the link modes
- *	and other link features that the link partner advertised
- *	through autonegotiation; 0 if unknown or not applicable.
- *	Read-only.
+ * @lp_advertising: Low bits of bitmask of %ADVERTISED_* flags for the
+ *      link modes and other link features that the link partner
+ *      advertised through autonegotiation; 0 if unknown or not
+ *      applicable.  Read-only. Please do not access this field
+ *      directly, use the %ethtool_cmd_lp_advertising_* family of
+ *      functions instead.
+ * @supported_hi: High bits of bitmask of %SUPPORTED_* flags. See
+ *      %supported. Please do not access this field directly, use the
+ *      %ethtool_cmd_supported_* family of functions instead.
+ * @advertising_hi: High bits of bitmask of %ADVERTISING_* flags. See
+ *      %advertising. Please do not access this field directly, use
+ *      the %ethtool_cmd_advertising_* family of functions instead.
+ * @lp_advertising_hi: High bits of bitmask of %ADVERTISING_* flags.
+ *      See %lp_advertising. Please do not access this field directly,
+ *      use the %ethtool_cmd_lp_advertising_* family of functions
+ *      instead.
  *
  * The link speed in Mbps is split between @speed and @speed_hi.  Use
  * the ethtool_cmd_speed() and ethtool_cmd_speed_set() functions to
@@ -107,7 +121,10 @@ struct ethtool_cmd {
 	__u8	eth_tp_mdix;
 	__u8	eth_tp_mdix_ctrl;
 	__u32	lp_advertising;
-	__u32	reserved[2];
+	__u16	supported_hi;
+	__u16	advertising_hi;
+	__u16	lp_advertising_hi;
+	__u16	reserved;
 };
 
 static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
@@ -123,6 +140,46 @@ static inline __u32 ethtool_cmd_speed(const struct ethtool_cmd *ep)
 	return (ep->speed_hi << 16) | ep->speed;
 }
 
+/**
+ * MAKE_ETHTOOL_LINK_MODE_ACCESSORS - create the link_mode accessors
+ * Macro to generate the %ethtool_cmd_supported_*,
+ * %ethtool_cmd_advertising_*, %ethtool_cmd_lp_advertising_*,
+ * %ethtool_eee_supported_*, %ethtool_eee_advertised_*,
+ * %ethtool_eee_lp_advertised_* families of functions.
+ *
+ * Macro args:
+ *  @struct_name: either %ethtool_cmd or %ethtool_eee
+ *  @field_name: name of the fields in %struct_name to
+ *      access. supported/advertising/lp_advertising for ethtool_cmd,
+ *      supported/advertised/lp_advertised for ethtool_eee
+ *
+ * Generates the following static functions:
+ *  @ethtool_cmd_supported(const struct ethtool_cmd*): returns
+ *      the 64b value of %supported fields (the upper bits 63..48 are 0)
+ *  @ethtool_cmd_supported_set(struct ethtool_cmd*,
+ *      ethtool_link_mode_mask_t value): set the %supported fields to
+ *      given %value (only the lower 48 bits used, upper bits 63..48
+ *      ignored)
+ *
+ * Same doc for all the other functions.
+ */
+#define MAKE_ETHTOOL_LINK_MODE_ACCESSORS(struct_name, field_name)	\
+	static inline ethtool_link_mode_mask_t				\
+	struct_name ## _ ## field_name(const struct struct_name *cmd)	\
+	{ ethtool_link_mode_mask_t r = cmd->field_name;			\
+	  r |= ((__u64)cmd->field_name ## _hi) << 32; return r; }	\
+	static inline void						\
+	struct_name ## _ ## field_name ## _set(struct struct_name *cmd,	\
+					       ethtool_link_mode_mask_t mask) \
+	{ cmd->field_name = mask & 0xffffffff;				\
+	  cmd->field_name ## _hi = (mask >> 32) & 0xffff; }
+
+typedef __u64 ethtool_link_mode_mask_t;
+
+MAKE_ETHTOOL_LINK_MODE_ACCESSORS(ethtool_cmd, supported);
+MAKE_ETHTOOL_LINK_MODE_ACCESSORS(ethtool_cmd, advertising);
+MAKE_ETHTOOL_LINK_MODE_ACCESSORS(ethtool_cmd, lp_advertising);
+
 /* Device supports clause 22 register access to PHY or peripherals
  * using the interface defined in <linux/mii.h>.  This should not be
  * set if there are known to be no such peripherals present or if
@@ -287,12 +344,18 @@ struct ethtool_eeprom {
 /**
  * struct ethtool_eee - Energy Efficient Ethernet information
  * @cmd: ETHTOOL_{G,S}EEE
- * @supported: Mask of %SUPPORTED_* flags for the speed/duplex combinations
- *	for which there is EEE support.
- * @advertised: Mask of %ADVERTISED_* flags for the speed/duplex combinations
- *	advertised as eee capable.
- * @lp_advertised: Mask of %ADVERTISED_* flags for the speed/duplex
- *	combinations advertised by the link partner as eee capable.
+ * @supported: Low bits of mask of %SUPPORTED_* flags for the
+ *	speed/duplex combinations for which there is EEE
+ *	support. Please do not access this field directly, use the
+ *	%ethtool_eee_supported_* family of functions instead.
+ * @advertised: Low bits of mask of %ADVERTISED_* flags for the
+ *	speed/duplex combinations advertised as eee capable. Please do
+ *	not access this field directly, use the
+ *	%ethtool_eee_advertised_* family of functions instead.
+ * @lp_advertised: Low bits of mask of %ADVERTISED_* flags for the
+ *	speed/duplex combinations advertised by the link partner as
+ *	eee capable.  Please do not access this field directly, use
+ *	the %ethtool_eee_lp_advertised_* family of functions instead.
  * @eee_active: Result of the eee auto negotiation.
  * @eee_enabled: EEE configured mode (enabled/disabled).
  * @tx_lpi_enabled: Whether the interface should assert its tx lpi, given
@@ -300,6 +363,16 @@ struct ethtool_eeprom {
  * @tx_lpi_timer: Time in microseconds the interface delays prior to asserting
  *	its tx lpi (after reaching 'idle' state). Effective only when eee
  *	was negotiated and tx_lpi_enabled was set.
+ * @supported_hi: High bits of mask of %SUPPORTED_* flags. See
+ *      %supported. Please do not access this field directly, use the
+ *      %ethtool_eee_supported_* family of functions instead.
+ * @advertised_hi: High bits of mask of %ADVERTISING_* flags. See
+ *      %advertising. Please do not access this field directly, use
+ *      the %ethtool_eee_advertising_* family of functions instead.
+ * @lp_advertised_hi: High bits of mask of %ADVERTISING_* flags.
+ *      See %lp_advertising. Please do not access this field directly,
+ *      use the %ethtool_eee_lp_advertising_* family of functions
+ *      instead.
  */
 struct ethtool_eee {
 	__u32	cmd;
@@ -310,9 +383,17 @@ struct ethtool_eee {
 	__u32	eee_enabled;
 	__u32	tx_lpi_enabled;
 	__u32	tx_lpi_timer;
-	__u32	reserved[2];
+	__u16	supported_hi;
+	__u16	advertised_hi;
+	__u16	lp_advertised_hi;
+	__u16	reserved;
 };
 
+MAKE_ETHTOOL_LINK_MODE_ACCESSORS(ethtool_eee, supported);
+MAKE_ETHTOOL_LINK_MODE_ACCESSORS(ethtool_eee, advertised);
+MAKE_ETHTOOL_LINK_MODE_ACCESSORS(ethtool_eee, lp_advertised);
+
+
 /**
  * struct ethtool_modinfo - plugin module eeprom information
  * @cmd: %ETHTOOL_GMODULEINFO
@@ -1192,6 +1273,9 @@ enum ethtool_sfeatures_retval_bits {
 #define SPARC_ETH_GSET		ETHTOOL_GSET
 #define SPARC_ETH_SSET		ETHTOOL_SSET
 
+/* Do not use the following macros directly to update
+ * ethtool_cmd::supported, ethtool_eee::supported. Please use
+ * ethtool_(cmd|eee)_supported(|_set) instead */
 #define SUPPORTED_10baseT_Half		(1 << 0)
 #define SUPPORTED_10baseT_Full		(1 << 1)
 #define SUPPORTED_100baseT_Half		(1 << 2)
@@ -1223,7 +1307,12 @@ enum ethtool_sfeatures_retval_bits {
 #define SUPPORTED_56000baseCR4_Full	(1 << 28)
 #define SUPPORTED_56000baseSR4_Full	(1 << 29)
 #define SUPPORTED_56000baseLR4_Full	(1 << 30)
+/* TODO: for bit indices >= 31, make sure to shift 1ULL instead of 1 */
 
+/* Do not use the following macros directly to update
+ * ethtool_cmd::advertising, ethtool_cmd::lp_advertising,
+ * ethtool_eee::advertised, ethtool_eee::lp_advertised. Please use
+ * ethtool_(cmd|eee)_*(|_set) */
 #define ADVERTISED_10baseT_Half		(1 << 0)
 #define ADVERTISED_10baseT_Full		(1 << 1)
 #define ADVERTISED_100baseT_Half	(1 << 2)
@@ -1255,6 +1344,7 @@ enum ethtool_sfeatures_retval_bits {
 #define ADVERTISED_56000baseCR4_Full	(1 << 28)
 #define ADVERTISED_56000baseSR4_Full	(1 << 29)
 #define ADVERTISED_56000baseLR4_Full	(1 << 30)
+/* TODO: for bit indices >= 31, make sure to shift 1ULL instead of 1 */
 
 /* The following are all involved in forcing a particular link
  * mode for the device for setting things.  When getting the
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH net-next v1 0/7] net: extend ethtool link mode bitmaps to 48 bits
From: David Decotigny @ 2015-01-04 20:56 UTC (permalink / raw)
  To: Amir Vadai, Florian Fainelli, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
  Cc: David Decotigny, 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, Saeed Mahameed, Venkata Duvvuru,
	Govindarajulu Varadarajan

From: David Decotigny <decot-Ypc/8FJVVoBWk0Htik3J/w@public.gmane.org>

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?).

It should not cause any backward compatibility issues for now, as long
as non-updated drivers don't depend on link modes beyond bit 31. But
it is recommended we gradually adopt the new get/set API in order to
correctly support future link modes. See ethtool.h for details.

I updated a couple drivers (mlx4, veth, tun), and some shared code in
drivers/net (phy, mii, mdio). It might be overkill for phy/mii/mdio,
and I might have missed other shared code in drivers/net. Please let
me know.

I used the compiler on my private copy to 1/ track where the
ethtool_cmd/ethtool_eee link mode fields were used 2/ track where the
link mode bitmaps are used and updated. So I believe that there is
some sort of transitive closure for the code I updated. Maybe tools
like coccinelle or clang could allow to automate these processes (1/
would probably be easy-ish, but 2/ seems a bit more complex)? I
reverted these internal "tracking" tricks for this version to minimize
impact on uapi/ethtool.h. The main resulting visible artifact of those
tricks is the new ethtool_link_mode_mask_t typedef (aka. u64). I kept
this trivial typedef here to help future refactoring, but I'd be happy
to rename it as plain "u64" if you prefer.

I can send updates to other drivers, even though it's rather pointless
to update 1G drivers at this point for example. Please let me know,
but I'd prefer to do this in follow-up patches outside this first
patch series.

############################################
# Patch Set Summary:

David Decotigny (7):
  net: ethtool: extend link mode support to 48 bits
  net: phy: extend link mode support to 48 bits
  net: mii: extend link mode support to 48 bits
  net: mdio: extend link mode support to 48 bits
  net: veth: extend link mode support to 48 bits
  net: tun: extend link mode support to 48 bits
  net: mlx4_en: extend link mode support to 48 bits

 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c |  67 +++++++-----
 drivers/net/mdio.c                              |  59 ++++++-----
 drivers/net/mii.c                               |  52 +++++-----
 drivers/net/phy/phy.c                           |  29 +++---
 drivers/net/phy/phy_device.c                    |   4 +-
 drivers/net/tun.c                               |   4 +-
 drivers/net/veth.c                              |   4 +-
 include/linux/mdio.h                            |  15 +--
 include/linux/mii.h                             |  31 +++---
 include/linux/phy.h                             |  10 +-
 include/uapi/linux/ethtool.h                    | 130 ++++++++++++++++++++----
 11 files changed, 262 insertions(+), 143 deletions(-)

-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply

* [PATCH iproute2 v3 3/3] ss: Filtering logic changing, with fixes
From: Vadim Kochan @ 2015-01-04 20:18 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan
In-Reply-To: <1420402720-28767-1-git-send-email-vadim4j@gmail.com>

From: Vadim Kochan <vadim4j@gmail.com>

This patch fixes some filtering combinations issues which does not
work on the 'master' version:

    $ ss -4
    shows inet & unix sockets, instead of only inet sockets

    $ ss -u
    needs to specify 'state closed'

    $ ss src unix:*X11*
    needs to specify '-x' shortcut for UNIX family

    $ ss -A all
    shows only sockets with established states

There might some other issues which was not observed.

Also changed logic for calculating families, socket types and
states filtering. I think that this version is a little simpler
one. Now there are 2 predefined default tables which describes
the following maping:

    family  -> (states, dbs)
    db      -> (states, families)

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 misc/ss.c | 318 ++++++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 207 insertions(+), 111 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 5058224..8879c32 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -129,6 +129,7 @@ enum
 #define PACKET_DBM ((1<<PACKET_DG_DB)|(1<<PACKET_R_DB))
 #define UNIX_DBM ((1<<UNIX_DG_DB)|(1<<UNIX_ST_DB)|(1<<UNIX_SQ_DB))
 #define ALL_DB ((1<<MAX_DB)-1)
+#define INET_DBM ((1<<TCP_DB)|(1<<UDP_DB)|(1<<DCCP_DB)|(1<<RAW_DB))
 
 enum {
 	SS_UNKNOWN,
@@ -146,7 +147,8 @@ enum {
 	SS_MAX
 };
 
-#define SS_ALL ((1<<SS_MAX)-1)
+#define SS_ALL ((1 << SS_MAX) - 1)
+#define SS_CONN (SS_ALL & ~((1<<SS_LISTEN)|(1<<SS_CLOSE)|(1<<SS_TIME_WAIT)|(1<<SS_SYN_RECV)))
 
 #include "ssfilter.h"
 
@@ -158,13 +160,126 @@ struct filter
 	struct ssfilter *f;
 };
 
-struct filter default_filter = {
-	.dbs	=  ~0,
-	.states = SS_ALL & ~((1<<SS_LISTEN)|(1<<SS_CLOSE)|(1<<SS_TIME_WAIT)|(1<<SS_SYN_RECV)),
-	.families= (1<<AF_INET)|(1<<AF_INET6),
+const struct filter default_dbs[MAX_DB] = {
+	[TCP_DB] = {
+		.states   = SS_CONN,
+		.families = (1 << AF_INET) | (1 << AF_INET6),
+	},
+	[DCCP_DB] = {
+		.states   = SS_CONN,
+		.families = (1 << AF_INET) | (1 << AF_INET6),
+	},
+	[UDP_DB] = {
+		.states   = (1 << SS_CLOSE),
+		.families = (1 << AF_INET) | (1 << AF_INET6),
+	},
+	[RAW_DB] = {
+		.states   = (1 << SS_CLOSE),
+		.families = (1 << AF_INET) | (1 << AF_INET6),
+	},
+	[UNIX_DG_DB] = {
+		.states   = (1 << SS_CLOSE),
+		.families = (1 << AF_UNIX),
+	},
+	[UNIX_ST_DB] = {
+		.states   = SS_CONN,
+		.families = (1 << AF_UNIX),
+	},
+	[UNIX_SQ_DB] = {
+		.states   = SS_CONN,
+		.families = (1 << AF_UNIX),
+	},
+	[PACKET_DG_DB] = {
+		.states   = (1 << SS_CLOSE),
+		.families = (1 << AF_PACKET),
+	},
+	[PACKET_R_DB] = {
+		.states   = (1 << SS_CLOSE),
+		.families = (1 << AF_PACKET),
+	},
+	[NETLINK_DB] = {
+		.states   = (1 << SS_CLOSE),
+		.families = (1 << AF_NETLINK),
+	},
 };
 
-struct filter current_filter;
+const struct filter default_afs[AF_MAX] = {
+	[AF_INET] = {
+		.dbs    = INET_DBM,
+		.states = SS_CONN,
+	},
+	[AF_INET6] = {
+		.dbs    = INET_DBM,
+		.states = SS_CONN,
+	},
+	[AF_UNIX] = {
+		.dbs    = UNIX_DBM,
+		.states = SS_CONN,
+	},
+	[AF_PACKET] = {
+		.dbs    = PACKET_DBM,
+		.states = (1 << SS_CLOSE),
+	},
+	[AF_NETLINK] = {
+		.dbs    = (1 << NETLINK_DB),
+		.states = (1 << SS_CLOSE),
+	},
+};
+
+static int do_default = 1;
+static struct filter current_filter;
+
+static void filter_db_set(struct filter *f, int db)
+{
+	f->states   |= default_dbs[db].states;
+	f->families |= default_dbs[db].families;
+	f->dbs	    |= 1 << db;
+	do_default   = 0;
+}
+
+static void filter_af_set(struct filter *f, int af)
+{
+	f->dbs	    |= default_afs[af].dbs;
+	f->states   |= default_afs[af].states;
+	f->families |= 1 << af;
+	do_default   = 0;
+}
+
+static int filter_af_get(struct filter *f, int af)
+{
+	return f->families & (1 << af);
+}
+
+static void filter_default_dbs(struct filter *f)
+{
+	filter_db_set(f, UDP_DB);
+	filter_db_set(f, DCCP_DB);
+	filter_db_set(f, TCP_DB);
+	filter_db_set(f, RAW_DB);
+	filter_db_set(f, UNIX_ST_DB);
+	filter_db_set(f, UNIX_DG_DB);
+	filter_db_set(f, UNIX_SQ_DB);
+	filter_db_set(f, PACKET_R_DB);
+	filter_db_set(f, PACKET_DG_DB);
+	filter_db_set(f, NETLINK_DB);
+}
+
+static void filter_merge(struct filter *af, struct filter *dbf, int states)
+{
+	if (af->families)
+		af->families = (af->families | dbf->families) & af->families;
+	else
+		af->families = dbf->families;
+
+	if (dbf->dbs)
+		af->dbs = (af->dbs | dbf->dbs) & dbf->dbs;
+
+	if (dbf->states)
+		af->states = (af->states | dbf->states) & dbf->states;
+
+	if (states)
+		af->states = (af->states | states) & states;
+}
 
 static FILE *generic_proc_open(const char *env, const char *name)
 {
@@ -1172,12 +1287,13 @@ void *parse_hostcond(char *addr)
 	char *port = NULL;
 	struct aafilter a;
 	struct aafilter *res;
-	int fam = preferred_family;
+	int fam = 0;
+	struct filter *f = &current_filter;
 
 	memset(&a, 0, sizeof(a));
 	a.port = -1;
 
-	if (fam == AF_UNIX || strncmp(addr, "unix:", 5) == 0) {
+	if (filter_af_get(f, AF_UNIX) || strncmp(addr, "unix:", 5) == 0) {
 		char *p;
 		a.addr.family = AF_UNIX;
 		if (strncmp(addr, "unix:", 5) == 0)
@@ -1185,10 +1301,11 @@ void *parse_hostcond(char *addr)
 		p = strdup(addr);
 		a.addr.bitlen = 8*strlen(p);
 		memcpy(a.addr.data, &p, sizeof(p));
+		fam = AF_UNIX;
 		goto out;
 	}
 
-	if (fam == AF_PACKET || strncmp(addr, "link:", 5) == 0) {
+	if (filter_af_get(f, AF_PACKET) || strncmp(addr, "link:", 5) == 0) {
 		a.addr.family = AF_PACKET;
 		a.addr.bitlen = 0;
 		if (strncmp(addr, "link:", 5) == 0)
@@ -1210,10 +1327,11 @@ void *parse_hostcond(char *addr)
 				return NULL;
 			a.addr.data[0] = ntohs(tmp);
 		}
+		fam = AF_PACKET;
 		goto out;
 	}
 
-	if (fam == AF_NETLINK || strncmp(addr, "netlink:", 8) == 0) {
+	if (filter_af_get(f, AF_NETLINK) || strncmp(addr, "netlink:", 8) == 0) {
 		a.addr.family = AF_NETLINK;
 		a.addr.bitlen = 0;
 		if (strncmp(addr, "netlink:", 8) == 0)
@@ -1235,13 +1353,14 @@ void *parse_hostcond(char *addr)
 			if (nl_proto_a2n(&a.addr.data[0], addr) == -1)
 				return NULL;
 		}
+		fam = AF_NETLINK;
 		goto out;
 	}
 
-	if (strncmp(addr, "inet:", 5) == 0) {
+	if (filter_af_get(f, AF_INET) || !strncmp(addr, "inet:", 5)) {
 		addr += 5;
 		fam = AF_INET;
-	} else if (strncmp(addr, "inet6:", 6) == 0) {
+	} else if (filter_af_get(f, AF_INET6) || !strncmp(addr, "inet6:", 6)) {
 		addr += 6;
 		fam = AF_INET6;
 	}
@@ -1310,7 +1429,10 @@ void *parse_hostcond(char *addr)
 		}
 	}
 
-	out:
+out:
+	if (fam)
+		filter_af_set(f, fam);
+
 	res = malloc(sizeof(*res));
 	if (res)
 		memcpy(res, &a, sizeof(a));
@@ -2460,6 +2582,9 @@ static int unix_show(struct filter *f)
 	int  cnt;
 	struct unixstat *list = NULL;
 
+	if (!filter_af_get(f, AF_UNIX))
+		return 0;
+
 	if (!getenv("PROC_NET_UNIX") && !getenv("PROC_ROOT")
 	    && unix_show_netlink(f) == 0)
 		return 0;
@@ -2710,7 +2835,7 @@ static int packet_show(struct filter *f)
 {
 	FILE *fp;
 
-	if (preferred_family != AF_PACKET && !(f->states & (1 << SS_CLOSE)))
+	if (!filter_af_get(f, AF_PACKET) || !(f->states & (1 << SS_CLOSE)))
 		return 0;
 
 	if (!getenv("PROC_NET_PACKET") && !getenv("PROC_ROOT") &&
@@ -2878,7 +3003,7 @@ static int netlink_show(struct filter *f)
 	int rq, wq, rc;
 	unsigned long long sk, cb;
 
-	if (preferred_family != AF_NETLINK && !(f->states & (1 << SS_CLOSE)))
+	if (!filter_af_get(f, AF_NETLINK) || !(f->states & (1 << SS_CLOSE)))
 		return 0;
 
 	if (!getenv("PROC_NET_NETLINK") && !getenv("PROC_ROOT") &&
@@ -3141,7 +3266,9 @@ static int scan_state(const char *state)
 		if (strcasecmp(state, sstate_namel[i]) == 0)
 			return (1<<i);
 	}
-	return 0;
+
+	fprintf(stderr, "ss: wrong state name: %s\n", state);
+	exit(-1);
 }
 
 static const struct option long_opts[] = {
@@ -3179,17 +3306,14 @@ static const struct option long_opts[] = {
 
 int main(int argc, char *argv[])
 {
-	int do_default = 1;
 	int saw_states = 0;
 	int saw_query = 0;
 	int do_summary = 0;
 	const char *dump_tcpdiag = NULL;
 	FILE *filter_fp = NULL;
 	int ch;
-
-	memset(&current_filter, 0, sizeof(current_filter));
-
-	current_filter.states = default_filter.states;
+	struct filter dbs_filter = {};
+	int state_filter = 0;
 
 	while ((ch = getopt_long(argc, argv, "dhaletuwxnro460spbf:miA:D:F:vVzZ",
 				 long_opts, NULL)) != EOF) {
@@ -3222,55 +3346,51 @@ int main(int argc, char *argv[])
 			show_bpf++;
 			break;
 		case 'd':
-			current_filter.dbs |= (1<<DCCP_DB);
-			do_default = 0;
+			filter_db_set(&dbs_filter, DCCP_DB);
 			break;
 		case 't':
-			current_filter.dbs |= (1<<TCP_DB);
-			do_default = 0;
+			filter_db_set(&dbs_filter, TCP_DB);
 			break;
 		case 'u':
-			current_filter.dbs |= (1<<UDP_DB);
-			do_default = 0;
+			filter_db_set(&dbs_filter, UDP_DB);
 			break;
 		case 'w':
-			current_filter.dbs |= (1<<RAW_DB);
-			do_default = 0;
+			filter_db_set(&dbs_filter, RAW_DB);
 			break;
 		case 'x':
-			current_filter.dbs |= UNIX_DBM;
-			do_default = 0;
+			filter_af_set(&current_filter, AF_UNIX);
 			break;
 		case 'a':
-			current_filter.states = SS_ALL;
+			state_filter = SS_ALL;
 			break;
 		case 'l':
-			current_filter.states = (1<<SS_LISTEN) | (1<<SS_CLOSE);
+			state_filter = (1 << SS_LISTEN) | (1 << SS_CLOSE);
 			break;
 		case '4':
-			preferred_family = AF_INET;
+			filter_af_set(&current_filter, AF_INET);
 			break;
 		case '6':
-			preferred_family = AF_INET6;
+			filter_af_set(&current_filter, AF_INET6);
 			break;
 		case '0':
-			preferred_family = AF_PACKET;
+			filter_af_set(&current_filter, AF_PACKET);
 			break;
 		case 'f':
 			if (strcmp(optarg, "inet") == 0)
-				preferred_family = AF_INET;
+				filter_af_set(&current_filter, AF_INET);
 			else if (strcmp(optarg, "inet6") == 0)
-				preferred_family = AF_INET6;
+				filter_af_set(&current_filter, AF_INET6);
 			else if (strcmp(optarg, "link") == 0)
-				preferred_family = AF_PACKET;
+				filter_af_set(&current_filter, AF_PACKET);
 			else if (strcmp(optarg, "unix") == 0)
-				preferred_family = AF_UNIX;
+				filter_af_set(&current_filter, AF_UNIX);
 			else if (strcmp(optarg, "netlink") == 0)
-				preferred_family = AF_NETLINK;
+				filter_af_set(&current_filter, AF_NETLINK);
 			else if (strcmp(optarg, "help") == 0)
 				help();
 			else {
-				fprintf(stderr, "ss: \"%s\" is invalid family\n", optarg);
+				fprintf(stderr, "ss: \"%s\" is invalid family\n",
+						optarg);
 				usage();
 			}
 			break;
@@ -3287,38 +3407,44 @@ int main(int argc, char *argv[])
 				if ((p1 = strchr(p, ',')) != NULL)
 					*p1 = 0;
 				if (strcmp(p, "all") == 0) {
-					current_filter.dbs = ALL_DB;
+					filter_default_dbs(&dbs_filter);
 				} else if (strcmp(p, "inet") == 0) {
-					current_filter.dbs |= (1<<TCP_DB)|(1<<DCCP_DB)|(1<<UDP_DB)|(1<<RAW_DB);
+					filter_db_set(&dbs_filter, UDP_DB);
+					filter_db_set(&dbs_filter, DCCP_DB);
+					filter_db_set(&dbs_filter, TCP_DB);
+					filter_db_set(&dbs_filter, RAW_DB);
 				} else if (strcmp(p, "udp") == 0) {
-					current_filter.dbs |= (1<<UDP_DB);
+					filter_db_set(&dbs_filter, UDP_DB);
 				} else if (strcmp(p, "dccp") == 0) {
-					current_filter.dbs |= (1<<DCCP_DB);
+					filter_db_set(&dbs_filter, DCCP_DB);
 				} else if (strcmp(p, "tcp") == 0) {
-					current_filter.dbs |= (1<<TCP_DB);
+					filter_db_set(&dbs_filter, TCP_DB);
 				} else if (strcmp(p, "raw") == 0) {
-					current_filter.dbs |= (1<<RAW_DB);
+					filter_db_set(&dbs_filter, RAW_DB);
 				} else if (strcmp(p, "unix") == 0) {
-					current_filter.dbs |= UNIX_DBM;
+					filter_db_set(&dbs_filter, UNIX_ST_DB);
+					filter_db_set(&dbs_filter, UNIX_DG_DB);
+					filter_db_set(&dbs_filter, UNIX_SQ_DB);
 				} else if (strcasecmp(p, "unix_stream") == 0 ||
 					   strcmp(p, "u_str") == 0) {
-					current_filter.dbs |= (1<<UNIX_ST_DB);
+					filter_db_set(&dbs_filter, UNIX_ST_DB);
 				} else if (strcasecmp(p, "unix_dgram") == 0 ||
 					   strcmp(p, "u_dgr") == 0) {
-					current_filter.dbs |= (1<<UNIX_DG_DB);
+					filter_db_set(&dbs_filter, UNIX_DG_DB);
 				} else if (strcasecmp(p, "unix_seqpacket") == 0 ||
 					   strcmp(p, "u_seq") == 0) {
-					current_filter.dbs |= (1<<UNIX_SQ_DB);
+					filter_db_set(&dbs_filter, UNIX_SQ_DB);
 				} else if (strcmp(p, "packet") == 0) {
-					current_filter.dbs |= PACKET_DBM;
+					filter_db_set(&dbs_filter, PACKET_R_DB);
+					filter_db_set(&dbs_filter, PACKET_DG_DB);
 				} else if (strcmp(p, "packet_raw") == 0 ||
 					   strcmp(p, "p_raw") == 0) {
-					current_filter.dbs |= (1<<PACKET_R_DB);
+					filter_db_set(&dbs_filter, PACKET_R_DB);
 				} else if (strcmp(p, "packet_dgram") == 0 ||
 					   strcmp(p, "p_dgr") == 0) {
-					current_filter.dbs |= (1<<PACKET_DG_DB);
+					filter_db_set(&dbs_filter, PACKET_DG_DB);
 				} else if (strcmp(p, "netlink") == 0) {
-					current_filter.dbs |= (1<<NETLINK_DB);
+					filter_db_set(&dbs_filter, NETLINK_DB);
 				} else {
 					fprintf(stderr, "ss: \"%s\" is illegal socket table id\n", p);
 					usage();
@@ -3380,57 +3506,6 @@ int main(int argc, char *argv[])
 			exit(0);
 	}
 
-	if (do_default)
-		current_filter.dbs = default_filter.dbs;
-
-	if (preferred_family == AF_UNSPEC) {
-		if (!(current_filter.dbs&~UNIX_DBM))
-			preferred_family = AF_UNIX;
-		else if (!(current_filter.dbs&~PACKET_DBM))
-			preferred_family = AF_PACKET;
-		else if (!(current_filter.dbs&~(1<<NETLINK_DB)))
-			preferred_family = AF_NETLINK;
-	}
-
-	if (preferred_family != AF_UNSPEC) {
-		int mask2;
-		if (preferred_family == AF_INET ||
-		    preferred_family == AF_INET6) {
-			mask2= current_filter.dbs;
-		} else if (preferred_family == AF_PACKET) {
-			mask2 = PACKET_DBM;
-		} else if (preferred_family == AF_UNIX) {
-			mask2 = UNIX_DBM;
-		} else if (preferred_family == AF_NETLINK) {
-			mask2 = (1<<NETLINK_DB);
-		} else {
-			mask2 = 0;
-		}
-
-		if (do_default)
-			current_filter.dbs = mask2;
-		else
-			current_filter.dbs &= mask2;
-		current_filter.families = (1<<preferred_family);
-	} else {
-		if (!do_default)
-			current_filter.families = ~0;
-		else
-			current_filter.families = default_filter.families;
-	}
-	if (current_filter.dbs == 0) {
-		fprintf(stderr, "ss: no socket tables to show with such filter.\n");
-		exit(0);
-	}
-	if (current_filter.families == 0) {
-		fprintf(stderr, "ss: no families to show with such filter.\n");
-		exit(0);
-	}
-
-	if (resolve_services && resolve_hosts &&
-	    (current_filter.dbs&(UNIX_DBM|(1<<TCP_DB)|(1<<UDP_DB)|(1<<DCCP_DB))))
-		init_service_resolver();
-
 	/* Now parse filter... */
 	if (argc == 0 && filter_fp) {
 		if (ssfilter_parse(&current_filter.f, 0, NULL, filter_fp))
@@ -3441,15 +3516,15 @@ int main(int argc, char *argv[])
 		if (strcmp(*argv, "state") == 0) {
 			NEXT_ARG();
 			if (!saw_states)
-				current_filter.states = 0;
-			current_filter.states |= scan_state(*argv);
+				state_filter = 0;
+			state_filter |= scan_state(*argv);
 			saw_states = 1;
 		} else if (strcmp(*argv, "exclude") == 0 ||
 			   strcmp(*argv, "excl") == 0) {
 			NEXT_ARG();
 			if (!saw_states)
-				current_filter.states = SS_ALL;
-			current_filter.states &= ~scan_state(*argv);
+				state_filter = SS_ALL;
+			state_filter &= ~scan_state(*argv);
 			saw_states = 1;
 		} else {
 			if (ssfilter_parse(&current_filter.f, argc, argv, filter_fp))
@@ -3459,6 +3534,27 @@ int main(int argc, char *argv[])
 		argc--; argv++;
 	}
 
+	if (do_default) {
+		state_filter = state_filter ? state_filter : SS_CONN;
+		filter_default_dbs(&current_filter);
+		filter_merge(&current_filter, &current_filter, state_filter);
+	} else {
+		filter_merge(&current_filter, &dbs_filter, state_filter);
+	}
+
+	if (resolve_services && resolve_hosts &&
+	    (current_filter.dbs&(UNIX_DBM|(1<<TCP_DB)|(1<<UDP_DB)|(1<<DCCP_DB))))
+		init_service_resolver();
+
+
+	if (current_filter.dbs == 0) {
+		fprintf(stderr, "ss: no socket tables to show with such filter.\n");
+		exit(0);
+	}
+	if (current_filter.families == 0) {
+		fprintf(stderr, "ss: no families to show with such filter.\n");
+		exit(0);
+	}
 	if (current_filter.states == 0) {
 		fprintf(stderr, "ss: no socket states to show with such filter.\n");
 		exit(0);
-- 
2.1.3

^ permalink raw reply related


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