* [PATCH net-next] sch_htb: do not report fake rate estimators
From: Eric Dumazet @ 2016-10-21 16:02 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Jiri Kosina, Jamal Hadi Salim
From: Eric Dumazet <edumazet@google.com>
When I prepared commit d250a5f90e53 ("pkt_sched: gen_estimator: Dont
report fake rate estimators"), htb still had an implicit rate estimator
for all its classes.
Then later, I made this rate estimator optional in commit 64153ce0a7b6
("net_sched: htb: do not setup default rate estimators"), but I forgot
to update htb use of gnet_stats_copy_rate_est()
After this patch, "tc -s qdisc ..." no longer report fake rate
estimators for HTB classes.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/sched/sch_htb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index c798d0de8a9daf067cae2485ad6bfeeac039efb0..9926fe4f3b6f7db9aeba22fbbfae3d47c4e2af60 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1145,7 +1145,7 @@ htb_dump_class_stats(struct Qdisc *sch, unsigned long arg, struct gnet_dump *d)
if (gnet_stats_copy_basic(qdisc_root_sleeping_running(sch),
d, NULL, &cl->bstats) < 0 ||
- gnet_stats_copy_rate_est(d, NULL, &cl->rate_est) < 0 ||
+ gnet_stats_copy_rate_est(d, &cl->bstats, &cl->rate_est) < 0 ||
gnet_stats_copy_queue(d, NULL, &qs, qlen) < 0)
return -1;
^ permalink raw reply related
* Re: [RFC PATCH 08/13] dwmac-meson8b: add support for phy selection
From: Neil Armstrong @ 2016-10-21 15:59 UTC (permalink / raw)
To: Andrew Lunn
Cc: khilman, carlo, linus.walleij, linux-gpio, devicetree,
linux-arm-kernel, linux-amlogic, linux-kernel, netdev
In-Reply-To: <20161021155408.GR16974@lunn.ch>
On 10/21/2016 05:54 PM, Andrew Lunn wrote:
> On Fri, Oct 21, 2016 at 04:40:33PM +0200, Neil Armstrong wrote:
>> The Meson GXL dwmac Glue Layer also provides switching between an external PHY
>> and an internal RMII 10/100 PHY.
>> Add a way to setup the correct PHY switching from a device tree attribute.
>
> Humm, actually. Maybe PHY_IS_INTERNAL in the phydrv->flags is the
> better solution. The MAC can then use phy_is_internal(ndev->phydev) to
> decide if this register needs programming.
>
> Andrew
>
Hi Andrew,
Yes this would be a good idea if we were able to scan the internal and external PHYs at the same time,
but with our limited knowledge the values we write in the register seems to switch a mux for the whole RMII
and MDIO signals to either interface.
Do you have knowledge if this case is already handled upstream ?
Thanks for your help,
Neil
^ permalink raw reply
* Re: [PATCH net] net: remove MTU limits on a few ether_setup callers
From: Andrew Lunn @ 2016-10-21 15:59 UTC (permalink / raw)
To: Jarod Wilson
Cc: Florian Fainelli, linux-kernel, netdev, Asbjoern Sloth Toennesen,
R Parameswaran, Vivien Didelot
In-Reply-To: <20161021154851.GE11396@redhat.com>
> I'm all for fine-tuning things to be more correct, but my initial
> approach here was to restore the ranges that were previously there,
> and DSA had no upper or lower bounds checks. I'm not at all familiar
> with DSA either way.
Please just restore the old behaviour.
We can consider changes later.
Andrew
^ permalink raw reply
* [PATCH] flow_dissector: avoid uninitialized variable access
From: Arnd Bergmann @ 2016-10-21 15:55 UTC (permalink / raw)
To: David S. Miller
Cc: Arnd Bergmann, Alexander Duyck, Tom Herbert, Jiri Pirko,
Hadar Hen Zion, Gao Feng, Eric Garver, Amir Vadai, netdev,
linux-kernel
gcc warns about an uninitialized pointer dereference in the vlan
priority handling:
net/core/flow_dissector.c: In function '__skb_flow_dissect':
net/core/flow_dissector.c:281:61: error: 'vlan' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>From all I can tell, this warning is about a real bug, and we
should not attempt look up the vlan header if there was
no vlan tag.
Fixes: f6a66927692e ("flow_dissector: Get vlan priority in addition to vlan id")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I'm not sure about this one, please have a closer look at what
the original code does before applying.
---
net/core/flow_dissector.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 44e6ba9d3a6b..dd6003bf27e1 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -245,7 +245,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
}
case htons(ETH_P_8021AD):
case htons(ETH_P_8021Q): {
- const struct vlan_hdr *vlan;
+ const struct vlan_hdr *vlan = NULL;
if (skb && skb_vlan_tag_present(skb))
proto = skb->protocol;
@@ -264,7 +264,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
}
skip_vlan = true;
- if (dissector_uses_key(flow_dissector,
+ if (vlan && dissector_uses_key(flow_dissector,
FLOW_DISSECTOR_KEY_VLAN)) {
key_vlan = skb_flow_dissector_target(flow_dissector,
FLOW_DISSECTOR_KEY_VLAN,
--
2.9.0
^ permalink raw reply related
* Re: [RFC PATCH 08/13] dwmac-meson8b: add support for phy selection
From: Andrew Lunn @ 2016-10-21 15:54 UTC (permalink / raw)
To: Neil Armstrong
Cc: khilman, carlo, linus.walleij, linux-gpio, devicetree,
linux-arm-kernel, linux-amlogic, linux-kernel, netdev
In-Reply-To: <1477060838-14164-9-git-send-email-narmstrong@baylibre.com>
On Fri, Oct 21, 2016 at 04:40:33PM +0200, Neil Armstrong wrote:
> The Meson GXL dwmac Glue Layer also provides switching between an external PHY
> and an internal RMII 10/100 PHY.
> Add a way to setup the correct PHY switching from a device tree attribute.
Humm, actually. Maybe PHY_IS_INTERNAL in the phydrv->flags is the
better solution. The MAC can then use phy_is_internal(ndev->phydev) to
decide if this register needs programming.
Andrew
^ permalink raw reply
* Re: [PATCH net] net: remove MTU limits on a few ether_setup callers
From: Jarod Wilson @ 2016-10-21 15:48 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, linux-kernel, netdev, Asbjoern Sloth Toennesen,
R Parameswaran, Vivien Didelot
In-Reply-To: <20161021084441.GC16974@lunn.ch>
On Fri, Oct 21, 2016 at 10:44:41AM +0200, Andrew Lunn wrote:
> On Thu, Oct 20, 2016 at 08:42:46PM -0700, Florian Fainelli wrote:
> > Le 20/10/2016 à 20:25, Jarod Wilson a écrit :
> > > These few drivers call ether_setup(), but have no ndo_change_mtu, and thus
> > > were overlooked for changes to MTU range checking behavior. They
> > > previously had no range checks, so for feature-parity, set their min_mtu
> > > to 0 and max_mtu to ETH_MAX_MTU (65535), instead of the 68 and 1500
> > > inherited from the ether_setup() changes. Fine-tuning can come after we get
> > > back to full feature-parity here.
> > >
> > > CC: netdev@vger.kernel.org
> > > Reported-by: Asbjoern Sloth Toennesen <asbjorn@asbjorn.st>
> > > CC: Asbjoern Sloth Toennesen <asbjorn@asbjorn.st>
> > > CC: R Parameswaran <parameswaran.r7@gmail.com>
> > > Signed-off-by: Jarod Wilson <jarod@redhat.com>
> > > ---
> >
> > > diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> > > index 68714a5..d0c7bce 100644
> > > --- a/net/dsa/slave.c
> > > +++ b/net/dsa/slave.c
> > > @@ -1247,6 +1247,8 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
> > > slave_dev->priv_flags |= IFF_NO_QUEUE;
> > > slave_dev->netdev_ops = &dsa_slave_netdev_ops;
> > > slave_dev->switchdev_ops = &dsa_slave_switchdev_ops;
> > > + slave_dev->min_mtu = 0;
> > > + slave_dev->max_mtu = ETH_MAX_MTU;
> > > SET_NETDEV_DEVTYPE(slave_dev, &dsa_type);
> >
> > Actually for DSA, a reasonable minimum is probably 68 for the following
> > reasons: ETH_ZLEN of 60 bytes + FCS (4 bytes) + 2/4/8 bytes of
> > Ethernet switch tag (which is dependent on the tagging protocol
> > used).
>
> Humm, isn't the switch tag added by the layer below this? So this
> should be - 2/4/8 bytes, so that the assembled frame that actually
> hits the conduit interface has an MTU of 64.
I'm all for fine-tuning things to be more correct, but my initial approach
here was to restore the ranges that were previously there, and DSA had no
upper or lower bounds checks. I'm not at all familiar with DSA either way.
> > Such an Ethernet interface would submit packets through the conduit
> > interface which is connected to an Ethernet switches, and those
> > typically discard packets smaller than 64 bytes +/- their custom tag
> > length.
>
> I have a purely theoretical observation, i.e. i have not checked the
> datasheets. You can also control some of the Marvell switches by
> sending it ethernet frames containing commands. Most commands are 4
> bytes long. So an Ethernet header + 4 bytes is < 64. I expect the
> switch will accept command frames which are padded to stop them being
> runts. Also, such frames will be submitted to the conduit interface,
> not the slave interface.
So presumably, slave_dev->min_mtu wouldn't come into play in that
scenario, and we wouldn't break that functionality if min_mtu did get get
to 64 or 68 or whatever > 0.
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply
* Re: [RFC PATCH 08/13] dwmac-meson8b: add support for phy selection
From: Andrew Lunn @ 2016-10-21 15:46 UTC (permalink / raw)
To: Neil Armstrong
Cc: khilman, carlo, linus.walleij, linux-gpio, devicetree,
linux-arm-kernel, linux-amlogic, linux-kernel, netdev
In-Reply-To: <1477060838-14164-9-git-send-email-narmstrong@baylibre.com>
On Fri, Oct 21, 2016 at 04:40:33PM +0200, Neil Armstrong wrote:
> The Meson GXL dwmac Glue Layer also provides switching between an external PHY
> and an internal RMII 10/100 PHY.
> Add a way to setup the correct PHY switching from a device tree attribute.
Hi Neil
Can this be made automatic?
The internal PHY appears to be on address 8. Can there also be an
external PHY on address 8? Could you follow the phy-handle link to the
phy node, check the address, and if it is 8, configure for an internal
PHY?
Andrew
^ permalink raw reply
* Re: [PATCH] net: sched: make default fifo qdiscs appear in the dump
From: Eric Dumazet @ 2016-10-21 15:45 UTC (permalink / raw)
To: David Miller
Cc: jikos, jhs, phil, xiyou.wangcong, daniel, linux-kernel, netdev
In-Reply-To: <20161021.110333.1897572143781385600.davem@davemloft.net>
On Fri, 2016-10-21 at 11:03 -0400, David Miller wrote:
> If this is changing default behavior we should approach this the other
> way around.
>
> Keep behaving the way we do, user asks for new behavior with the attribute.
SGTM
^ permalink raw reply
* Re: [Patch net v2] ipv6: fix a potential deadlock in do_ipv6_setsockopt()
From: David Miller @ 2016-10-21 15:29 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, eric.dumazet, sploving1, marcelo.leitner
In-Reply-To: <1476945312-15572-1-git-send-email-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Wed, 19 Oct 2016 23:35:12 -0700
> Baozeng reported this deadlock case:
>
> CPU0 CPU1
> ---- ----
> lock([ 165.136033] sk_lock-AF_INET6);
> lock([ 165.136033] rtnl_mutex);
> lock([ 165.136033] sk_lock-AF_INET6);
> lock([ 165.136033] rtnl_mutex);
>
> Similar to commit 87e9f0315952
> ("ipv4: fix a potential deadlock in mcast getsockopt() path")
> this is due to we still have a case, ipv6_sock_mc_close(),
> where we acquire sk_lock before rtnl_lock. Close this deadlock
> with the similar solution, that is always acquire rtnl lock first.
>
> Fixes: baf606d9c9b1 ("ipv4,ipv6: grab rtnl before locking the socket")
> Reported-by: Baozeng Ding <sploving1@gmail.com>
> Tested-by: Baozeng Ding <sploving1@gmail.com>
> Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] hv_netvsc: fix a race between netvsc_send() and netvsc_init_buf()
From: David Miller @ 2016-10-21 15:28 UTC (permalink / raw)
To: vkuznets; +Cc: sthemmin, netdev, haiyangz, linux-kernel, devel
In-Reply-To: <1476885181-3456-1-git-send-email-vkuznets@redhat.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: Wed, 19 Oct 2016 15:53:01 +0200
> Fix in commit 880988348270 ("hv_netvsc: set nvdev link after populating
> chn_table") turns out to be incomplete. A crash in
> netvsc_get_next_send_section() is observed on mtu change when the device
> is under load. The race I identified is: if we get to netvsc_send() after
> we set net_device_ctx->nvdev link in netvsc_device_add() but before we
> finish netvsc_connect_vsp()->netvsc_init_buf() send_section_map is not
> allocated and we crash. Unfortunately we can't set net_device_ctx->nvdev
> link after the netvsc_init_buf() call as during the negotiation we need
> to receive packets and on the receive path we check for it. It would
> probably be possible to split nvdev into a pair of nvdev_in and nvdev_out
> links and check them accordingly in get_outbound_net_device()/
> get_inbound_net_device() but this looks like an overkill.
>
> Check that send_section_map is allocated in netvsc_send().
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net: sched: make default fifo qdiscs appear in the dump
From: Eric Dumazet @ 2016-10-21 15:27 UTC (permalink / raw)
To: David Miller
Cc: jikos, jhs, phil, xiyou.wangcong, daniel, linux-kernel, netdev
In-Reply-To: <20161021.105504.662057621984429928.davem@davemloft.net>
On Fri, 2016-10-21 at 10:55 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 21 Oct 2016 04:36:57 -0700
>
> > I guess we'll need to not pull this patch in our kernels.
>
> Eric, quite frankly, this whole "we won't pull this patch into Google
> kernels" threat is getting _REALLY_ _OLD_.
I am sorry you felt it was a threat, this was certainly not my intent.
^ permalink raw reply
* Re: [PATCH net-next] hv_netvsc: fix a race between netvsc_send() and netvsc_init_buf()
From: David Miller @ 2016-10-21 15:27 UTC (permalink / raw)
To: vkuznets; +Cc: sthemmin, netdev, haiyangz, linux-kernel, devel
In-Reply-To: <87twc5ybnl.fsf@vitty.brq.redhat.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: Fri, 21 Oct 2016 17:17:18 +0200
> David Miller <davem@davemloft.net> writes:
>
>> From: Vitaly Kuznetsov <vkuznets@redhat.com>
>> Date: Fri, 21 Oct 2016 13:15:53 +0200
>>
>>> David Miller <davem@davemloft.net> writes:
>>>
>>>> From: Vitaly Kuznetsov <vkuznets@redhat.com>
>>>> Date: Thu, 20 Oct 2016 10:51:04 +0200
>>>>
>>>>> Stephen Hemminger <sthemmin@microsoft.com> writes:
>>>>>
>>>>>> Do we need ACCESS_ONCE() here to avoid check/use issues?
>>>>>>
>>>>>
>>>>> I think we don't: this is the only place in the function where we read
>>>>> the variable so we'll get normal read. We're not trying to syncronize
>>>>> with netvsc_init_buf() as that would require locking, if we read stale
>>>>> NULL value after it was already updated on a different CPU we're fine,
>>>>> we'll just return -EAGAIN.
>>>>
>>>> The concern is if we race with netvsc_destroy_buf() and this pointer
>>>> becomes NULL after the test you are adding.
>>>
>>> Thanks, this is interesting.
>>>
>>> We may reach to netvsc_destroy_buf() by 3 pathes:
>>>
>>> 1) cleanup path in netvsc_init_buf(). It is never called with
>>> send_section_map being not NULL so it seems we're safe.
>>>
>>> 2) from netvsc_remove() when the device is being removed. As far as I
>>> understand we can't be on the transmit path after we call
>>> unregister_netdev() so we're safe.
>>>
>>> 3) from netvsc_change_mtu() and netvsc_set_channels(). These pathes are
>>> specific to netvsc driver as basically we need to remove the device and
>>> add it back to change mtu/number of channels. The underligning 'struct
>>> net_device' stays but the rest is being removed and added back. On both
>>> pathes we first call netvsc_close() which does netif_tx_disable() and as
>>> far as I understand (I may be wrong) this guarantees that we won't be in
>>> netvsc_send().
>>>
>>> So *I think* that we can't ever free send_section_map while in
>>> netvsc_send() and we can't even get to netvsc_send() after it is freed
>>> but I may have missed something.
>>
>> Ok you may be right.
>>
>> Can't the device be taken down by asynchronous events as well? For example
>> if the peer end of the interface in the other guest disappears.
>
> The device may be hot removed from host's side. In this case we follow
> the following call chain:
>
> ... -> vmbus_device_unregister() -> ... -> vmbus_remove() -> netvsc_remove()
>
> and netvsc_remove() does netif_tx_disable(); unregister_netdev();
> before calling rndis_filter_device_remove() leading to netvsc_destroy_buf().
>
> So it seems we can't be in netvsc_send() when the device is
> disappearing.
Ok, it all looks good then.
^ permalink raw reply
* Re: [PATCH net-next] hv_netvsc: fix a race between netvsc_send() and netvsc_init_buf()
From: Vitaly Kuznetsov @ 2016-10-21 15:17 UTC (permalink / raw)
To: David Miller; +Cc: sthemmin, netdev, haiyangz, linux-kernel, devel
In-Reply-To: <20161021.105258.685851223067760389.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> Date: Fri, 21 Oct 2016 13:15:53 +0200
>
>> David Miller <davem@davemloft.net> writes:
>>
>>> From: Vitaly Kuznetsov <vkuznets@redhat.com>
>>> Date: Thu, 20 Oct 2016 10:51:04 +0200
>>>
>>>> Stephen Hemminger <sthemmin@microsoft.com> writes:
>>>>
>>>>> Do we need ACCESS_ONCE() here to avoid check/use issues?
>>>>>
>>>>
>>>> I think we don't: this is the only place in the function where we read
>>>> the variable so we'll get normal read. We're not trying to syncronize
>>>> with netvsc_init_buf() as that would require locking, if we read stale
>>>> NULL value after it was already updated on a different CPU we're fine,
>>>> we'll just return -EAGAIN.
>>>
>>> The concern is if we race with netvsc_destroy_buf() and this pointer
>>> becomes NULL after the test you are adding.
>>
>> Thanks, this is interesting.
>>
>> We may reach to netvsc_destroy_buf() by 3 pathes:
>>
>> 1) cleanup path in netvsc_init_buf(). It is never called with
>> send_section_map being not NULL so it seems we're safe.
>>
>> 2) from netvsc_remove() when the device is being removed. As far as I
>> understand we can't be on the transmit path after we call
>> unregister_netdev() so we're safe.
>>
>> 3) from netvsc_change_mtu() and netvsc_set_channels(). These pathes are
>> specific to netvsc driver as basically we need to remove the device and
>> add it back to change mtu/number of channels. The underligning 'struct
>> net_device' stays but the rest is being removed and added back. On both
>> pathes we first call netvsc_close() which does netif_tx_disable() and as
>> far as I understand (I may be wrong) this guarantees that we won't be in
>> netvsc_send().
>>
>> So *I think* that we can't ever free send_section_map while in
>> netvsc_send() and we can't even get to netvsc_send() after it is freed
>> but I may have missed something.
>
> Ok you may be right.
>
> Can't the device be taken down by asynchronous events as well? For example
> if the peer end of the interface in the other guest disappears.
The device may be hot removed from host's side. In this case we follow
the following call chain:
... -> vmbus_device_unregister() -> ... -> vmbus_remove() -> netvsc_remove()
and netvsc_remove() does netif_tx_disable(); unregister_netdev();
before calling rndis_filter_device_remove() leading to netvsc_destroy_buf().
So it seems we can't be in netvsc_send() when the device is
disappearing.
--
Vitaly
^ permalink raw reply
* Re: [PATCH] net: sched: make default fifo qdiscs appear in the dump
From: David Miller @ 2016-10-21 15:03 UTC (permalink / raw)
To: eric.dumazet
Cc: jikos, jhs, phil, xiyou.wangcong, daniel, linux-kernel, netdev
In-Reply-To: <1477055663.7065.67.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 21 Oct 2016 06:14:23 -0700
> On Fri, 2016-10-21 at 14:56 +0200, Jiri Kosina wrote:
>> On Fri, 21 Oct 2016, Eric Dumazet wrote:
>>
>> > Some of us are dealing with huge HTB hierarchies, so adding default fifo
>> > in the dump will add more data pumped from the kernel.
>> >
>> > BwE [1] for instance dumps qdisc/classes every 5 seconds.
>> >
>> > I guess we'll need to not pull this patch in our kernels.
>>
>> Okay, so I probably misunderstood you here:
>>
>> https://marc.info/?l=linux-kernel&m=146073234818214&w=2
>>
>> as I thought that as long as we move towards the hashtable, you wouldn't
>> have any issues with this.
>>
>> I'd really like to unhide the default qdiscs, it makes little sense to be
>> inconsistent in this way.
>>
>> Random shot into darkness -- how about making this a
>> CONFIG/sysctl-selectable?
>
> Oh sorry for the confusion, I believe your patch is fine.
>
> We could add an netlink attribute later for the users that really do not
> want default fifo being dumped, but there is no hurry.
If this is changing default behavior we should approach this the other
way around.
Keep behaving the way we do, user asks for new behavior with the attribute.
^ permalink raw reply
* Re: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()
From: Ben Hutchings @ 2016-10-21 15:00 UTC (permalink / raw)
To: Jon Maloy, Ying Xue; +Cc: netdev@vger.kernel.org, Qian Zhang, Eric Dumazet
In-Reply-To: <A2BAEFC30C8FD34388F02C9B3121859D22588579@eusaamb103.ericsson.se>
[-- Attachment #1: Type: text/plain, Size: 2046 bytes --]
On Fri, 2016-10-21 at 14:57 +0000, Jon Maloy wrote:
> > -----Original Message-----
> > > > From: Ben Hutchings [mailto:ben@decadent.org.uk]
> > Sent: Thursday, 20 October, 2016 12:40
> > > > To: Jon Maloy <jon.maloy@ericsson.com>; Ying Xue <ying.xue0@gmail.com>
> > > > > > Cc: netdev@vger.kernel.org; Qian Zhang <zhangqian-c@360.cn>; Eric Dumazet
> > > > <edumazet@google.com>
> > Subject: Re: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()
> >
> > On Thu, 2016-10-20 at 14:51 +0000, Jon Maloy wrote:
> > [...]
> > > > At this point we're about to copy INT_H_SIZE + mhsz bytes into the
> > > > first fragment. If that's already limited to be less than or equal to
> > > > MAX_H_SIZE, comparing with MAX_H_SIZE would be fine. But if
> >
> > MAX_H_SIZE
> > > > is the maximum value of mhsz, that won't be good enough.
> > >
> > >
> > >
> > > MAX_H_SIZE is 60 bytes, but in practice you will never see an mhsz larger than
> >
> > the biggest header we are actually using, which is MCAST_H_SIZE (==44 bytes).
> > > INT_H_SIZE is 40 bytes, so you are in reality testing for whether we have an mtu
> >
> > < 84 bytes.
> > > You won't find any interfaces or protocols that come even close to this
> >
> > limitation, so to me this test is redundant.
> >
> > But I can easily create such an interface:
> >
> > $ unshare -n -U -r
> > # ip l set lo mtu 1
> >
> > Ben.
>
>
> It won't be very useful though. But I assume you mean it could be a
> possible exploit,
Exactly.
> and I suspect a few other things would break both in TIPC and in
> other stacks if you do anything like that. I think the solution to
> this is not to fix all possible places in the code where this can go
> wrong, but rather to have a generic test where we refuse to attach
> bearers/interfaces offering an mtu < e.g. 1000 bytes. This can easily
> be done in tipc_enable_l2_media().
Yes.
Ben.
--
Ben Hutchings
One of the nice things about standards is that there are so many of
them.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH] net: sched: make default fifo qdiscs appear in the dump
From: David Miller @ 2016-10-21 14:59 UTC (permalink / raw)
To: jikos; +Cc: eric.dumazet, jhs, phil, xiyou.wangcong, daniel, linux-kernel,
netdev
In-Reply-To: <alpine.LNX.2.00.1610211450440.31629@cbobk.fhfr.pm>
From: Jiri Kosina <jikos@kernel.org>
Date: Fri, 21 Oct 2016 14:56:33 +0200 (CEST)
> I'd really like to unhide the default qdiscs, it makes little sense to be
> inconsistent in this way.
Something that's been invisible for such a long time... there is no
way applications need or require this.
If some new ones do, they can ask for it via the netlink request
inside of a new netlink attribute or similar.
That makes everyone happy.
^ permalink raw reply
* RE: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()
From: Jon Maloy @ 2016-10-21 14:57 UTC (permalink / raw)
To: Ben Hutchings, Ying Xue; +Cc: netdev@vger.kernel.org, Qian Zhang, Eric Dumazet
In-Reply-To: <1476981609.2709.56.camel@decadent.org.uk>
> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Thursday, 20 October, 2016 12:40
> To: Jon Maloy <jon.maloy@ericsson.com>; Ying Xue <ying.xue0@gmail.com>
> Cc: netdev@vger.kernel.org; Qian Zhang <zhangqian-c@360.cn>; Eric Dumazet
> <edumazet@google.com>
> Subject: Re: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()
>
> On Thu, 2016-10-20 at 14:51 +0000, Jon Maloy wrote:
> [...]
> > > At this point we're about to copy INT_H_SIZE + mhsz bytes into the
> > > first fragment. If that's already limited to be less than or equal to
> > > MAX_H_SIZE, comparing with MAX_H_SIZE would be fine. But if
> MAX_H_SIZE
> > > is the maximum value of mhsz, that won't be good enough.
> >
> >
> > MAX_H_SIZE is 60 bytes, but in practice you will never see an mhsz larger than
> the biggest header we are actually using, which is MCAST_H_SIZE (==44 bytes).
> > INT_H_SIZE is 40 bytes, so you are in reality testing for whether we have an mtu
> < 84 bytes.
> > You won't find any interfaces or protocols that come even close to this
> limitation, so to me this test is redundant.
>
> But I can easily create such an interface:
>
> $ unshare -n -U -r
> # ip l set lo mtu 1
>
> Ben.
It won't be very useful though. But I assume you mean it could be a possible exploit, and I suspect a few other things would break both in TIPC and in other stacks if you do anything like that. I think the solution to this is not to fix all possible places in the code where this can go wrong, but rather to have a generic test where we refuse to attach bearers/interfaces offering an mtu < e.g. 1000 bytes. This can easily be done in tipc_enable_l2_media().
///jon
>
> --
> Ben Hutchings
> Never put off till tomorrow what you can avoid all together.
^ permalink raw reply
* RE: [PATCH v8 0/8] thunderbolt: Introducing Thunderbolt(TM) Networking
From: Mario.Limonciello @ 2016-10-21 14:57 UTC (permalink / raw)
To: amir.jer.levy, gregkh, andreas.noever
Cc: bhelgaas, corbet, linux-kernel, linux-pci, netdev, linux-doc,
thunderbolt-linux, mika.westerberg, tomas.winkler, xiong.y.zhang
In-Reply-To: <1475073870-2126-1-git-send-email-amir.jer.levy@intel.com>
> -----Original Message-----
> From: Amir Levy [mailto:amir.jer.levy@intel.com]
> Sent: Wednesday, September 28, 2016 9:44 AM
> To: gregkh@linuxfoundation.org
> Cc: andreas.noever@gmail.com; bhelgaas@google.com; corbet@lwn.net;
> linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org;
> netdev@vger.kernel.org; linux-doc@vger.kernel.org; Limonciello, Mario
> <Mario_Limonciello@Dell.com>; thunderbolt-linux@intel.com;
> mika.westerberg@intel.com; tomas.winkler@intel.com;
> xiong.y.zhang@intel.com; Amir Levy <amir.jer.levy@intel.com>
> Subject: [PATCH v8 0/8] thunderbolt: Introducing Thunderbolt(TM)
> Networking
>
> This driver enables Thunderbolt Networking on non-Apple platforms
> running Linux.
>
> Thunderbolt Networking provides peer-to-peer connections to transfer
> files between computers, perform PC migrations, and/or set up small
> workgroups with shared storage.
>
> This is a virtual connection that emulates an Ethernet adapter that
> enables Ethernet networking with the benefit of Thunderbolt superfast
> medium capability.
>
> Thunderbolt Networking enables two hosts and several devices that
> have a Thunderbolt controller to be connected together in a linear
> (Daisy chain) series from a single port.
>
> Thunderbolt Networking for Linux is compatible with Thunderbolt
> Networking on systems running macOS or Windows and also supports
> Thunderbolt generation 2 and 3 controllers.
>
> Note that all pre-existing Thunderbolt generation 3 features, such as
> USB, Display and other Thunderbolt device connectivity will continue
> to function exactly as they did prior to enabling Thunderbolt Networking.
>
> Code and Software Specifications:
> This kernel code creates a virtual ethernet device for computer to
> computer communication over a Thunderbolt cable.
> The new driver is a separate driver to the existing Thunderbolt driver.
> It is designed to work on systems running Linux that
> interface with Intel Connection Manager (ICM) firmware based
> Thunderbolt controllers that support Thunderbolt Networking.
> The kernel code operates in coordination with the Thunderbolt user-
> space daemon to implement full Thunderbolt networking functionality.
>
> Hardware Specifications:
> Thunderbolt Hardware specs have not yet been published but are used
> where necessary for register definitions.
>
> Changes since v7:
> - Removed debug prints
> - Edited error prints
> - Edited copyright notice
> - Changed the Kconfig patch to be after the code changes
>
> These patches were pushed to GitHub where they can be reviewed more
> comfortably with green/red highlighting:
> https://github.com/01org/thunderbolt-software-kernel-tree
>
> Daemon code:
> https://github.com/01org/thunderbolt-software-daemon
>
> For reference, here's a link to version 6:
> [v7]: https://lkml.org/lkml/2016/9/27/244
>
> Amir Levy (8):
> thunderbolt: Macro rename
> thunderbolt: Updating the register definitions
> thunderbolt: Communication with the ICM (firmware)
> thunderbolt: Networking state machine
> thunderbolt: Networking transmit and receive
> thunderbolt: Kconfig for Thunderbolt Networking
> thunderbolt: Networking doc
> thunderbolt: Adding maintainer entry
>
> Documentation/00-INDEX | 2 +
> Documentation/thunderbolt/networking.txt | 132 ++
> MAINTAINERS | 8 +-
> drivers/thunderbolt/Kconfig | 27 +-
> drivers/thunderbolt/Makefile | 3 +-
> drivers/thunderbolt/icm/Makefile | 2 +
> drivers/thunderbolt/icm/icm_nhi.c | 1514 ++++++++++++++++++++
> drivers/thunderbolt/icm/icm_nhi.h | 82 ++
> drivers/thunderbolt/icm/net.c | 2254
> ++++++++++++++++++++++++++++++
> drivers/thunderbolt/icm/net.h | 287 ++++
> drivers/thunderbolt/nhi_regs.h | 115 +-
> 11 files changed, 4417 insertions(+), 9 deletions(-)
> create mode 100644 Documentation/thunderbolt/networking.txt
> create mode 100644 drivers/thunderbolt/icm/Makefile
> create mode 100644 drivers/thunderbolt/icm/icm_nhi.c
> create mode 100644 drivers/thunderbolt/icm/icm_nhi.h
> create mode 100644 drivers/thunderbolt/icm/net.c
> create mode 100644 drivers/thunderbolt/icm/net.h
>
> --
> 2.7.4
Hi Amir,
I've tested your v8 series on Dell hardware with Thunderbolt
Controllers again between a Linux and Windows box.
Functionally it's working well.
Tested-By: Mario Limonciello <mario.limonciello@dell.com>
Andreas,
Following the history of this thread, I believe Greg was still looking for
an ack from you that Amir is using the interface properly.
Thanks,
^ permalink raw reply
* Re: [PATCH] net: sched: make default fifo qdiscs appear in the dump
From: David Miller @ 2016-10-21 14:55 UTC (permalink / raw)
To: eric.dumazet
Cc: jikos, jhs, phil, xiyou.wangcong, daniel, linux-kernel, netdev
In-Reply-To: <1477049817.7065.56.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 21 Oct 2016 04:36:57 -0700
> I guess we'll need to not pull this patch in our kernels.
Eric, quite frankly, this whole "we won't pull this patch into Google
kernels" threat is getting _REALLY_ _OLD_.
If you have a valid technical argument as to why a change should or
should not be applied, make it. But don't add all of this unnecessary
information, it's simply irrelevant to the technical discussion and
is just emotionally charged.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next] hv_netvsc: fix a race between netvsc_send() and netvsc_init_buf()
From: David Miller @ 2016-10-21 14:52 UTC (permalink / raw)
To: vkuznets; +Cc: sthemmin, netdev, haiyangz, linux-kernel, devel
In-Reply-To: <87d1iuymty.fsf@vitty.brq.redhat.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: Fri, 21 Oct 2016 13:15:53 +0200
> David Miller <davem@davemloft.net> writes:
>
>> From: Vitaly Kuznetsov <vkuznets@redhat.com>
>> Date: Thu, 20 Oct 2016 10:51:04 +0200
>>
>>> Stephen Hemminger <sthemmin@microsoft.com> writes:
>>>
>>>> Do we need ACCESS_ONCE() here to avoid check/use issues?
>>>>
>>>
>>> I think we don't: this is the only place in the function where we read
>>> the variable so we'll get normal read. We're not trying to syncronize
>>> with netvsc_init_buf() as that would require locking, if we read stale
>>> NULL value after it was already updated on a different CPU we're fine,
>>> we'll just return -EAGAIN.
>>
>> The concern is if we race with netvsc_destroy_buf() and this pointer
>> becomes NULL after the test you are adding.
>
> Thanks, this is interesting.
>
> We may reach to netvsc_destroy_buf() by 3 pathes:
>
> 1) cleanup path in netvsc_init_buf(). It is never called with
> send_section_map being not NULL so it seems we're safe.
>
> 2) from netvsc_remove() when the device is being removed. As far as I
> understand we can't be on the transmit path after we call
> unregister_netdev() so we're safe.
>
> 3) from netvsc_change_mtu() and netvsc_set_channels(). These pathes are
> specific to netvsc driver as basically we need to remove the device and
> add it back to change mtu/number of channels. The underligning 'struct
> net_device' stays but the rest is being removed and added back. On both
> pathes we first call netvsc_close() which does netif_tx_disable() and as
> far as I understand (I may be wrong) this guarantees that we won't be in
> netvsc_send().
>
> So *I think* that we can't ever free send_section_map while in
> netvsc_send() and we can't even get to netvsc_send() after it is freed
> but I may have missed something.
Ok you may be right.
Can't the device be taken down by asynchronous events as well? For example
if the peer end of the interface in the other guest disappears.
^ permalink raw reply
* Re: [PATCH] net: stmmac: Add OXNAS Glue Driver
From: Neil Armstrong @ 2016-10-21 14:43 UTC (permalink / raw)
To: Giuseppe CAVALLARO, Joachim Eastwood
Cc: alexandre.torgue, netdev, linux-oxnas,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree
In-Reply-To: <474261f0-1cb7-5932-4c9f-0cbcc705fa61@st.com>
On 10/21/2016 01:53 PM, Giuseppe CAVALLARO wrote:
> Hello
>
> some my minor cents below
>
> On 10/21/2016 12:20 PM, Joachim Eastwood wrote:
>> Hi Neil,
>>
>> On 21 October 2016 at 10:44, Neil Armstrong <narmstrong@baylibre.com> wrote:
>>> Add Synopsys Designware MAC Glue layer for the Oxford Semiconductor OX820.
>>>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>> .../devicetree/bindings/net/oxnas-dwmac.txt | 44 +++++
>>> drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 ++
>>> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
>>> drivers/net/ethernet/stmicro/stmmac/dwmac-oxnas.c | 219 +++++++++++++++++++++
>>> 4 files changed, 275 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/net/oxnas-dwmac.txt
>>> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-oxnas.c
>>>
>>> Changes since RFC at https://patchwork.kernel.org/patch/9387257 :
>>> - Drop init/exit callbacks
>>> - Implement proper remove and PM callback
>>> - Call init from probe
>>> - Disable/Unprepare clock if stmmac probe fails
>>
>> <snip>
>>
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-oxnas.c
>>> @@ -0,0 +1,219 @@
>>> +/*
>>> + * Oxford Semiconductor OXNAS DWMAC glue layer
>>> + *
>>> + * Copyright (C) 2016 Neil Armstrong <narmstrong@baylibre.com>
>>> + * Copyright (C) 2014 Daniel Golle <daniel@makrotopia.org>
>>> + * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>
>>> + * Copyright (C) 2012 John Crispin <blogic@openwrt.org>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/device.h>
>>> +#include <linux/io.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/mfd/syscon.h>
>>> +#include <linux/stmmac.h>
>>> +
>>> +#include "stmmac_platform.h"
>>> +
>>> +/* System Control regmap offsets */
>>> +#define OXNAS_DWMAC_CTRL_REGOFFSET 0x78
>>> +#define OXNAS_DWMAC_DELAY_REGOFFSET 0x100
>>> +
>>> +/* Control Register */
>>> +#define DWMAC_CKEN_RX_IN 14
>>> +#define DWMAC_CKEN_RXN_OUT 13
>>> +#define DWMAC_CKEN_RX_OUT 12
>>> +#define DWMAC_CKEN_TX_IN 10
>>> +#define DWMAC_CKEN_TXN_OUT 9
>>> +#define DWMAC_CKEN_TX_OUT 8
>>> +#define DWMAC_RX_SOURCE 7
>>> +#define DWMAC_TX_SOURCE 6
>>> +#define DWMAC_LOW_TX_SOURCE 4
>>> +#define DWMAC_AUTO_TX_SOURCE 3
>>> +#define DWMAC_RGMII 2
>>> +#define DWMAC_SIMPLE_MUX 1
>>> +#define DWMAC_CKEN_GTX 0
>>> +
>>> +/* Delay register */
>>> +#define DWMAC_TX_VARDELAY_SHIFT 0
>>> +#define DWMAC_TXN_VARDELAY_SHIFT 8
>>> +#define DWMAC_RX_VARDELAY_SHIFT 16
>>> +#define DWMAC_RXN_VARDELAY_SHIFT 24
>>> +#define DWMAC_TX_VARDELAY(d) ((d) << DWMAC_TX_VARDELAY_SHIFT)
>>> +#define DWMAC_TXN_VARDELAY(d) ((d) << DWMAC_TXN_VARDELAY_SHIFT)
>>> +#define DWMAC_RX_VARDELAY(d) ((d) << DWMAC_RX_VARDELAY_SHIFT)
>>> +#define DWMAC_RXN_VARDELAY(d) ((d) << DWMAC_RXN_VARDELAY_SHIFT)
>>> +
>>> +struct oxnas_dwmac {
>>> + struct device *dev;
>>> + struct clk *clk;
>>> + struct regmap *regmap;
>>> +};
>>> +
>>> +static int oxnas_dwmac_init(struct oxnas_dwmac *dwmac)
>>> +{
>>> + unsigned int value;
>>> + int ret;
>>> +
>>> + /* Reset HW here before changing the glue configuration */
>>> + ret = device_reset(dwmac->dev);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + clk_prepare_enable(dwmac->clk);
>>
>> You might want to check the return value from clk_prepare_enable() as well.
>>
>>> +
>>> + ret = regmap_read(dwmac->regmap, OXNAS_DWMAC_CTRL_REGOFFSET, &value);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + /* Enable GMII_GTXCLK to follow GMII_REFCLK, required for gigabit PHY */
>>> + value |= BIT(DWMAC_CKEN_GTX);
>>> + /* Use simple mux for 25/125 Mhz clock switching */
>>> + value |= BIT(DWMAC_SIMPLE_MUX);
>>> + /* set auto switch tx clock source */
>>> + value |= BIT(DWMAC_AUTO_TX_SOURCE);
>>> + /* enable tx & rx vardelay */
>>> + value |= BIT(DWMAC_CKEN_TX_OUT);
>>> + value |= BIT(DWMAC_CKEN_TXN_OUT);
>>> + value |= BIT(DWMAC_CKEN_TX_IN);
>>> + value |= BIT(DWMAC_CKEN_RX_OUT);
>>> + value |= BIT(DWMAC_CKEN_RXN_OUT);
>>> + value |= BIT(DWMAC_CKEN_RX_IN);
>>> + regmap_write(dwmac->regmap, OXNAS_DWMAC_CTRL_REGOFFSET, value);
>>> +
>>> + /* set tx & rx vardelay */
>>> + value = DWMAC_TX_VARDELAY(4);
>>> + value |= DWMAC_TXN_VARDELAY(2);
>>> + value |= DWMAC_RX_VARDELAY(10);
>>> + value |= DWMAC_RXN_VARDELAY(8);
>>> + regmap_write(dwmac->regmap, OXNAS_DWMAC_DELAY_REGOFFSET, value);
>
>
> there is no if condition so, I can suggest you, to hardwire
> value with macros instead of computing at runtime:
>
> e.g.
>
> var = DWMAC_VARDELAY where
> #define DWMAC_VARDELAY (DWMAC_TX_VARDELAY(4) | ...)
>
> ... same for OXNAS_DWMAC_CTRL_REGOFFSET where
> BIT(DWMAC_CKEN_ ... ) should be re-organized as macros,
> I mean:
> #define DWMAC_CKEN_.. BIT(xxx)
I will think about something similar for v2,
Thanks,
Neil
[...]
^ permalink raw reply
* Re: [PATCH] net: stmmac: Add OXNAS Glue Driver
From: Neil Armstrong @ 2016-10-21 14:43 UTC (permalink / raw)
To: Joachim Eastwood
Cc: peppe.cavallaro, alexandre.torgue, netdev, linux-oxnas,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree
In-Reply-To: <CAGhQ9Vxe8fe8kfF_WY40xdh6YX9TuxQ3-Dd6pBG=AHpLtrHnMA@mail.gmail.com>
On 10/21/2016 12:20 PM, Joachim Eastwood wrote:
> Hi Neil,
>
> On 21 October 2016 at 10:44, Neil Armstrong <narmstrong@baylibre.com> wrote:
>> Add Synopsys Designware MAC Glue layer for the Oxford Semiconductor OX820.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> .../devicetree/bindings/net/oxnas-dwmac.txt | 44 +++++
>> drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 ++
>> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
>> drivers/net/ethernet/stmicro/stmmac/dwmac-oxnas.c | 219 +++++++++++++++++++++
>> 4 files changed, 275 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/oxnas-dwmac.txt
>> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-oxnas.c
>>
>> Changes since RFC at https://patchwork.kernel.org/patch/9387257 :
>> - Drop init/exit callbacks
>> - Implement proper remove and PM callback
>> - Call init from probe
>> - Disable/Unprepare clock if stmmac probe fails
>
> <snip>
>
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-oxnas.c
>> @@ -0,0 +1,219 @@
>> +/*
>> + * Oxford Semiconductor OXNAS DWMAC glue layer
>> + *
>> + * Copyright (C) 2016 Neil Armstrong <narmstrong@baylibre.com>
>> + * Copyright (C) 2014 Daniel Golle <daniel@makrotopia.org>
>> + * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>
>> + * Copyright (C) 2012 John Crispin <blogic@openwrt.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/stmmac.h>
>> +
>> +#include "stmmac_platform.h"
>> +
>> +/* System Control regmap offsets */
>> +#define OXNAS_DWMAC_CTRL_REGOFFSET 0x78
>> +#define OXNAS_DWMAC_DELAY_REGOFFSET 0x100
>> +
>> +/* Control Register */
>> +#define DWMAC_CKEN_RX_IN 14
>> +#define DWMAC_CKEN_RXN_OUT 13
>> +#define DWMAC_CKEN_RX_OUT 12
>> +#define DWMAC_CKEN_TX_IN 10
>> +#define DWMAC_CKEN_TXN_OUT 9
>> +#define DWMAC_CKEN_TX_OUT 8
>> +#define DWMAC_RX_SOURCE 7
>> +#define DWMAC_TX_SOURCE 6
>> +#define DWMAC_LOW_TX_SOURCE 4
>> +#define DWMAC_AUTO_TX_SOURCE 3
>> +#define DWMAC_RGMII 2
>> +#define DWMAC_SIMPLE_MUX 1
>> +#define DWMAC_CKEN_GTX 0
>> +
>> +/* Delay register */
>> +#define DWMAC_TX_VARDELAY_SHIFT 0
>> +#define DWMAC_TXN_VARDELAY_SHIFT 8
>> +#define DWMAC_RX_VARDELAY_SHIFT 16
>> +#define DWMAC_RXN_VARDELAY_SHIFT 24
>> +#define DWMAC_TX_VARDELAY(d) ((d) << DWMAC_TX_VARDELAY_SHIFT)
>> +#define DWMAC_TXN_VARDELAY(d) ((d) << DWMAC_TXN_VARDELAY_SHIFT)
>> +#define DWMAC_RX_VARDELAY(d) ((d) << DWMAC_RX_VARDELAY_SHIFT)
>> +#define DWMAC_RXN_VARDELAY(d) ((d) << DWMAC_RXN_VARDELAY_SHIFT)
>> +
>> +struct oxnas_dwmac {
>> + struct device *dev;
>> + struct clk *clk;
>> + struct regmap *regmap;
>> +};
>> +
>> +static int oxnas_dwmac_init(struct oxnas_dwmac *dwmac)
>> +{
>> + unsigned int value;
>> + int ret;
>> +
>> + /* Reset HW here before changing the glue configuration */
>> + ret = device_reset(dwmac->dev);
>> + if (ret)
>> + return ret;
>> +
>> + clk_prepare_enable(dwmac->clk);
>
> You might want to check the return value from clk_prepare_enable() as well.
>
>> +
>> + ret = regmap_read(dwmac->regmap, OXNAS_DWMAC_CTRL_REGOFFSET, &value);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* Enable GMII_GTXCLK to follow GMII_REFCLK, required for gigabit PHY */
>> + value |= BIT(DWMAC_CKEN_GTX);
>> + /* Use simple mux for 25/125 Mhz clock switching */
>> + value |= BIT(DWMAC_SIMPLE_MUX);
>> + /* set auto switch tx clock source */
>> + value |= BIT(DWMAC_AUTO_TX_SOURCE);
>> + /* enable tx & rx vardelay */
>> + value |= BIT(DWMAC_CKEN_TX_OUT);
>> + value |= BIT(DWMAC_CKEN_TXN_OUT);
>> + value |= BIT(DWMAC_CKEN_TX_IN);
>> + value |= BIT(DWMAC_CKEN_RX_OUT);
>> + value |= BIT(DWMAC_CKEN_RXN_OUT);
>> + value |= BIT(DWMAC_CKEN_RX_IN);
>> + regmap_write(dwmac->regmap, OXNAS_DWMAC_CTRL_REGOFFSET, value);
>> +
>> + /* set tx & rx vardelay */
>> + value = DWMAC_TX_VARDELAY(4);
>> + value |= DWMAC_TXN_VARDELAY(2);
>> + value |= DWMAC_RX_VARDELAY(10);
>> + value |= DWMAC_RXN_VARDELAY(8);
>> + regmap_write(dwmac->regmap, OXNAS_DWMAC_DELAY_REGOFFSET, value);
>> +
>> + return 0;
>> +}
>> +
>> +static int oxnas_dwmac_probe(struct platform_device *pdev)
>> +{
>> + struct plat_stmmacenet_data *plat_dat;
>> + struct stmmac_resources stmmac_res;
>> + struct device_node *sysctrl;
>> + struct oxnas_dwmac *dwmac;
>> + int ret;
>> +
>> + sysctrl = of_parse_phandle(pdev->dev.of_node, "oxsemi,sys-ctrl", 0);
>> + if (!sysctrl) {
>> + dev_err(&pdev->dev, "failed to get sys-ctrl node\n");
>> + return -EINVAL;
>> + }
>> +
>> + ret = stmmac_get_platform_resources(pdev, &stmmac_res);
>> + if (ret)
>> + return ret;
>> +
>> + plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
>> + if (IS_ERR(plat_dat))
>> + return PTR_ERR(plat_dat);
>> +
>> + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
>> + if (!dwmac)
>> + return -ENOMEM;
>> +
>> + dwmac->dev = &pdev->dev;
>> + plat_dat->bsp_priv = dwmac;
>> +
>> + dwmac->regmap = syscon_node_to_regmap(sysctrl);
>> + if (IS_ERR(dwmac->regmap)) {
>> + dev_err(&pdev->dev, "failed to have sysctrl regmap\n");
>> + return PTR_ERR(dwmac->regmap);
>> + }
>> +
>> + dwmac->clk = devm_clk_get(&pdev->dev, "gmac");
>> + if (IS_ERR(dwmac->clk))
>> + return PTR_ERR(dwmac->clk);
>> +
>> + ret = oxnas_dwmac_init(dwmac);
>> + if (ret)
>> + return ret;
>> +
>> + ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>> + if (ret)
>> + clk_disable_unprepare(dwmac->clk);
>> +
>> + return ret;
>> +}
>> +
>> +static int oxnas_dwmac_remove(struct platform_device *pdev)
>> +{
>> + struct net_device *ndev = platform_get_drvdata(pdev);
>> + struct stmmac_priv *priv = netdev_priv(ndev);
>> + struct oxnas_dwmac *dwmac = priv->plat->bsp_priv;
>
> Instead of this long dance of variables use the get_stmmac_bsp_priv()-helper.
>
> You can take a look at dwmac-meson8b.c for reference.
>
>
>> + int ret = stmmac_dvr_remove(&pdev->dev);
>> +
>> + clk_disable_unprepare(dwmac->clk);
>> +
>> + return ret;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int oxnas_dwmac_suspend(struct device *dev)
>> +{
>> + struct net_device *ndev = dev_get_drvdata(dev);
>> + struct stmmac_priv *priv = netdev_priv(ndev);
>> + struct oxnas_dwmac *dwmac = priv->plat->bsp_priv;
>
> get_stmmac_bsp_priv()
>
>
>> + int ret;
>> +
>> + ret = stmmac_suspend(dev);
>> + clk_disable_unprepare(dwmac->clk);
>> +
>> + return ret;
>> +}
>> +
>> +static int oxnas_dwmac_resume(struct device *dev)
>> +{
>> + struct net_device *ndev = dev_get_drvdata(dev);
>> + struct stmmac_priv *priv = netdev_priv(ndev);
>> + struct oxnas_dwmac *dwmac = priv->plat->bsp_priv;
>
> get_stmmac_bsp_priv()
>
>
>> + int ret;
>> +
>> + ret = oxnas_dwmac_init(dwmac);
>> + if (ret)
>> + return ret;
>> +
>> + ret = stmmac_resume(dev);
>> +
>> + return ret;
>> +}
>> +#endif /* CONFIG_PM_SLEEP */
>
> With these changes:
> Acked-by: Joachim Eastwood <manabian@gmail.com>
>
>
> best regards,
> Joachim Eastwood
>
Thanks,
Will do this for v2
Neil
^ permalink raw reply
* [RFC PATCH 10/13] ARM64: dts: meson-gxl: Add ethernet nodes with internal PHY
From: Neil Armstrong @ 2016-10-21 14:40 UTC (permalink / raw)
To: khilman, carlo, linus.walleij
Cc: Neil Armstrong, linux-gpio, devicetree, linux-arm-kernel,
linux-amlogic, linux-kernel, netdev
In-Reply-To: <1477060838-14164-1-git-send-email-narmstrong@baylibre.com>
Add Ethernet node with Internal PHY selection for the Amlogic GXL SoCs
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index d1bf381..85969c6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -49,6 +49,34 @@
compatible = "amlogic,meson-gxl";
};
+ðmac {
+ reg = <0x0 0xc9410000 0x0 0x10000
+ 0x0 0xc8834540 0x0 0x4
+ 0x0 0xc8834558 0x0 0xc>;
+
+ clocks = <&clkc CLKID_ETH>,
+ <&clkc CLKID_FCLK_DIV2>,
+ <&clkc CLKID_MPLL2>;
+ clock-names = "stmmaceth", "clkin0", "clkin1";
+
+ /* Select Internal PHY by default */
+ amlogic,phy-select = <0xe40908ff>;
+ phy-mode = "rmii";
+ phy-handle = <&internal_phy>;
+
+ mdio0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "snps,dwmac-mdio";
+
+ internal_phy: ethernet-phy@8 {
+ compatible = "ethernet-phy-id0181.4400", "ethernet-phy-ieee802.3-c22";
+ reg = <8>;
+ max-speed = <100>;
+ };
+ };
+};
+
&aobus {
pinctrl_aobus: pinctrl@14 {
compatible = "amlogic,meson-gxl-aobus-pinctrl";
--
1.9.1
^ permalink raw reply related
* [RFC PATCH 08/13] dwmac-meson8b: add support for phy selection
From: Neil Armstrong @ 2016-10-21 14:40 UTC (permalink / raw)
To: khilman, carlo, linus.walleij
Cc: Neil Armstrong, linux-gpio, devicetree, linux-arm-kernel,
linux-amlogic, linux-kernel, netdev
In-Reply-To: <1477060838-14164-1-git-send-email-narmstrong@baylibre.com>
The Meson GXL dwmac Glue Layer also provides switching between an external PHY
and an internal RMII 10/100 PHY.
Add a way to setup the correct PHY switching from a device tree attribute.
Currently, the register format is unknown and this is a temporary workaround
until a clarification on the registers fields is received from Amlogic.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
.../net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 250e4ce..875cd7c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -53,10 +53,15 @@
#define MUX_CLK_NUM_PARENTS 2
+#define PHYSEL_REG0 0x0
+#define PHYSEL_REG0_VALUE 0x10110181
+#define PHYSEL_REG1 0x4
+
struct meson8b_dwmac {
struct platform_device *pdev;
void __iomem *regs;
+ void __iomem *physel_regs;
phy_interface_t phy_mode;
@@ -244,6 +249,23 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TX_AND_PHY_REF_CLK,
PRG_ETH0_TX_AND_PHY_REF_CLK);
+ /* Select PHY, either internal or external if specified */
+ if (!IS_ERR(dwmac->physel_regs) &&
+ of_find_property(dwmac->pdev->dev.of_node,
+ "amlogic,phy-select", NULL)) {
+ u32 val;
+
+ ret = of_property_read_u32(dwmac->pdev->dev.of_node,
+ "amlogic,phy-select", &val);
+ if (ret) {
+ dev_err(&dwmac->pdev->dev, "invalid phy-select property\n");
+ } else {
+ writel(PHYSEL_REG0_VALUE,
+ dwmac->physel_regs + PHYSEL_REG0);
+ writel(val, dwmac->physel_regs + PHYSEL_REG1);
+ }
+ }
+
return 0;
}
@@ -272,6 +294,9 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
if (IS_ERR(dwmac->regs))
return PTR_ERR(dwmac->regs);
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+ dwmac->physel_regs = devm_ioremap_resource(&pdev->dev, res);
+
dwmac->pdev = pdev;
dwmac->phy_mode = of_get_phy_mode(pdev->dev.of_node);
if (dwmac->phy_mode < 0) {
--
1.9.1
^ permalink raw reply related
* [RFC PATCH 06/13] ARM64: dts: meson-gxl: Add MMC/SD/SDIO nodes
From: Neil Armstrong @ 2016-10-21 14:40 UTC (permalink / raw)
To: khilman, carlo, linus.walleij
Cc: Neil Armstrong, linux-gpio, devicetree, linux-arm-kernel,
linux-amlogic, linux-kernel, netdev
In-Reply-To: <1477060838-14164-1-git-send-email-narmstrong@baylibre.com>
Add MMC/SD/SDIO nodes clock attributes for Amlogic Meson GXL.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index b45df2a..d1bf381 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -235,3 +235,24 @@
&i2c_C {
clocks = <&clkc CLKID_I2C>;
};
+
+&sd_emmc_a {
+ clocks = <&clkc CLKID_SD_EMMC_A>,
+ <&xtal>,
+ <&clkc CLKID_FCLK_DIV2>;
+ clock-names = "core", "clkin0", "clkin1";
+};
+
+&sd_emmc_b {
+ clocks = <&clkc CLKID_SD_EMMC_B>,
+ <&xtal>,
+ <&clkc CLKID_FCLK_DIV2>;
+ clock-names = "core", "clkin0", "clkin1";
+};
+
+&sd_emmc_c {
+ clocks = <&clkc CLKID_SD_EMMC_C>,
+ <&xtal>,
+ <&clkc CLKID_FCLK_DIV2>;
+ clock-names = "core", "clkin0", "clkin1";
+};
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox