Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v2 1/3] net: ipv6: Allow shorthand delete of all nexthops in multipath route
From: David Ahern @ 2017-01-17  1:27 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, ddutt
In-Reply-To: <20170116.195139.2265183195514595519.davem@davemloft.net>

On 1/16/17 5:51 PM, David Miller wrote:
> From: David Ahern <dsa@cumulusnetworks.com>
> Date: Sun, 15 Jan 2017 12:07:04 -0800
> 
>> @@ -2143,6 +2143,26 @@ int ip6_del_rt(struct rt6_info *rt)
>>  	return __ip6_del_rt(rt, &info);
>>  }
>>  
>> +/* called with table lock held */
>  ...
>> @@ -2176,10 +2196,9 @@ static int ip6_route_del(struct fib6_config *cfg)
>>  				continue;
>>  			if (cfg->fc_protocol && cfg->fc_protocol != rt->rt6i_protocol)
>>  				continue;
>> -			dst_hold(&rt->dst);
>> -			read_unlock_bh(&table->tb6_lock);
>>  
>> -			return __ip6_del_rt(rt, &cfg->fc_nlinfo);
>> +			err = __ip6_route_del(rt, cfg);
>> +			break;
>>  		}
> 
> fib6_del() (invoked by __ip6_route_del()) has to be invoked with the
> table lock held a sa writer, but here you are only holding it as a
> reader.

That table lock is still held. If you look up 2 lines I remove the line that releases the lock.

> 
> I also think some more thought has to be put into whether we can
> change behavior like this without using a flag, as suggested by Roopa.
> 

I'm all for changing the behavior by default, but AIUI that breaks the golden rule.

libnl as of libnl3_2_16-16-g29b71371e764  (a patch by Roopa) already handles both flavors -- if RTA_MULTIPATH is sent it handles it fine and if a series of routes are sent they are consolidated to 1 object with next hops.

iproute2 can handle it just fine as well. I made no changes to get the display format consistent. The problem here is scripts that rely on the old format.

quagga does *not* correctly handle the RTA_MULTIPATH attribute for IPv6 but then it does not properly handle IPv6 multipath routes it receives from the kernel at all. Same result with current kernel code or with this patch set - only the last nexthop is retained. (quagga routes pushed to kernel works, but kernel routes fed to quagga does not)

^ permalink raw reply

* Re: fq_codel and skb->hash
From: Eric Dumazet @ 2017-01-17  1:44 UTC (permalink / raw)
  To: Andrew Collins; +Cc: netdev, edumazet
In-Reply-To: <f9913fca-da16-8620-d237-d7d8d09cced2@cradlepoint.com>

On Mon, 2017-01-16 at 17:04 -0700, Andrew Collins wrote:
> The fq_codel packet scheduler always regenerates the skb flow hash.  Is there any reason
> to do this other than the recent hash perturbation additions?
> 
> I ask because I have a case where an incoming set of TCP flows is encapsulated in a
> single ipsec tunnel, which is then classified on egress into a single flow by fq_codel
> resulting in poor behavior.
> 
> Reusing the skb hash set in receive results in much better behavior, as the value is
> determined pre-encapsulation and flows end up being distributed more naturally across
> codel queues.



> 
> Is opportunistically using the pre-existing skb hash (if available) problematic?

I guess this would be fine.

We never exported the 'perturbation' or allowed to change/set it.

The only 'risk' would be having a buggy device setting a wrong L4
skb->hash, or buggy protocol messing with skb->hash for a given flow.

>   Is there a better way to manage flow separation in routed+encapsulated traffic?

Encapsulated traffic is fine, since flow dissector skips encap header(s)
up to the L4 header.

Problem is about encrypted traffic, since presumably this L4 header is
opaque for flow dissector ;)

^ permalink raw reply

* Re: [PATCH net-next v2 1/3] net: ipv6: Allow shorthand delete of all nexthops in multipath route
From: David Ahern @ 2017-01-17  1:38 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, ddutt
In-Reply-To: <20170116.203717.949572770050738032.davem@davemloft.net>

On 1/16/17 6:37 PM, David Miller wrote:
> Is it clear now?

yes. time for a trip to the eye doctor

^ permalink raw reply

* Re: [PATCH net-next v2 1/3] net: ipv6: Allow shorthand delete of all nexthops in multipath route
From: David Miller @ 2017-01-17  1:37 UTC (permalink / raw)
  To: dsa; +Cc: netdev, ddutt
In-Reply-To: <06d4b3c0-0713-04b8-32d2-99b9b083b825@cumulusnetworks.com>

From: David Ahern <dsa@cumulusnetworks.com>
Date: Mon, 16 Jan 2017 18:27:36 -0700

> On 1/16/17 5:51 PM, David Miller wrote:
>> From: David Ahern <dsa@cumulusnetworks.com>
>> Date: Sun, 15 Jan 2017 12:07:04 -0800
>> 
>>> @@ -2143,6 +2143,26 @@ int ip6_del_rt(struct rt6_info *rt)
>>>  	return __ip6_del_rt(rt, &info);
>>>  }
>>>  
>>> +/* called with table lock held */
>>  ...
>>> @@ -2176,10 +2196,9 @@ static int ip6_route_del(struct fib6_config *cfg)
>>>  				continue;
>>>  			if (cfg->fc_protocol && cfg->fc_protocol != rt->rt6i_protocol)
>>>  				continue;
>>> -			dst_hold(&rt->dst);
>>> -			read_unlock_bh(&table->tb6_lock);
>>>  
>>> -			return __ip6_del_rt(rt, &cfg->fc_nlinfo);
>>> +			err = __ip6_route_del(rt, cfg);
>>> +			break;
>>>  		}
>> 
>> fib6_del() (invoked by __ip6_route_del()) has to be invoked with the
>> table lock held a sa writer, but here you are only holding it as a
>> reader.
> 
> That table lock is still held. If you look up 2 lines I remove the line that releases the lock.

It's held in this function as a reader, it needs to be held as a writer.

That's why the lock is dropped in the current code and the existing
wrapper around fib6_del() takes it as a writer.

Is it clear now?

	read_lock_bh(&table->lock);
	fib6_del();

is invalid.

	write_lock_bh(&table->lock);
	fib6_del();

is required.

^ permalink raw reply

* Re: [PATCH v2 net-next 0/2] mvneta xmit_more and bql support
From: David Miller @ 2017-01-17  1:13 UTC (permalink / raw)
  To: mw
  Cc: thomas.petazzoni, andrew, linux, jason, netdev, linux-kernel, jaz,
	gregory.clement, linux-arm-kernel, sebastian.hesselbarth
In-Reply-To: <1484586512-16412-1-git-send-email-mw@semihalf.com>

From: Marcin Wojtas <mw@semihalf.com>
Date: Mon, 16 Jan 2017 18:08:30 +0100

> This is a delayed v2 of short patchset, which introduces xmit_more and BQL
> to mvneta driver. The only one change was added in xmit_more support -
> condition check preventing excessive descriptors concatenation before
> flushing in HW.
> 
> Any comments or feedback would be welcome.
 ...
> Changelog:
> v1 -> v2:
> 
> * Add checking condition that ensures too much descriptors are not
>   concatenated before flushing in HW.

Looks good, series applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next v2 1/3] net: ipv6: Allow shorthand delete of all nexthops in multipath route
From: David Miller @ 2017-01-17  0:51 UTC (permalink / raw)
  To: dsa; +Cc: netdev, ddutt
In-Reply-To: <1484510826-2723-2-git-send-email-dsa@cumulusnetworks.com>

From: David Ahern <dsa@cumulusnetworks.com>
Date: Sun, 15 Jan 2017 12:07:04 -0800

> @@ -2143,6 +2143,26 @@ int ip6_del_rt(struct rt6_info *rt)
>  	return __ip6_del_rt(rt, &info);
>  }
>  
> +/* called with table lock held */
 ...
> @@ -2176,10 +2196,9 @@ static int ip6_route_del(struct fib6_config *cfg)
>  				continue;
>  			if (cfg->fc_protocol && cfg->fc_protocol != rt->rt6i_protocol)
>  				continue;
> -			dst_hold(&rt->dst);
> -			read_unlock_bh(&table->tb6_lock);
>  
> -			return __ip6_del_rt(rt, &cfg->fc_nlinfo);
> +			err = __ip6_route_del(rt, cfg);
> +			break;
>  		}

fib6_del() (invoked by __ip6_route_del()) has to be invoked with the
table lock held a sa writer, but here you are only holding it as a
reader.

I also think some more thought has to be put into whether we can
change behavior like this without using a flag, as suggested by Roopa.

^ permalink raw reply

* Re: [PATCH net 1/1] net sched actions: fix refcnt when GETing of action after bind
From: David Miller @ 2017-01-17  0:46 UTC (permalink / raw)
  To: jhs; +Cc: netdev, xiyou.wangcong
In-Reply-To: <1484493246-10911-1-git-send-email-jhs@emojatatu.com>

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Sun, 15 Jan 2017 10:14:06 -0500

> From: Jamal Hadi Salim <jhs@mojatatu.com>
 ...
> Fixes: aecc5cefc389 ("net sched actions: fix GETing actions")
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Applied and queued up for -stable, thanks Jamal.

^ permalink raw reply

* RE: [PATCH] net: add regs attribute to phy device for user diagnose
From: YUAN Linyu @ 2017-01-17  0:11 UTC (permalink / raw)
  To: David Miller, f.fainelli@gmail.com
  Cc: cugyly@163.com, andrew@lunn.ch, netdev@vger.kernel.org
In-Reply-To: <20170116.165401.1642553705400599921.davem@davemloft.net>



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, January 17, 2017 5:54 AM
> To: f.fainelli@gmail.com
> Cc: cugyly@163.com; andrew@lunn.ch; netdev@vger.kernel.org; YUAN Linyu
> Subject: Re: [PATCH] net: add regs attribute to phy device for user diagnose
> 
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Mon, 16 Jan 2017 12:22:16 -0800
> 
> >
> > So why not add support in ethtool for reading PHY registers if you need
> > it? There are handful of PHY "things" in ethtool, such as reading
> > counters, configuring downshift etc., adding support for dumping
> > registers does not sound out of space.
Thanks, I will try this direction.
> 
> Agreed.

^ permalink raw reply

* fq_codel and skb->hash
From: Andrew Collins @ 2017-01-17  0:04 UTC (permalink / raw)
  To: netdev; +Cc: edumazet

The fq_codel packet scheduler always regenerates the skb flow hash.  Is there any reason
to do this other than the recent hash perturbation additions?

I ask because I have a case where an incoming set of TCP flows is encapsulated in a
single ipsec tunnel, which is then classified on egress into a single flow by fq_codel
resulting in poor behavior.

Reusing the skb hash set in receive results in much better behavior, as the value is
determined pre-encapsulation and flows end up being distributed more naturally across
codel queues.

Is opportunistically using the pre-existing skb hash (if available) problematic?  Is
there a better way to manage flow separation in routed+encapsulated traffic?

Thanks,
Andrew Collins

^ permalink raw reply

* [PATCH 1/1] ip: fix igmp parsing when iface is long
From: Petr Vorel @ 2017-01-16 23:25 UTC (permalink / raw)
  To: netdev; +Cc: Petr Vorel

Entries with long vhost names in /proc/net/igmp have no whitespace
between name and colon, so sscanf() adds it to vhost and
'ip maddr show iface' doesn't include inet result.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 ip/ipmaddr.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ip/ipmaddr.c b/ip/ipmaddr.c
index 22eb407..4f726fd 100644
--- a/ip/ipmaddr.c
+++ b/ip/ipmaddr.c
@@ -136,13 +136,17 @@ static void read_igmp(struct ma_info **result_p)
 
 	while (fgets(buf, sizeof(buf), fp)) {
 		struct ma_info *ma;
+		size_t len;
 
 		if (buf[0] != '\t') {
 			sscanf(buf, "%d%s", &m.index, m.name);
+			len = strlen(m.name);
+			if (m.name[len - 1] == ':')
+				len--;
 			continue;
 		}
 
-		if (filter.dev && strcmp(filter.dev, m.name))
+		if (filter.dev && strncmp(filter.dev, m.name, len))
 			continue;
 
 		sscanf(buf, "%08x%d", (__u32 *)&m.addr.data, &m.users);
-- 
2.11.0

^ permalink raw reply related

* [PATCH net] vxlan: fix byte order of vxlan-gpe port number
From: Lance Richardson @ 2017-01-16 23:37 UTC (permalink / raw)
  To: netdev, jbenc

vxlan->cfg.dst_port is in network byte order, so an htons()
is needed here. Also reduced comment length to stay closer
to 80 column width (still slightly over, however).

Fixes: e1e5314de08b ("vxlan: implement GPE")
Signed-off-by: Lance Richardson <lrichard@redhat.com>
---
 drivers/net/vxlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index ca7196c..8a79cfc 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2890,7 +2890,7 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
 	memcpy(&vxlan->cfg, conf, sizeof(*conf));
 	if (!vxlan->cfg.dst_port) {
 		if (conf->flags & VXLAN_F_GPE)
-			vxlan->cfg.dst_port = 4790; /* IANA assigned VXLAN-GPE port */
+			vxlan->cfg.dst_port = htons(4790); /* IANA VXLAN-GPE port */
 		else
 			vxlan->cfg.dst_port = default_port;
 	}
-- 
2.5.5

^ permalink raw reply related

* Re: [pull request][for-next] Mellanox mlx5 Reorganize core driver directory layout
From: Saeed Mahameed @ 2017-01-16 23:36 UTC (permalink / raw)
  To: Tom Herbert
  Cc: David Miller, Saeed Mahameed, Doug Ledford, Linux Netdev List,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky
In-Reply-To: <CALx6S377hzH8_DbUSKafkbCzMEunr0H7vrfcb2WfEWFHFnfUpw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Mon, Jan 16, 2017 at 11:06 PM, Tom Herbert <tom-BjP2VixgY4xUbtYUoyoikg@public.gmane.org> wrote:
> On Mon, Jan 16, 2017 at 12:30 PM, Saeed Mahameed
> <saeedm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
>> On Sat, Jan 14, 2017 at 12:07 AM, Saeed Mahameed
>> <saeedm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
>>> On Fri, Jan 13, 2017 at 7:14 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
>>>> From: Saeed Mahameed <saeedm-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>> Date: Thu, 12 Jan 2017 19:22:34 +0200
>>>>
>>>>> This pull request includes one patch from Leon, this patch as described
>>>>> below will change the driver directory structure and layout for better,
>>>>> logical and modular driver files separation.
>>>>>
>>>>> This change is important to both rdma and net maintainers in order to
>>>>> have smoother management of driver patches for different mlx5 sub modules
>>>>> and smoother rdma-next vs. net-next features submissions.
>>>>>
>>>>> Please find more info below -in the tag commit message-,
>>>>> review and let us know if there's any problem.
>>>>>
>>>>> This change doesn't introduce any conflicts with the current mlx5
>>>>> fixes and cleanups posted on 2017-01-10 to net branch, and merge tests
>>>>> worked flawlessly with no issues.
>>>>>
>>>>> This is the last pull request meant for both rdma-next and net-next.
>>>>> Once pulled, this will be the base shared code for both trees.
>>>>
>>>> This is pretty crazy, it will make all bug fix backporting to -stable
>>>> a complete nightmare for myself, Doug, various distribution maintainers
>>>> and many other people who quietly have to maintain their own trees and
>>>> do backporting.
>>>>
>>>
>>> I hear you,
>>> But please bear with me here, what if we queue this patch up to -stable ? and we
>>> (Mellanox) and specifically our dedicated inbox team, will make sure
>>> that this patch
>>> will land on the various distributions and for those maintaining their
>>> own trees.
>>> This patch is really straight forward (rename files) and I already
>>> tried to cherry-pick it
>>> on older kernels, I only got a couple of conflicts on some of the
>>> "#inlcude" lines we've
>>> changed, and they are pretty straightforward to fix, we can even avoid
>>> this if we decide to
>>> not move mlx5 header files in this phase.
>>>
>>> If this is possible then all trees will be aligned and it will be a
>>> win win situation.
>>>
>>
>> Hi Dave,
>>
>> Any chance you saw my -stable suggestion above ?
>> I think it would really close the backporting gap.
>>
>> Sorry i am bothering you with this topic, but we really desire the new
>> structure and
>> I never got your feedback on this suggestion, so i would like to hear
>> your thoughts.
>>
> Saeed,
>
> I've already you specific suggestions on your new structure, please
> consider your reviewers feedback more carefully. Again, the starting

I know, sorry i didn't respond on time, I though Leno's response was sufficient.

> point for your restructuring should be to separate out the minimum set
> of files required to build reasonable driver and then cleanly
> compartmentalize the rest of the features to make it easy for your
> users to include or not include those in their build. Unless you've

I see your point and i am 100% with you on this, we are all on the
same page here,
we all want to achieve the same goal -modular break down and features
logic separation-.

And in order to achieve this we need to do some big steps.
some of them will have real benefits and others are just moving code around.

One thing I need to point out (that Leon already mentioned) that the
current driver files sitting flat
in mlx5/core directory can be categorized to 4 categories
1. core related & init flows
2. common (infinband/ethernet resource management API)
3. infiniband related logic and FW commands
4. ethernet related logic and FW commands

so we figured that first we should split the bigger chunks (ethernet
vs. inifniband) into their own directories.
then i can follow up with ethernet features break down and kconfig separation.

> done this I'm not seeing much benefit for this restructuring. Also, I
> would rather see this done in one shot then expecting some sort of
> evolution over time to the right solution-- as Dave said the
> complexity of this driver is to far down the road to do that.
>

Well, I also prefer to do this in one shot, but I don't want to start
wasting time doing such a huge change
then get a NACK on submission.

Let's assume we did this in one shot i.e:
1. restructure  files and directories
2. re-arrange code (separate logic)
3. Kconfig features control - to show the immediate benefit of this change

If this is acceptable by you and we get Dave's blessing on this then i
would be happy to provide (as this is our
long term plan), but if Dave doesn't approve, then ... I don't really know.

Also for 1. backporting is really easy since such patch can be applied
easily to any kernel release back to 4.4.
but 2. and 3. are really tricky ones.

> Tom
>
>> Thanks,
>> Saeed.
>>
>>>> I really don't think you can justify this rearrangement based upon the
>>>> consequences and how much activity happens in this driver.
>>>>
>>>
>>> Right, but this is not the only justification, I can sum it up to that
>>> we would like
>>> to lay out the foundation for many years to come for a well designed
>>> driver with a modular
>>> sub modules break down and scalable infrastructure. We already plan to
>>> submit more mlx5
>>> independent  sub modules - just like mlx5e (en_*) files and mlx5_ib
>>> driver- so this was also
>>> a reason for us to consider this re-engagement at this stage.
>>>
>>>> You should have thought long and hard about the layout a long time ago
>>>> rather than after the driver has been in the tree for many years.
>>>>
>>>
>>> I had this Idea for the separation before the mlx5 Ethernet
>>> submission, but I wasn't the maintainer
>>> back then, and i have been itching to submit such patch for long as
>>> well, still i don't think
>>> it is too late, We (Me and Leon) will keep maintaining this driver for
>>> only god knows how many years to come,
>>> and the mlx5 drivers are meant to serve at least 3-4 more future HW generations.
>>>
>>> Long story short, We would like to re-arrange the driver in a way that
>>> would serve us (the maintainers) and serve those
>>> who are going do develop the future Stack features and the future HW
>>> generations over the well designed (Hopefully)
>>> mlx5 infrastructure.
>>> Keeping it as it is today, will only make the situation worst in the
>>> future and it will be really hard to avoid having a spaghetti code
>>> in the mlx5 driver. All i want to point out here is that maintaining
>>> such a flat subtree is also nightmare.
>>>
>>> So i am only asking you to reconsider this change and give my -stable
>>> suggestion a thought.
>>>
>>> Thank you.
>>> Saeed.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH net-next] bridge: sparse fixes in br_ip6_multicast_alloc_query()
From: Lance Richardson @ 2017-01-16 23:11 UTC (permalink / raw)
  To: netdev

Changed type of csum field in struct igmpv3_query from __be16 to
__sum16 to eliminate type warning, made same change in struct
igmpv3_report for consistency.

Fixed up an ntohs() where htons() should have been used instead.

Signed-off-by: Lance Richardson <lrichard@redhat.com>
---
 include/uapi/linux/igmp.h | 4 ++--
 net/bridge/br_multicast.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/igmp.h b/include/uapi/linux/igmp.h
index ccbb32a..a97f9a7 100644
--- a/include/uapi/linux/igmp.h
+++ b/include/uapi/linux/igmp.h
@@ -53,7 +53,7 @@ struct igmpv3_grec {
 struct igmpv3_report {
 	__u8 type;
 	__u8 resv1;
-	__be16 csum;
+	__sum16 csum;
 	__be16 resv2;
 	__be16 ngrec;
 	struct igmpv3_grec grec[0];
@@ -62,7 +62,7 @@ struct igmpv3_report {
 struct igmpv3_query {
 	__u8 type;
 	__u8 code;
-	__be16 csum;
+	__sum16 csum;
 	__be32 group;
 #if defined(__LITTLE_ENDIAN_BITFIELD)
 	__u8 qrv:3,
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index b30e77e..f6634612 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -540,7 +540,7 @@ static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge *br,
 		break;
 	case 2:
 		mld2q = (struct mld2_query *)icmp6_hdr(skb);
-		mld2q->mld2q_mrc = ntohs((u16)jiffies_to_msecs(interval));
+		mld2q->mld2q_mrc = htons((u16)jiffies_to_msecs(interval));
 		mld2q->mld2q_type = ICMPV6_MGM_QUERY;
 		mld2q->mld2q_code = 0;
 		mld2q->mld2q_cksum = 0;
-- 
2.5.5

^ permalink raw reply related

* RE: [PATCH net-next 6/9] net/mlx4_en: Adding support of turning off link autonegotiation via ethtool
From: Ariel Levkovich @ 2017-01-16 23:08 UTC (permalink / raw)
  To: Or Gerlitz, Tariq Toukan
  Cc: David S. Miller, Linux Netdev List, Eran Ben Elisha
In-Reply-To: <CAJ3xEMjEPQv=Agz5Wr-2UEL+iv7nz4NAcpCKY9=C4vs6SfKLLA@mail.gmail.com>

+       MLX4_PTYS_AN_DISABLE_CAP   = 1 << 5,

It's there.

Best Regards,

Ariel Levkovich
Staff Engineer, SW
Mellanox Technologies
Beit Mellanox, Yokneam, P.O.Box 586, Israel 20692 
Office: +972-74723-7652, Mobile: +972-52-3947-704 
www.mellanox.com 
Follow us on Facebook and Twitter

> -----Original Message-----
> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> Sent: Monday, January 16, 2017 4:48 PM
> To: Tariq Toukan <tariqt@mellanox.com>; Ariel Levkovich
> <lariel@mellanox.com>
> Cc: David S. Miller <davem@davemloft.net>; Linux Netdev List
> <netdev@vger.kernel.org>; Eran Ben Elisha <eranbe@mellanox.com>
> Subject: Re: [PATCH net-next 6/9] net/mlx4_en: Adding support of turning
> off link autonegotiation via ethtool
> 
> On Mon, Jan 16, 2017 at 7:30 PM, Tariq Toukan <tariqt@mellanox.com>
> wrote:
> 0644
> > --- a/include/linux/mlx4/device.h
> > +++ b/include/linux/mlx4/device.h
> > @@ -1539,8 +1539,13 @@ enum mlx4_ptys_proto {
> >         MLX4_PTYS_EN = 1<<2,
> >  };
> >
> > +enum mlx4_ptys_flags {
> > +       MLX4_PTYS_AN_DISABLE_CAP   = 1 << 5,
> > +       MLX4_PTYS_AN_DISABLE_ADMIN = 1 << 6, };
> > +
> >  struct mlx4_ptys_reg {
> > -       u8 resrvd1;
> > +       u8 flags;
> >         u8 local_port;
> >         u8 resrvd2;
> >         u8 proto_mask;
> 
> Hi Ariel,
> 
> I didn't see any new dev cap bit, what happens with FW version which don't
> support this, is the result well defined? what is it?
> 
> Or.

^ permalink raw reply

* Re: [PATCH net-next 3/9] net/mlx4_core: Set EQ affinity hint to local NUMA CPUs
From: Daniel Jurgens @ 2017-01-16 22:21 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Tariq Toukan, David S. Miller, Linux Netdev List, Eran Ben Elisha
In-Reply-To: <CAJ3xEMjGv8xm7vOgASd0CvpS9uQFLv8tT_n7aqsuoB7eTUAZUg@mail.gmail.com>

On 1/16/2017 3:59 PM, Or Gerlitz wrote:
> On Mon, Jan 16, 2017 at 11:54 PM, Daniel Jurgens <danielj@mellanox.com> wrote:
>> On 1/16/2017 3:44 PM, Or Gerlitz wrote:
>>> On Mon, Jan 16, 2017 at 7:29 PM, Tariq Toukan <tariqt@mellanox.com> wrote:
>>>> From: Daniel Jurgens <danielj@mellanox.com>
>>>>
>>>> Use CPUs on the close NUMA when setting the EQ affinity hints.
>>> Dan, are we sure there are no down-sides for always doing this? this
>>> code is probably there for many years and we're introducing here new
>>> behaviour to potentially to many Ms installs when they get distro
>>> update that includes this patch.
>
>> I don't see a downside, this just favors using the node local CPUs before others.
> OK, so this just favors before others and not limits (not in front of
> the code now)? would be good to improve the change log and make this
> clear.
Right, doesn't limit.  Just favors the local node.  I can work with Tariq to update the commit message if you think it necessary.

^ permalink raw reply

* Re: [patch net-next] stmmac: indent an if statement
From: Julia Lawall @ 2017-01-16 22:10 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David Miller, peppe.cavallaro, Joao.Pinto, alexandre.torgue,
	netdev, linux-kernel, kernel-janitors
In-Reply-To: <20170116215615.GG4104@mwanda>



On Tue, 17 Jan 2017, Dan Carpenter wrote:

> On Mon, Jan 16, 2017 at 10:46:22PM +0100, Julia Lawall wrote:
> >
> >
> > On Mon, 16 Jan 2017, Dan Carpenter wrote:
> >
> > > On Mon, Jan 16, 2017 at 12:19:24PM +0300, Dan Carpenter wrote:
> > > > On Sun, Jan 15, 2017 at 10:14:38PM -0500, David Miller wrote:
> > > > > From: Dan Carpenter <dan.carpenter@oracle.com>
> > > > > Date: Thu, 12 Jan 2017 21:46:32 +0300
> > > > >
> > > > > > The break statement should be indented one more tab.
> > > > > >
> > > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > >
> > > > > Applied, but like Julia I think we might have a missing of_node_put()
> > > > > here.
> > > >
> > > > Of course, sorry for dropping the ball on this.  I'll send a patch for
> > > > that.
> > > >
> > >
> > > Actually, I've looked at it some more and I think this function is OK.
> > > We're supposed to do an of_node_put() later...  I can't find where that
> > > happens, but presumably that's because I don't know stmmac well.  This
> > > code here, though, is fine.
> >
> > Why do you think it is fine?  Does anyone in the calling context know
> > which child would have caused the break?
>
> Yeah.  It's saved in plat->mdio_node and we expect to be holding on
> either path through the function.
>
> (It would be better if one of the stmmac people were responding here
> insead of a random fix the indenting weenie like myself.)

OK, I agree that there should not be an of_node_put with the break.

Perhaps there should be an of_node_put on plat->mdio_node in
stmmac_remove_config_dt, like there is an of_node_put on plat->phy_node.
But it would certainly be helpful to hear from someone who knows the code
better.

julia

^ permalink raw reply

* Re: [patch net-next] stmmac: indent an if statement
From: David Miller @ 2017-01-16 22:00 UTC (permalink / raw)
  To: dan.carpenter
  Cc: julia.lawall, peppe.cavallaro, Joao.Pinto, alexandre.torgue,
	netdev, linux-kernel, kernel-janitors
In-Reply-To: <20170116215615.GG4104@mwanda>

From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Tue, 17 Jan 2017 00:56:15 +0300

> (It would be better if one of the stmmac people were responding here
> insead of a random fix the indenting weenie like myself.)

They are all too busy trying to rename the driver, because that's so
much more important.

^ permalink raw reply

* Re: [PATCH net-next 3/9] net/mlx4_core: Set EQ affinity hint to local NUMA CPUs
From: Or Gerlitz @ 2017-01-16 21:59 UTC (permalink / raw)
  To: Daniel Jurgens
  Cc: Tariq Toukan, David S. Miller, Linux Netdev List, Eran Ben Elisha
In-Reply-To: <VI1PR0501MB2429F11A34431293F2D0C327C47D0@VI1PR0501MB2429.eurprd05.prod.outlook.com>

On Mon, Jan 16, 2017 at 11:54 PM, Daniel Jurgens <danielj@mellanox.com> wrote:
> On 1/16/2017 3:44 PM, Or Gerlitz wrote:
>> On Mon, Jan 16, 2017 at 7:29 PM, Tariq Toukan <tariqt@mellanox.com> wrote:
>>> From: Daniel Jurgens <danielj@mellanox.com>
>>>
>>> Use CPUs on the close NUMA when setting the EQ affinity hints.
>> Dan, are we sure there are no down-sides for always doing this? this
>> code is probably there for many years and we're introducing here new
>> behaviour to potentially to many Ms installs when they get distro
>> update that includes this patch.


> I don't see a downside, this just favors using the node local CPUs before others.

OK, so this just favors before others and not limits (not in front of
the code now)? would be good to improve the change log and make this
clear.

> I don't understand your 2nd sentence there.  "Ms installs"?

Millions of installs that run Linux driver.

^ permalink raw reply

* Re: [PATCH net-next 3/9] net/mlx4_core: Set EQ affinity hint to local NUMA CPUs
From: David Miller @ 2017-01-16 21:57 UTC (permalink / raw)
  To: danielj; +Cc: gerlitz.or, tariqt, netdev, eranbe
In-Reply-To: <VI1PR0501MB2429F11A34431293F2D0C327C47D0@VI1PR0501MB2429.eurprd05.prod.outlook.com>

From: Daniel Jurgens <danielj@mellanox.com>
Date: Mon, 16 Jan 2017 21:54:36 +0000

> I don't understand your 2nd sentence there.  "Ms installs"?

I think he meant "Mellanox's installs", meaning installations of Linux
using Mellanox hardware.  At least that's how I read it.

^ permalink raw reply

* Re: [PATCH 1/2] qed: Add support for hardware offloaded FCoE.
From: David Miller @ 2017-01-16 21:56 UTC (permalink / raw)
  To: chad.dupuis
  Cc: martin.petersen, linux-scsi, fcoe-devel, netdev, yuval.mintz,
	QLogic-Storage-Upstream
In-Reply-To: <alpine.OSX.2.00.1701161633290.2060@administrators-macbook-pro.local>

From: Chad Dupuis <chad.dupuis@cavium.com>
Date: Mon, 16 Jan 2017 16:47:52 -0500

> I forgot to add netdev-next to the subject line.  Is a repost needed
> here?

Not this time, no.

^ permalink raw reply

* Re: [patch net-next] stmmac: indent an if statement
From: Dan Carpenter @ 2017-01-16 21:56 UTC (permalink / raw)
  To: Julia Lawall
  Cc: David Miller, peppe.cavallaro, Joao.Pinto, alexandre.torgue,
	netdev, linux-kernel, kernel-janitors
In-Reply-To: <alpine.DEB.2.20.1701162240100.2031@hadrien>

On Mon, Jan 16, 2017 at 10:46:22PM +0100, Julia Lawall wrote:
> 
> 
> On Mon, 16 Jan 2017, Dan Carpenter wrote:
> 
> > On Mon, Jan 16, 2017 at 12:19:24PM +0300, Dan Carpenter wrote:
> > > On Sun, Jan 15, 2017 at 10:14:38PM -0500, David Miller wrote:
> > > > From: Dan Carpenter <dan.carpenter@oracle.com>
> > > > Date: Thu, 12 Jan 2017 21:46:32 +0300
> > > >
> > > > > The break statement should be indented one more tab.
> > > > >
> > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > >
> > > > Applied, but like Julia I think we might have a missing of_node_put()
> > > > here.
> > >
> > > Of course, sorry for dropping the ball on this.  I'll send a patch for
> > > that.
> > >
> >
> > Actually, I've looked at it some more and I think this function is OK.
> > We're supposed to do an of_node_put() later...  I can't find where that
> > happens, but presumably that's because I don't know stmmac well.  This
> > code here, though, is fine.
> 
> Why do you think it is fine?  Does anyone in the calling context know
> which child would have caused the break?

Yeah.  It's saved in plat->mdio_node and we expect to be holding on
either path through the function.

(It would be better if one of the stmmac people were responding here
insead of a random fix the indenting weenie like myself.)

regards,
dan caprenter


^ permalink raw reply

* Re: [PATCH V2] audit: log 32-bit socketcalls
From: David Miller @ 2017-01-16 21:55 UTC (permalink / raw)
  To: paul; +Cc: rgb, netdev, linux-kernel, linux-audit
In-Reply-To: <CAHC9VhQwVOQ+-iZ1iC-efVvCUadMCdRJS60wqgUQWgExL51yEw@mail.gmail.com>

From: Paul Moore <paul@paul-moore.com>
Date: Mon, 16 Jan 2017 15:38:33 -0500

> David, assuming Richard makes your requested changes, any objection if
> I merge this via the audit tree instead of the netdev tree?  It's a
> bit easier for us from a testing perspective this way ...

No objection.

^ permalink raw reply

* Re: [net-next 0/3] tipc: improve interaction socket-link
From: David Miller @ 2017-01-16 21:54 UTC (permalink / raw)
  To: jon.maloy
  Cc: netdev, viro, parthasarathy.bhuvaragan, ying.xue, maloy,
	tipc-discussion
In-Reply-To: <A2BAEFC30C8FD34388F02C9B3121859D225F44DE@eusaamb103.ericsson.se>

From: Jon Maloy <jon.maloy@ericsson.com>
Date: Mon, 16 Jan 2017 20:35:08 +0000

> 
> 
>> -----Original Message-----
>> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>> On Behalf Of David Miller
>> Sent: Monday, 16 January, 2017 14:45
>> To: Jon Maloy <jon.maloy@ericsson.com>
>> Cc: netdev@vger.kernel.org; viro@zeniv.linux.org.uk; Parthasarathy Bhuvaragan
>> <parthasarathy.bhuvaragan@ericsson.com>; Ying Xue
>> <ying.xue@windriver.com>; maloy@donjonn.com; tipc-
>> discussion@lists.sourceforge.net
>> Subject: Re: [net-next 0/3] tipc: improve interaction socket-link
>> 
>> From: Jon Maloy <jon.maloy@ericsson.com>
>> Date: Tue,  3 Jan 2017 10:26:45 -0500
>> 
>> > We fix a very real starvation problem that may occur when a link
>> > encounters send buffer congestion. At the same time we make the
>> > interaction between the socket and link layer simpler and more
>> > consistent.
>> 
>> This doesn't apply to net-next, also the Date in your emails is 10 days
>> in the past.  What's going on here?
> 
> I don't know. This series was sent in on Jan 3rd, and applied by you the same day. Maybe the mail server decided to send you a duplicate?

If you look in the headers, they have "Received-by: " fields with
January 13th in the date.  So something pushed them out again.

Really strange...

^ permalink raw reply

* Re: [PATCH net-next 3/9] net/mlx4_core: Set EQ affinity hint to local NUMA CPUs
From: Daniel Jurgens @ 2017-01-16 21:54 UTC (permalink / raw)
  To: Or Gerlitz, Tariq Toukan
  Cc: David S. Miller, Linux Netdev List, Eran Ben Elisha
In-Reply-To: <CAJ3xEMiGQHV1Qc0aW43SJaNO7X5TRLdYRaZgEiA6099vbBAAXw@mail.gmail.com>

On 1/16/2017 3:44 PM, Or Gerlitz wrote:
> On Mon, Jan 16, 2017 at 7:29 PM, Tariq Toukan <tariqt@mellanox.com> wrote:
>> From: Daniel Jurgens <danielj@mellanox.com>
>>
>> Use CPUs on the close NUMA when setting the EQ affinity hints.
> Dan, are we sure there are no down-sides for always doing this? this
> code is probably there for many years and we're introducing here new
> behaviour to potentially to many Ms installs when they get distro
> update that includes this patch.
>
I don't see a downside, this just favors using the node local CPUs before others.  I don't understand your 2nd sentence there.  "Ms installs"?


^ permalink raw reply

* Re: [PATCH] net: add regs attribute to phy device for user diagnose
From: David Miller @ 2017-01-16 21:54 UTC (permalink / raw)
  To: f.fainelli; +Cc: cugyly, andrew, netdev, Linyu.Yuan
In-Reply-To: <cf3e3125-582f-8b91-9c23-05dbdd2c6d24@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon, 16 Jan 2017 12:22:16 -0800

> On 01/16/2017 04:59 AM, yuan linyu wrote:
>> On 日, 2017-01-15 at 18:21 +0100, Andrew Lunn wrote:
>>> On Sun, Jan 15, 2017 at 09:51:03AM +0800, yuan linyu wrote:
>>>>  
>>>> I hope user/developer can read this attribute file "regs" to do
>>>> a full check of all registers value, and they can write any register
>>>> inside PHY through this file.
>>> Since this is intended for debug, it should not be sysfs, but debugfs.
>> agree,
>>> However, in general, Linux does not allow user space to peek and poke
>>> device registers. Can you point me at examples where i can do the same
>>> to my GPU? SATA controller? Ethernet controller, I2C temperature
>>> sensor? Any device?
>> we can read registers of ethernet controller(memory register accessed) through devmem or ethtool
> 
> So why not add support in ethtool for reading PHY registers if you need
> it? There are handful of PHY "things" in ethtool, such as reading
> counters, configuring downshift etc., adding support for dumping
> registers does not sound out of space.

Agreed.

^ permalink raw reply


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