* Re: [PATCH] net: stmmac: add sanity check to device_property_read_u32_array call
From: Colin Ian King @ 2019-06-28 8:32 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: alexandre.torgue, davem, joabreu, kernel-janitors,
linux-arm-kernel, linux-kernel, linux-stm32, mcoquelin.stm32,
netdev, peppe.cavallaro
In-Reply-To: <CAFBinCBk5aPVE+vq5px3QKS1T_R=WGXXxEJMC9X676KGvi9jdg@mail.gmail.com>
On 28/06/2019 05:15, Martin Blumenstingl wrote:
> On Tue, Jun 25, 2019 at 9:58 AM Colin Ian King <colin.king@canonical.com> wrote:
>>
>> On 25/06/2019 05:44, Martin Blumenstingl wrote:
>>> Hi Colin,
>>>
>>> On Thu, Jun 20, 2019 at 3:34 AM Martin Blumenstingl
>>> <martin.blumenstingl@googlemail.com> wrote:
>>>>
>>>> Hi Colin,
>>>>
>>>> On Wed, Jun 19, 2019 at 8:55 AM Colin Ian King <colin.king@canonical.com> wrote:
>>>>>
>>>>> On 19/06/2019 06:13, Martin Blumenstingl wrote:
>>>>>> Hi Colin,
>>>>>>
>>>>>>> Currently the call to device_property_read_u32_array is not error checked
>>>>>>> leading to potential garbage values in the delays array that are then used
>>>>>>> in msleep delays. Add a sanity check to the property fetching.
>>>>>>>
>>>>>>> Addresses-Coverity: ("Uninitialized scalar variable")
>>>>>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>>>>> I have also sent a patch [0] to fix initialize the array.
>>>>>> can you please look at my patch so we can work out which one to use?
>>>>>>
>>>>>> my concern is that the "snps,reset-delays-us" property is optional,
>>>>>> the current dt-bindings documentation states that it's a required
>>>>>> property. in reality it isn't, there are boards (two examples are
>>>>>> mentioned in my patch: [0]) without it.
>>>>>>
>>>>>> so I believe that the resulting behavior has to be:
>>>>>> 1. don't delay if this property is missing (instead of delaying for
>>>>>> <garbage value> ms)
>>>>>> 2. don't error out if this property is missing
>>>>>>
>>>>>> your patch covers #1, can you please check whether #2 is also covered?
>>>>>> I tested case #2 when submitting my patch and it worked fine (even
>>>>>> though I could not reproduce the garbage values which are being read
>>>>>> on some boards)
>>> in the meantime I have tested your patch.
>>> when I don't set the "snps,reset-delays-us" property then I get the
>>> following error:
>>> invalid property snps,reset-delays-us
>>>
>>> my patch has landed in the meantime: [0]
>>> how should we proceed with your patch?
Your fix is good, so I think we should just drop/forget about my fix.
Colin
>>
>> I'm out of the office today. I'll get back to you on this tomorrow.
> gentle ping
> (I will be away for the weekend but I can reply on Monday)
>
^ permalink raw reply
* Re: XDP multi-buffer incl. jumbo-frames (Was: [RFC V1 net-next 1/1] net: ena: implement XDP drop support)
From: Jesper Dangaard Brouer @ 2019-06-28 8:22 UTC (permalink / raw)
To: Eelco Chaudron
Cc: Machulsky, Zorik, Jubran, Samih, davem, netdev, Woodhouse, David,
Matushevsky, Alexander, Bshara, Saeed, Wilson, Matt,
Liguori, Anthony, Bshara, Nafea, Tzalik, Guy, Belgazal, Netanel,
Saidi, Ali, Herrenschmidt, Benjamin, Kiyanovski, Arthur,
Daniel Borkmann, Toke Høiland-Jørgensen,
Ilias Apalodimas, Alexei Starovoitov, Jakub Kicinski, xdp-newbies,
brouer, Steffen Klassert
In-Reply-To: <CC99D6DE-5B6B-42F3-8D68-7F9AFF1712FF@redhat.com>
On Fri, 28 Jun 2019 09:14:39 +0200
"Eelco Chaudron" <echaudro@redhat.com> wrote:
> On 26 Jun 2019, at 10:38, Jesper Dangaard Brouer wrote:
>
> > On Tue, 25 Jun 2019 03:19:22 +0000
> > "Machulsky, Zorik" <zorik@amazon.com> wrote:
> >
> >> On 6/23/19, 7:21 AM, "Jesper Dangaard Brouer" <brouer@redhat.com>
> >> wrote:
> >>
> >> On Sun, 23 Jun 2019 10:06:49 +0300 <sameehj@amazon.com> wrote:
> >>
> >> > This commit implements the basic functionality of drop/pass logic in the
> >> > ena driver.
> >>
> >> Usually we require a driver to implement all the XDP return codes,
> >> before we accept it. But as Daniel and I discussed with Zorik during
> >> NetConf[1], we are going to make an exception and accept the driver
> >> if you also implement XDP_TX.
> >>
> >> As we trust that Zorik/Amazon will follow and implement XDP_REDIRECT
> >> later, given he/you wants AF_XDP support which requires XDP_REDIRECT.
> >>
> >> Jesper, thanks for your comments and very helpful discussion during
> >> NetConf! That's the plan, as we agreed. From our side I would like to
> >> reiterate again the importance of multi-buffer support by xdp frame.
> >> We would really prefer not to see our MTU shrinking because of xdp
> >> support.
> >
> > Okay we really need to make a serious attempt to find a way to support
> > multi-buffer packets with XDP. With the important criteria of not
> > hurting performance of the single-buffer per packet design.
> >
> > I've created a design document[2], that I will update based on our
> > discussions: [2]
> > https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
> >
> > The use-case that really convinced me was Eric's packet header-split.
> >
> >
> > Lets refresh: Why XDP don't have multi-buffer support:
> >
> > XDP is designed for maximum performance, which is why certain
> > driver-level
> > use-cases were not supported, like multi-buffer packets (like
> > jumbo-frames).
> > As it e.g. complicated the driver RX-loop and memory model handling.
> >
> > The single buffer per packet design, is also tied into eBPF
> > Direct-Access
> > (DA) to packet data, which can only be allowed if the packet memory is
> > in
> > contiguous memory. This DA feature is essential for XDP performance.
> >
> >
> > One way forward is to define that XDP only get access to the first
> > packet buffer, and it cannot see subsequent buffers. For XDP_TX and
> > XDP_REDIRECT to work then XDP still need to carry pointers (plus
> > len+offset) to the other buffers, which is 16 bytes per extra buffer.
>
>
> I’ve seen various network processor HW designs, and they normally get
> the first x bytes (128 - 512) which they can manipulate
> (append/prepend/insert/modify/delete).
Good data point, thank you! It confirms that XDP only getting access to
the first packet-buffer makes sense, for most use-cases.
We also have to remember that XDP it not meant to handle every
use-case. XDP is a software fast-path, that can accelerate certain
use-case. We have the existing network stack as a fall-back for
handling the corner-cases, that would otherwise slowdown our XDP
fast-path.
> There are designs where they can “page in” the additional fragments
> but it’s expensive as it requires additional memory transfers. But
> the majority do not care (cannot change) the remaining fragments. Can
> also not think of a reason why you might want to remove something at
> the end of the frame (thinking about routing/forwarding needs here).
Use-cases that need to adjust tail of packet:
- ICMP replies directly from XDP[1] need to shorten packet tail, but
this use-case doesn't use fragments.
- IPsec need to add/extend packet tail for IPset-trailer[2], again
unlikely that this needs fragments(?). (This use-case convinced me
that we need to add extend-tail support to bpf_xdp_adjust_tail)
- DNS or memcached replies directly from XDP, need to extend packet
tail, to have room for reply. (It would be interesting to allow larger
replies, but I'm not sure we should ever support that).
> If we do want XDP to access other fragments we could do this through
> a helper which swaps the packet context?
That might be a way forward. If the XDP developer have to call a
helper, then they should realize and "buy into" an additional
overhead/cost.
[1] https://github.com/torvalds/linux/blob/master/samples/bpf/xdp_adjust_tail_kern.c
[2] http://vger.kernel.org/netconf2019_files/xfrm_xdp.pdf
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH bpf-next v5 2/3] bpf_xdp_redirect_map: Perform map lookup in eBPF helper
From: Toke Høiland-Jørgensen @ 2019-06-28 8:18 UTC (permalink / raw)
To: Daniel Borkmann, netdev
Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
Jonathan Lemon
In-Reply-To: <d3f76a90-5cf4-4437-e3e1-75fda1248e53@iogearbox.net>
Daniel Borkmann <daniel@iogearbox.net> writes:
> On 06/28/2019 09:17 AM, Toke Høiland-Jørgensen wrote:
>> Daniel Borkmann <daniel@iogearbox.net> writes:
>>
>>> On 06/23/2019 04:17 AM, Toke Høiland-Jørgensen wrote:
>>>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>>>>
>>>> The bpf_redirect_map() helper used by XDP programs doesn't return any
>>>> indication of whether it can successfully redirect to the map index it was
>>>> given. Instead, BPF programs have to track this themselves, leading to
>>>> programs using duplicate maps to track which entries are populated in the
>>>> devmap.
>>>>
>>>> This patch fixes this by moving the map lookup into the bpf_redirect_map()
>>>> helper, which makes it possible to return failure to the eBPF program. The
>>>> lower bits of the flags argument is used as the return code, which means
>>>> that existing users who pass a '0' flag argument will get XDP_ABORTED.
>>>>
>>>> With this, a BPF program can check the return code from the helper call and
>>>> react by, for instance, substituting a different redirect. This works for
>>>> any type of map used for redirect.
>>>>
>>>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>>>
>>> Overall series looks good to me. Just very small things inline here & in the
>>> other two patches:
>>>
>>> [...]
>>>> @@ -3750,9 +3742,16 @@ BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex,
>>>> {
>>>> struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
>>>>
>>>> - if (unlikely(flags))
>>>> + /* Lower bits of the flags are used as return code on lookup failure */
>>>> + if (unlikely(flags > XDP_TX))
>>>> return XDP_ABORTED;
>>>>
>>>> + ri->item = __xdp_map_lookup_elem(map, ifindex);
>>>> + if (unlikely(!ri->item)) {
>>>> + WRITE_ONCE(ri->map, NULL);
>>>
>>> This WRITE_ONCE() is not needed. We never set it before at this point.
>>
>> You mean the WRITE_ONCE() wrapper is not needed, or the set-to-NULL is
>> not needed? The reason I added it is in case an eBPF program calls the
>> helper twice before returning, where the first lookup succeeds but the
>> second fails; in that case we want to clear the ->map pointer, no?
>
> Yeah I meant the set-to-NULL. So if first call succeeds, and the second one
> fails, then the expected semantics wrt the first call are as if the program
> would have called bpf_xdp_redirect() only?
>
> Looking at the code again, if we set ri->item to NULL, then we /must/
> also set ri->map to NULL. I guess there are two options: i) leave as
> is, ii) keep the __xdp_map_lookup_elem() result in a temp var, if it's
> NULL return flags, otherwise only /then/ update ri->item, so that
> semantics are similar to the invalid flags check earlier. I guess fine
> either way, in case of i) there should probably be a comment since
> it's less obvious.
Yeah, I think a temp var is probably clearer, will do that :)
-Toke
^ permalink raw reply
* Re: [PATCH bpf-next v5 2/3] bpf_xdp_redirect_map: Perform map lookup in eBPF helper
From: Daniel Borkmann @ 2019-06-28 8:12 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, netdev
Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
Jonathan Lemon
In-Reply-To: <874l4a9o2b.fsf@toke.dk>
On 06/28/2019 09:17 AM, Toke Høiland-Jørgensen wrote:
> Daniel Borkmann <daniel@iogearbox.net> writes:
>
>> On 06/23/2019 04:17 AM, Toke Høiland-Jørgensen wrote:
>>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>>>
>>> The bpf_redirect_map() helper used by XDP programs doesn't return any
>>> indication of whether it can successfully redirect to the map index it was
>>> given. Instead, BPF programs have to track this themselves, leading to
>>> programs using duplicate maps to track which entries are populated in the
>>> devmap.
>>>
>>> This patch fixes this by moving the map lookup into the bpf_redirect_map()
>>> helper, which makes it possible to return failure to the eBPF program. The
>>> lower bits of the flags argument is used as the return code, which means
>>> that existing users who pass a '0' flag argument will get XDP_ABORTED.
>>>
>>> With this, a BPF program can check the return code from the helper call and
>>> react by, for instance, substituting a different redirect. This works for
>>> any type of map used for redirect.
>>>
>>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>>
>> Overall series looks good to me. Just very small things inline here & in the
>> other two patches:
>>
>> [...]
>>> @@ -3750,9 +3742,16 @@ BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex,
>>> {
>>> struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
>>>
>>> - if (unlikely(flags))
>>> + /* Lower bits of the flags are used as return code on lookup failure */
>>> + if (unlikely(flags > XDP_TX))
>>> return XDP_ABORTED;
>>>
>>> + ri->item = __xdp_map_lookup_elem(map, ifindex);
>>> + if (unlikely(!ri->item)) {
>>> + WRITE_ONCE(ri->map, NULL);
>>
>> This WRITE_ONCE() is not needed. We never set it before at this point.
>
> You mean the WRITE_ONCE() wrapper is not needed, or the set-to-NULL is
> not needed? The reason I added it is in case an eBPF program calls the
> helper twice before returning, where the first lookup succeeds but the
> second fails; in that case we want to clear the ->map pointer, no?
Yeah I meant the set-to-NULL. So if first call succeeds, and the second one
fails, then the expected semantics wrt the first call are as if the program
would have called bpf_xdp_redirect() only?
Looking at the code again, if we set ri->item to NULL, then we /must/ also
set ri->map to NULL. I guess there are two options: i) leave as is, ii) keep
the __xdp_map_lookup_elem() result in a temp var, if it's NULL return flags,
otherwise only /then/ update ri->item, so that semantics are similar to the
invalid flags check earlier. I guess fine either way, in case of i) there
should probably be a comment since it's less obvious.
Thanks,
Daniel
^ permalink raw reply
* [PATCH bpf v6 2/2] xdp: fix hang while unregistering device bound to xdp socket
From: Ilya Maximets @ 2019-06-28 8:04 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, bpf, xdp-newbies, David S. Miller,
Björn Töpel, Magnus Karlsson, Jonathan Lemon,
Jakub Kicinski, Daniel Borkmann, Ilya Maximets
In-Reply-To: <20190628080407.30354-1-i.maximets@samsung.com>
Device that bound to XDP socket will not have zero refcount until the
userspace application will not close it. This leads to hang inside
'netdev_wait_allrefs()' if device unregistering requested:
# ip link del p1
< hang on recvmsg on netlink socket >
# ps -x | grep ip
5126 pts/0 D+ 0:00 ip link del p1
# journalctl -b
Jun 05 07:19:16 kernel:
unregister_netdevice: waiting for p1 to become free. Usage count = 1
Jun 05 07:19:27 kernel:
unregister_netdevice: waiting for p1 to become free. Usage count = 1
...
Fix that by implementing NETDEV_UNREGISTER event notification handler
to properly clean up all the resources and unref device.
This should also allow socket killing via ss(8) utility.
Fixes: 965a99098443 ("xsk: add support for bind for Rx")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
include/net/xdp_sock.h | 5 +++
net/xdp/xdp_umem.c | 10 ++---
net/xdp/xdp_umem.h | 1 +
net/xdp/xsk.c | 87 ++++++++++++++++++++++++++++++++++++------
4 files changed, 87 insertions(+), 16 deletions(-)
diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index d074b6d60f8a..82d153a637c7 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -61,6 +61,11 @@ struct xdp_sock {
struct xsk_queue *tx ____cacheline_aligned_in_smp;
struct list_head list;
bool zc;
+ enum {
+ XSK_READY = 0,
+ XSK_BOUND,
+ XSK_UNBOUND,
+ } state;
/* Protects multiple processes in the control path */
struct mutex mutex;
/* Mutual exclusion of NAPI TX thread and sendmsg error paths
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 267b82a4cbcf..20c91f02d3d8 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -140,11 +140,13 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
return err;
}
-static void xdp_umem_clear_dev(struct xdp_umem *umem)
+void xdp_umem_clear_dev(struct xdp_umem *umem)
{
struct netdev_bpf bpf;
int err;
+ ASSERT_RTNL();
+
if (!umem->dev)
return;
@@ -153,17 +155,13 @@ static void xdp_umem_clear_dev(struct xdp_umem *umem)
bpf.xsk.umem = NULL;
bpf.xsk.queue_id = umem->queue_id;
- rtnl_lock();
err = umem->dev->netdev_ops->ndo_bpf(umem->dev, &bpf);
- rtnl_unlock();
if (err)
WARN(1, "failed to disable umem!\n");
}
- rtnl_lock();
xdp_clear_umem_at_qid(umem->dev, umem->queue_id);
- rtnl_unlock();
dev_put(umem->dev);
umem->dev = NULL;
@@ -195,7 +193,9 @@ static void xdp_umem_unaccount_pages(struct xdp_umem *umem)
static void xdp_umem_release(struct xdp_umem *umem)
{
+ rtnl_lock();
xdp_umem_clear_dev(umem);
+ rtnl_unlock();
ida_simple_remove(&umem_ida, umem->id);
diff --git a/net/xdp/xdp_umem.h b/net/xdp/xdp_umem.h
index 27603227601b..a63a9fb251f5 100644
--- a/net/xdp/xdp_umem.h
+++ b/net/xdp/xdp_umem.h
@@ -10,6 +10,7 @@
int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
u16 queue_id, u16 flags);
+void xdp_umem_clear_dev(struct xdp_umem *umem);
bool xdp_umem_validate_queues(struct xdp_umem *umem);
void xdp_get_umem(struct xdp_umem *umem);
void xdp_put_umem(struct xdp_umem *umem);
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index a14e8864e4fa..336723948a36 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -335,6 +335,22 @@ static int xsk_init_queue(u32 entries, struct xsk_queue **queue,
return 0;
}
+static void xsk_unbind_dev(struct xdp_sock *xs)
+{
+ struct net_device *dev = xs->dev;
+
+ if (!dev || xs->state != XSK_BOUND)
+ return;
+
+ xs->state = XSK_UNBOUND;
+
+ /* Wait for driver to stop using the xdp socket. */
+ xdp_del_sk_umem(xs->umem, xs);
+ xs->dev = NULL;
+ synchronize_net();
+ dev_put(dev);
+}
+
static int xsk_release(struct socket *sock)
{
struct sock *sk = sock->sk;
@@ -354,15 +370,7 @@ static int xsk_release(struct socket *sock)
sock_prot_inuse_add(net, sk->sk_prot, -1);
local_bh_enable();
- if (xs->dev) {
- struct net_device *dev = xs->dev;
-
- /* Wait for driver to stop using the xdp socket. */
- xdp_del_sk_umem(xs->umem, xs);
- xs->dev = NULL;
- synchronize_net();
- dev_put(dev);
- }
+ xsk_unbind_dev(xs);
xskq_destroy(xs->rx);
xskq_destroy(xs->tx);
@@ -412,7 +420,7 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
return -EINVAL;
mutex_lock(&xs->mutex);
- if (xs->dev) {
+ if (xs->state != XSK_READY) {
err = -EBUSY;
goto out_release;
}
@@ -492,6 +500,8 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
out_unlock:
if (err)
dev_put(dev);
+ else
+ xs->state = XSK_BOUND;
out_release:
mutex_unlock(&xs->mutex);
return err;
@@ -520,6 +530,10 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
return -EFAULT;
mutex_lock(&xs->mutex);
+ if (xs->state != XSK_READY) {
+ mutex_unlock(&xs->mutex);
+ return -EBUSY;
+ }
q = (optname == XDP_TX_RING) ? &xs->tx : &xs->rx;
err = xsk_init_queue(entries, q, false);
mutex_unlock(&xs->mutex);
@@ -534,7 +548,7 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
return -EFAULT;
mutex_lock(&xs->mutex);
- if (xs->umem) {
+ if (xs->state != XSK_READY || xs->umem) {
mutex_unlock(&xs->mutex);
return -EBUSY;
}
@@ -561,6 +575,10 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
return -EFAULT;
mutex_lock(&xs->mutex);
+ if (xs->state != XSK_READY) {
+ mutex_unlock(&xs->mutex);
+ return -EBUSY;
+ }
if (!xs->umem) {
mutex_unlock(&xs->mutex);
return -EINVAL;
@@ -662,6 +680,9 @@ static int xsk_mmap(struct file *file, struct socket *sock,
unsigned long pfn;
struct page *qpg;
+ if (xs->state != XSK_READY)
+ return -EBUSY;
+
if (offset == XDP_PGOFF_RX_RING) {
q = READ_ONCE(xs->rx);
} else if (offset == XDP_PGOFF_TX_RING) {
@@ -693,6 +714,38 @@ static int xsk_mmap(struct file *file, struct socket *sock,
size, vma->vm_page_prot);
}
+static int xsk_notifier(struct notifier_block *this,
+ unsigned long msg, void *ptr)
+{
+ struct net_device *dev = netdev_notifier_info_to_dev(ptr);
+ struct net *net = dev_net(dev);
+ struct sock *sk;
+
+ switch (msg) {
+ case NETDEV_UNREGISTER:
+ mutex_lock(&net->xdp.lock);
+ sk_for_each(sk, &net->xdp.list) {
+ struct xdp_sock *xs = xdp_sk(sk);
+
+ mutex_lock(&xs->mutex);
+ if (xs->dev == dev) {
+ sk->sk_err = ENETDOWN;
+ if (!sock_flag(sk, SOCK_DEAD))
+ sk->sk_error_report(sk);
+
+ xsk_unbind_dev(xs);
+
+ /* Clear device references in umem. */
+ xdp_umem_clear_dev(xs->umem);
+ }
+ mutex_unlock(&xs->mutex);
+ }
+ mutex_unlock(&net->xdp.lock);
+ break;
+ }
+ return NOTIFY_DONE;
+}
+
static struct proto xsk_proto = {
.name = "XDP",
.owner = THIS_MODULE,
@@ -764,6 +817,7 @@ static int xsk_create(struct net *net, struct socket *sock, int protocol,
sock_set_flag(sk, SOCK_RCU_FREE);
xs = xdp_sk(sk);
+ xs->state = XSK_READY;
mutex_init(&xs->mutex);
spin_lock_init(&xs->tx_completion_lock);
@@ -784,6 +838,10 @@ static const struct net_proto_family xsk_family_ops = {
.owner = THIS_MODULE,
};
+static struct notifier_block xsk_netdev_notifier = {
+ .notifier_call = xsk_notifier,
+};
+
static int __net_init xsk_net_init(struct net *net)
{
mutex_init(&net->xdp.lock);
@@ -816,8 +874,15 @@ static int __init xsk_init(void)
err = register_pernet_subsys(&xsk_net_ops);
if (err)
goto out_sk;
+
+ err = register_netdevice_notifier(&xsk_netdev_notifier);
+ if (err)
+ goto out_pernet;
+
return 0;
+out_pernet:
+ unregister_pernet_subsys(&xsk_net_ops);
out_sk:
sock_unregister(PF_XDP);
out_proto:
--
2.17.1
^ permalink raw reply related
* [PATCH bpf v6 1/2] xdp: hold device for umem regardless of zero-copy mode
From: Ilya Maximets @ 2019-06-28 8:04 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, bpf, xdp-newbies, David S. Miller,
Björn Töpel, Magnus Karlsson, Jonathan Lemon,
Jakub Kicinski, Daniel Borkmann, Ilya Maximets
In-Reply-To: <20190628080407.30354-1-i.maximets@samsung.com>
Device pointer stored in umem regardless of zero-copy mode,
so we heed to hold the device in all cases.
Fixes: c9b47cc1fabc ("xsk: fix bug when trying to use both copy and zero-copy on one queue id")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
net/xdp/xdp_umem.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 9c6de4f114f8..267b82a4cbcf 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -105,6 +105,9 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
umem->dev = dev;
umem->queue_id = queue_id;
+
+ dev_hold(dev);
+
if (force_copy)
/* For copy-mode, we are done. */
goto out_rtnl_unlock;
@@ -124,7 +127,6 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
goto err_unreg_umem;
rtnl_unlock();
- dev_hold(dev);
umem->zc = true;
return 0;
@@ -163,10 +165,9 @@ static void xdp_umem_clear_dev(struct xdp_umem *umem)
xdp_clear_umem_at_qid(umem->dev, umem->queue_id);
rtnl_unlock();
- if (umem->zc) {
- dev_put(umem->dev);
- umem->zc = false;
- }
+ dev_put(umem->dev);
+ umem->dev = NULL;
+ umem->zc = false;
}
static void xdp_umem_unpin_pages(struct xdp_umem *umem)
--
2.17.1
^ permalink raw reply related
* [PATCH bpf v6 0/2] xdp: fix hang while unregistering device bound to xdp socket
From: Ilya Maximets @ 2019-06-28 8:04 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, bpf, xdp-newbies, David S. Miller,
Björn Töpel, Magnus Karlsson, Jonathan Lemon,
Jakub Kicinski, Daniel Borkmann, Ilya Maximets
In-Reply-To: <CGME20190628080413eucas1p13f3400f657b4827414737af42f02a57b@eucas1p1.samsung.com>
Version 6:
* Better names for socket state.
Version 5:
* Fixed incorrect handling of rtnl_lock.
Version 4:
* 'xdp_umem_clear_dev' exposed to be used while unregistering.
* Added XDP socket state to track if resources already unbinded.
* Splitted in two fixes.
Version 3:
* Declaration lines ordered from longest to shortest.
* Checking of event type moved to the top to avoid unnecessary
locking.
Version 2:
* Completely re-implemented using netdev event handler.
Ilya Maximets (2):
xdp: hold device for umem regardless of zero-copy mode
xdp: fix hang while unregistering device bound to xdp socket
include/net/xdp_sock.h | 5 +++
net/xdp/xdp_umem.c | 21 +++++-----
net/xdp/xdp_umem.h | 1 +
net/xdp/xsk.c | 87 ++++++++++++++++++++++++++++++++++++------
4 files changed, 93 insertions(+), 21 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH bpf v5 2/2] xdp: fix hang while unregistering device bound to xdp socket
From: Ilya Maximets @ 2019-06-28 8:01 UTC (permalink / raw)
To: Jonathan Lemon
Cc: netdev, linux-kernel, bpf, xdp-newbies, David S. Miller,
Björn Töpel, Magnus Karlsson, Jakub Kicinski,
Daniel Borkmann
In-Reply-To: <74C6C13C-651D-4CD1-BCA1-1B8998A4FA31@gmail.com>
On 28.06.2019 1:04, Jonathan Lemon wrote:
> On 27 Jun 2019, at 3:15, Ilya Maximets wrote:
>
>> Device that bound to XDP socket will not have zero refcount until the
>> userspace application will not close it. This leads to hang inside
>> 'netdev_wait_allrefs()' if device unregistering requested:
>>
>> # ip link del p1
>> < hang on recvmsg on netlink socket >
>>
>> # ps -x | grep ip
>> 5126 pts/0 D+ 0:00 ip link del p1
>>
>> # journalctl -b
>>
>> Jun 05 07:19:16 kernel:
>> unregister_netdevice: waiting for p1 to become free. Usage count = 1
>>
>> Jun 05 07:19:27 kernel:
>> unregister_netdevice: waiting for p1 to become free. Usage count = 1
>> ...
>>
>> Fix that by implementing NETDEV_UNREGISTER event notification handler
>> to properly clean up all the resources and unref device.
>>
>> This should also allow socket killing via ss(8) utility.
>>
>> Fixes: 965a99098443 ("xsk: add support for bind for Rx")
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>> include/net/xdp_sock.h | 5 +++
>> net/xdp/xdp_umem.c | 10 ++---
>> net/xdp/xdp_umem.h | 1 +
>> net/xdp/xsk.c | 87 ++++++++++++++++++++++++++++++++++++------
>> 4 files changed, 87 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
>> index d074b6d60f8a..82d153a637c7 100644
>> --- a/include/net/xdp_sock.h
>> +++ b/include/net/xdp_sock.h
>> @@ -61,6 +61,11 @@ struct xdp_sock {
>> struct xsk_queue *tx ____cacheline_aligned_in_smp;
>> struct list_head list;
>> bool zc;
>> + enum {
>> + XSK_UNINITIALIZED = 0,
>> + XSK_BINDED,
>> + XSK_UNBINDED,
>> + } state;
>
> I'd prefer that these were named better, perhaps:
> XSK_READY,
> XSK_BOUND,
> XSK_UNBOUND,
Sure. Thanks for suggestion!
>
> Other than that:
> Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
>
I'll send a new version with the new state names keeping your ACK.
Best regards, Ilya Maximets.
^ permalink raw reply
* Re: XDP multi-buffer incl. jumbo-frames (Was: [RFC V1 net-next 1/1] net: ena: implement XDP drop support)
From: Toke Høiland-Jørgensen @ 2019-06-28 7:46 UTC (permalink / raw)
To: Eelco Chaudron, Jesper Dangaard Brouer
Cc: Machulsky, Zorik, Jubran, Samih, davem, netdev, Woodhouse, David,
Matushevsky, Alexander, Bshara, Saeed, Wilson, Matt,
Liguori, Anthony, Bshara, Nafea, Tzalik, Guy, Belgazal, Netanel,
Saidi, Ali, Herrenschmidt, Benjamin, Kiyanovski, Arthur,
Daniel Borkmann, Ilias Apalodimas, Alexei Starovoitov,
Jakub Kicinski, xdp-newbies
In-Reply-To: <CC99D6DE-5B6B-42F3-8D68-7F9AFF1712FF@redhat.com>
"Eelco Chaudron" <echaudro@redhat.com> writes:
> On 26 Jun 2019, at 10:38, Jesper Dangaard Brouer wrote:
>
>> On Tue, 25 Jun 2019 03:19:22 +0000
>> "Machulsky, Zorik" <zorik@amazon.com> wrote:
>>
>>> On 6/23/19, 7:21 AM, "Jesper Dangaard Brouer" <brouer@redhat.com>
>>> wrote:
>>>
>>> On Sun, 23 Jun 2019 10:06:49 +0300 <sameehj@amazon.com> wrote:
>>>
>>> > This commit implements the basic functionality of drop/pass
>>> logic in the
>>> > ena driver.
>>>
>>> Usually we require a driver to implement all the XDP return
>>> codes,
>>> before we accept it. But as Daniel and I discussed with Zorik
>>> during
>>> NetConf[1], we are going to make an exception and accept the
>>> driver
>>> if you also implement XDP_TX.
>>>
>>> As we trust that Zorik/Amazon will follow and implement
>>> XDP_REDIRECT
>>> later, given he/you wants AF_XDP support which requires
>>> XDP_REDIRECT.
>>>
>>> Jesper, thanks for your comments and very helpful discussion during
>>> NetConf! That's the plan, as we agreed. From our side I would like to
>>> reiterate again the importance of multi-buffer support by xdp frame.
>>> We would really prefer not to see our MTU shrinking because of xdp
>>> support.
>>
>> Okay we really need to make a serious attempt to find a way to support
>> multi-buffer packets with XDP. With the important criteria of not
>> hurting performance of the single-buffer per packet design.
>>
>> I've created a design document[2], that I will update based on our
>> discussions: [2]
>> https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
>>
>> The use-case that really convinced me was Eric's packet header-split.
>>
>>
>> Lets refresh: Why XDP don't have multi-buffer support:
>>
>> XDP is designed for maximum performance, which is why certain
>> driver-level
>> use-cases were not supported, like multi-buffer packets (like
>> jumbo-frames).
>> As it e.g. complicated the driver RX-loop and memory model handling.
>>
>> The single buffer per packet design, is also tied into eBPF
>> Direct-Access
>> (DA) to packet data, which can only be allowed if the packet memory is
>> in
>> contiguous memory. This DA feature is essential for XDP performance.
>>
>>
>> One way forward is to define that XDP only get access to the first
>> packet buffer, and it cannot see subsequent buffers. For XDP_TX and
>> XDP_REDIRECT to work then XDP still need to carry pointers (plus
>> len+offset) to the other buffers, which is 16 bytes per extra buffer.
>
>
> I’ve seen various network processor HW designs, and they normally get
> the first x bytes (128 - 512) which they can manipulate
> (append/prepend/insert/modify/delete).
>
> There are designs where they can “page in” the additional fragments
> but it’s expensive as it requires additional memory transfers. But the
> majority do not care (cannot change) the remaining fragments. Can also
> not think of a reason why you might want to remove something at the end
> of the frame (thinking about routing/forwarding needs here).
>
> If we do want XDP to access other fragments we could do this through a
> helper which swaps the packet context?
Yeah, I was also going to suggest a helper for that. It doesn't
necessarily need to swap the packet context, it could just return a new
pointer?
-Toke
^ permalink raw reply
* Re: [RFC] longer netdev names proposal
From: Jiri Pirko @ 2019-06-28 7:35 UTC (permalink / raw)
To: Dan Williams
Cc: Stephen Hemminger, Michal Kubecek, netdev, Andrew Lunn,
David Ahern, davem, jakub.kicinski, mlxsw
In-Reply-To: <e3188fc08a13ddff9ea5a200a69aa6adbd9278ed.camel@redhat.com>
Thu, Jun 27, 2019 at 09:35:27PM CEST, dcbw@redhat.com wrote:
>On Thu, 2019-06-27 at 12:20 -0700, Stephen Hemminger wrote:
>> On Thu, 27 Jun 2019 20:39:48 +0200
>> Michal Kubecek <mkubecek@suse.cz> wrote:
>>
>> > > $ ip li set dev enp3s0 alias "Onboard Ethernet"
>> > > # ip link show "Onboard Ethernet"
>> > > Device "Onboard Ethernet" does not exist.
>> > >
>> > > So it does not really appear to be an alias, it is a label. To be
>> > > truly useful, it needs to be more than a label, it needs to be a
>> > > real
>> > > alias which you can use.
>> >
>> > That's exactly what I meant: to be really useful, one should be
>> > able to
>> > use the alias(es) for setting device options, for adding routes, in
>> > netfilter rules etc.
>> >
>> > Michal
>>
>> The kernel doesn't enforce uniqueness of alias.
>
>Can we even enforce unique aliases/labels? Given that the kernel hasn't
>enforced that in the past there's a good possibility of breaking stuff
>if it started. (unfortunately)
Correct. I think that Michal's idea to introduce "real aliases" is very
intereting. However, the existing "alias" as we have it does not seem
right to be used. Also because of the UAPI. We have IFLA_IFALIAS which
is a single value. For "real aliases" we need nested array.
[...]
^ permalink raw reply
* [PATCH net-next v2 02/10] net: stmmac: Do not try to enable PHY EEE if MAC does not support it
From: Jose Abreu @ 2019-06-28 7:29 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>
Do not enable EEE feature in the PHY if MAC does not support it.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 91f24b63ea16..0b1900bf4e9e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -896,7 +896,7 @@ static void stmmac_mac_link_up(struct phylink_config *config,
struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
stmmac_mac_set(priv, priv->ioaddr, true);
- if (phy) {
+ if (phy && priv->dma_cap.eee) {
priv->eee_active = phy_init_eee(phy, 1) >= 0;
priv->eee_enabled = stmmac_eee_init(priv);
stmmac_set_eee_pls(priv, priv->hw, true);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 01/10] net: stmmac: dwxgmac: Enable EDMA by default
From: Jose Abreu @ 2019-06-28 7:29 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>
Enable the EDMA feature by default which gives higher performance.
Changes from v1:
- Do not use magic values (David)
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 4 ++++
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 2 ++
2 files changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index b8296eb41011..c9e802faabb5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -177,6 +177,10 @@
#define XGMAC_BLEN8 BIT(2)
#define XGMAC_BLEN4 BIT(1)
#define XGMAC_UNDEF BIT(0)
+#define XGMAC_TX_EDMA_CTRL 0x00003040
+#define XGMAC_TDPS GENMASK(29, 0)
+#define XGMAC_RX_EDMA_CTRL 0x00003044
+#define XGMAC_RDPS GENMASK(29, 0)
#define XGMAC_DMA_CH_CONTROL(x) (0x00003100 + (0x80 * (x)))
#define XGMAC_PBLx8 BIT(16)
#define XGMAC_DMA_CH_TX_CONTROL(x) (0x00003104 + (0x80 * (x)))
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 7861a938420a..42448973c36e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -122,6 +122,8 @@ static void dwxgmac2_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
}
writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
+ writel(XGMAC_TDPS, ioaddr + XGMAC_TX_EDMA_CTRL);
+ writel(XGMAC_RDPS, ioaddr + XGMAC_RX_EDMA_CTRL);
}
static void dwxgmac2_dma_rx_mode(void __iomem *ioaddr, int mode,
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 09/10] net: stmmac: Only disable interrupts if NAPI is scheduled
From: Jose Abreu @ 2019-06-28 7:29 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>
Only disable the interrupts if RX NAPI gets to be scheduled. Also,
schedule the TX NAPI only when the interrupts are disabled.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a629b3144a0d..3425d4dda03d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2053,12 +2053,12 @@ static int stmmac_napi_check(struct stmmac_priv *priv, u32 chan)
&priv->xstats, chan);
struct stmmac_channel *ch = &priv->channel[chan];
- if (status)
- status |= handle_rx | handle_tx;
-
if ((status & handle_rx) && (chan < priv->plat->rx_queues_to_use)) {
- stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
- napi_schedule_irqoff(&ch->rx_napi);
+ if (napi_schedule_prep(&ch->rx_napi)) {
+ stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
+ __napi_schedule_irqoff(&ch->rx_napi);
+ status |= handle_tx;
+ }
}
if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use))
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 07/10] net: stmmac: Enable support for > 32 Bits addressing in XGMAC
From: Jose Abreu @ 2019-06-28 7:29 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Andrew Lunn
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>
Currently, stmmac only supports 32 bits addressing for SKB. Enable the
support for upto 48 bits addressing in XGMAC core.
This avoids the use of bounce buffers and increases performance.
Changes from v1:
- Fallback to 32 bits in failure (Andrew)
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 1 +
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 +
.../net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 4 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 19 +++++++-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 55 +++++++++++++++++-----
5 files changed, 66 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 9e8f4dbdcc22..2403a65167b2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -351,6 +351,7 @@ struct dma_features {
unsigned int frpsel;
unsigned int frpbs;
unsigned int frpes;
+ unsigned int addr64;
};
/* GMAC TX FIFO is 8K, Rx FIFO is 16K */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index 29db9b538874..9a9792527530 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -87,6 +87,7 @@
#define XGMAC_HWFEAT_GMIISEL BIT(1)
#define XGMAC_HW_FEATURE1 0x00000120
#define XGMAC_HWFEAT_TSOEN BIT(18)
+#define XGMAC_HWFEAT_ADDR64 GENMASK(15, 14)
#define XGMAC_HWFEAT_TXFIFOSIZE GENMASK(10, 6)
#define XGMAC_HWFEAT_RXFIFOSIZE GENMASK(4, 0)
#define XGMAC_HW_FEATURE2 0x00000124
@@ -172,6 +173,7 @@
#define XGMAC_EN_LPI BIT(15)
#define XGMAC_LPI_XIT_PKT BIT(14)
#define XGMAC_AAL BIT(12)
+#define XGMAC_EAME BIT(11)
#define XGMAC_BLEN GENMASK(7, 1)
#define XGMAC_BLEN256 BIT(7)
#define XGMAC_BLEN128 BIT(6)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
index 98fa471da7c0..c4c45402b8f8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
@@ -242,8 +242,8 @@ static void dwxgmac2_get_addr(struct dma_desc *p, unsigned int *addr)
static void dwxgmac2_set_addr(struct dma_desc *p, dma_addr_t addr)
{
- p->des0 = cpu_to_le32(addr);
- p->des1 = 0;
+ p->des0 = cpu_to_le32(lower_32_bits(addr));
+ p->des1 = cpu_to_le32(upper_32_bits(addr));
}
static void dwxgmac2_clear(struct dma_desc *p)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index b739673f8842..229c58758cbd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -27,7 +27,7 @@ static void dwxgmac2_dma_init(void __iomem *ioaddr,
if (dma_cfg->aal)
value |= XGMAC_AAL;
- writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
+ writel(value | XGMAC_EAME, ioaddr + XGMAC_DMA_SYSBUS_MODE);
}
static void dwxgmac2_dma_init_chan(void __iomem *ioaddr,
@@ -361,6 +361,23 @@ static void dwxgmac2_get_hw_feature(void __iomem *ioaddr,
/* MAC HW feature 1 */
hw_cap = readl(ioaddr + XGMAC_HW_FEATURE1);
dma_cap->tsoen = (hw_cap & XGMAC_HWFEAT_TSOEN) >> 18;
+
+ dma_cap->addr64 = (hw_cap & XGMAC_HWFEAT_ADDR64) >> 14;
+ switch (dma_cap->addr64) {
+ case 0:
+ dma_cap->addr64 = 32;
+ break;
+ case 1:
+ dma_cap->addr64 = 40;
+ break;
+ case 2:
+ dma_cap->addr64 = 48;
+ break;
+ default:
+ dma_cap->addr64 = 32;
+ break;
+ }
+
dma_cap->tx_fifo_size =
128 << ((hw_cap & XGMAC_HWFEAT_TXFIFOSIZE) >> 6);
dma_cap->rx_fifo_size =
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e8f3e76889c8..620dd387e3b1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2772,7 +2772,7 @@ static int stmmac_release(struct net_device *dev)
* This function fills descriptor and request new descriptors according to
* buffer length to fill
*/
-static void stmmac_tso_allocator(struct stmmac_priv *priv, unsigned int des,
+static void stmmac_tso_allocator(struct stmmac_priv *priv, dma_addr_t des,
int total_len, bool last_segment, u32 queue)
{
struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
@@ -2783,11 +2783,18 @@ static void stmmac_tso_allocator(struct stmmac_priv *priv, unsigned int des,
tmp_len = total_len;
while (tmp_len > 0) {
+ dma_addr_t curr_addr;
+
tx_q->cur_tx = STMMAC_GET_ENTRY(tx_q->cur_tx, DMA_TX_SIZE);
WARN_ON(tx_q->tx_skbuff[tx_q->cur_tx]);
desc = tx_q->dma_tx + tx_q->cur_tx;
- desc->des0 = cpu_to_le32(des + (total_len - tmp_len));
+ curr_addr = des + (total_len - tmp_len);
+ if (priv->dma_cap.addr64 <= 32)
+ desc->des0 = cpu_to_le32(curr_addr);
+ else
+ stmmac_set_desc_addr(priv, desc, curr_addr);
+
buff_size = tmp_len >= TSO_MAX_BUFF_SIZE ?
TSO_MAX_BUFF_SIZE : tmp_len;
@@ -2833,11 +2840,12 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
struct stmmac_priv *priv = netdev_priv(dev);
int nfrags = skb_shinfo(skb)->nr_frags;
u32 queue = skb_get_queue_mapping(skb);
- unsigned int first_entry, des;
+ unsigned int first_entry;
struct stmmac_tx_queue *tx_q;
int tmp_pay_len = 0;
u32 pay_len, mss;
u8 proto_hdr_len;
+ dma_addr_t des;
int i;
tx_q = &priv->tx_queue[queue];
@@ -2894,14 +2902,19 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
tx_q->tx_skbuff_dma[first_entry].buf = des;
tx_q->tx_skbuff_dma[first_entry].len = skb_headlen(skb);
- first->des0 = cpu_to_le32(des);
+ if (priv->dma_cap.addr64 <= 32) {
+ first->des0 = cpu_to_le32(des);
- /* Fill start of payload in buff2 of first descriptor */
- if (pay_len)
- first->des1 = cpu_to_le32(des + proto_hdr_len);
+ /* Fill start of payload in buff2 of first descriptor */
+ if (pay_len)
+ first->des1 = cpu_to_le32(des + proto_hdr_len);
- /* If needed take extra descriptors to fill the remaining payload */
- tmp_pay_len = pay_len - TSO_MAX_BUFF_SIZE;
+ /* If needed take extra descriptors to fill the remaining payload */
+ tmp_pay_len = pay_len - TSO_MAX_BUFF_SIZE;
+ } else {
+ stmmac_set_desc_addr(priv, first, des);
+ tmp_pay_len = pay_len;
+ }
stmmac_tso_allocator(priv, des, tmp_pay_len, (nfrags == 0), queue);
@@ -3031,12 +3044,12 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
int i, csum_insertion = 0, is_jumbo = 0;
u32 queue = skb_get_queue_mapping(skb);
int nfrags = skb_shinfo(skb)->nr_frags;
- int entry;
- unsigned int first_entry;
struct dma_desc *desc, *first;
struct stmmac_tx_queue *tx_q;
+ unsigned int first_entry;
unsigned int enh_desc;
- unsigned int des;
+ dma_addr_t des;
+ int entry;
tx_q = &priv->tx_queue[queue];
@@ -4316,6 +4329,24 @@ int stmmac_dvr_probe(struct device *device,
priv->tso = true;
dev_info(priv->device, "TSO feature enabled\n");
}
+
+ if (priv->dma_cap.addr64) {
+ ret = dma_set_mask_and_coherent(device,
+ DMA_BIT_MASK(priv->dma_cap.addr64));
+ if (!ret) {
+ dev_info(priv->device, "Using %d bits DMA width\n",
+ priv->dma_cap.addr64);
+ } else {
+ ret = dma_set_mask_and_coherent(device, DMA_BIT_MASK(32));
+ if (ret) {
+ dev_err(priv->device, "Failed to set DMA Mask\n");
+ goto error_hw_init;
+ }
+
+ priv->dma_cap.addr64 = 32;
+ }
+ }
+
ndev->features |= ndev->hw_features | NETIF_F_HIGHDMA;
ndev->watchdog_timeo = msecs_to_jiffies(watchdog);
#ifdef STMMAC_VLAN_TAG_USED
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 03/10] net: stmmac: Decrease default RX Watchdog value
From: Jose Abreu @ 2019-06-28 7:29 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>
For performance reasons decrease the default RX Watchdog value for the
minimum allowed.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index ad9e9368535d..14d8f6d7cb9a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -246,7 +246,7 @@ struct stmmac_safety_stats {
/* Max/Min RI Watchdog Timer count value */
#define MAX_DMA_RIWT 0xff
-#define MIN_DMA_RIWT 0x20
+#define MIN_DMA_RIWT 0x10
/* Tx coalesce parameters */
#define STMMAC_COAL_TX_TIMER 1000
#define STMMAC_MAX_COAL_TX_TICK 100000
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 0b1900bf4e9e..40408a067400 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2516,9 +2516,9 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
priv->tx_lpi_timer = STMMAC_DEFAULT_TWT_LS;
if (priv->use_riwt) {
- ret = stmmac_rx_watchdog(priv, priv->ioaddr, MAX_DMA_RIWT, rx_cnt);
+ ret = stmmac_rx_watchdog(priv, priv->ioaddr, MIN_DMA_RIWT, rx_cnt);
if (!ret)
- priv->rx_riwt = MAX_DMA_RIWT;
+ priv->rx_riwt = MIN_DMA_RIWT;
}
if (priv->hw->pcs)
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 00/10] net: stmmac: 10GbE using XGMAC
From: Jose Abreu @ 2019-06-28 7:29 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
Support for 10Gb Link using XGMAC core plus some performance tweaks.
Tested in a PCI based setup.
iperf3 TCP results:
TSO ON, MTU=1500, TX Queues = 1, RX Queues = 1, Flow Control ON
Pinned CPU (-A), Zero-Copy (-Z)
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-600.00 sec 643 GBytes 9.21 Gbits/sec 1 sender
[ 5] 0.00-600.00 sec 643 GBytes 9.21 Gbits/sec receiver
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Jose Abreu (10):
net: stmmac: dwxgmac: Enable EDMA by default
net: stmmac: Do not try to enable PHY EEE if MAC does not support it
net: stmmac: Decrease default RX Watchdog value
net: stmmac: dwxgmac: Fix the undefined burst setting
net: stmmac: Add the missing speeds that XGMAC supports
net: stmmac: Do not disable interrupts when cleaning TX
net: stmmac: Enable support for > 32 Bits addressing in XGMAC
net: stmmac: Update RX Tail Pointer to last free entry
net: stmmac: Only disable interrupts if NAPI is scheduled
net: stmmac: Update Kconfig entry
drivers/net/ethernet/stmicro/stmmac/Kconfig | 2 +-
drivers/net/ethernet/stmicro/stmmac/common.h | 9 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 16 ++-
.../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 14 +-
.../net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 4 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 27 +++-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 160 ++++++++++++++++-----
7 files changed, 176 insertions(+), 56 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH net-next v2 04/10] net: stmmac: dwxgmac: Fix the undefined burst setting
From: Jose Abreu @ 2019-06-28 7:29 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>
Undefined burst shall only be set if pdata asks to.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 42448973c36e..b739673f8842 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -91,11 +91,11 @@ static void dwxgmac2_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
value |= (axi->axi_rd_osr_lmt << XGMAC_RD_OSR_LMT_SHIFT) &
XGMAC_RD_OSR_LMT;
+ if (!axi->axi_fb)
+ value |= XGMAC_UNDEF;
+
value &= ~XGMAC_BLEN;
for (i = 0; i < AXI_BLEN; i++) {
- if (axi->axi_blen[i])
- value &= ~XGMAC_UNDEF;
-
switch (axi->axi_blen[i]) {
case 256:
value |= XGMAC_BLEN256;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 05/10] net: stmmac: Add the missing speeds that XGMAC supports
From: Jose Abreu @ 2019-06-28 7:29 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>
XGMAC supports following speeds:
- 10G XGMII
- 5G XGMII
- 2.5G XGMII
- 2.5G GMII
- 1G GMII
- 100M MII
- 10M MII
Add them to the stmmac driver.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 6 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 10 ++-
.../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 14 ++--
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 79 ++++++++++++++++++----
4 files changed, 86 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 14d8f6d7cb9a..9e8f4dbdcc22 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -392,8 +392,12 @@ struct mac_link {
u32 speed100;
u32 speed1000;
u32 speed2500;
- u32 speed10000;
u32 duplex;
+ struct {
+ u32 speed2500;
+ u32 speed5000;
+ u32 speed10000;
+ } xgmii;
};
struct mii_regs {
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index c9e802faabb5..29db9b538874 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -15,10 +15,14 @@
/* MAC Registers */
#define XGMAC_TX_CONFIG 0x00000000
#define XGMAC_CONFIG_SS_OFF 29
-#define XGMAC_CONFIG_SS_MASK GENMASK(30, 29)
+#define XGMAC_CONFIG_SS_MASK GENMASK(31, 29)
#define XGMAC_CONFIG_SS_10000 (0x0 << XGMAC_CONFIG_SS_OFF)
-#define XGMAC_CONFIG_SS_2500 (0x2 << XGMAC_CONFIG_SS_OFF)
-#define XGMAC_CONFIG_SS_1000 (0x3 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_2500_GMII (0x2 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_1000_GMII (0x3 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_100_MII (0x4 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_5000 (0x5 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_2500 (0x6 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_10_MII (0x7 << XGMAC_CONFIG_SS_OFF)
#define XGMAC_CONFIG_SARC GENMASK(22, 20)
#define XGMAC_CONFIG_SARC_SHIFT 20
#define XGMAC_CONFIG_JD BIT(16)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index bfa7d6913fd4..0a32c96a7854 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -36,7 +36,7 @@ static void dwxgmac2_core_init(struct mac_device_info *hw,
switch (hw->ps) {
case SPEED_10000:
- tx |= hw->link.speed10000;
+ tx |= hw->link.xgmii.speed10000;
break;
case SPEED_2500:
tx |= hw->link.speed2500;
@@ -381,11 +381,13 @@ int dwxgmac2_setup(struct stmmac_priv *priv)
mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
mac->link.duplex = 0;
- mac->link.speed10 = 0;
- mac->link.speed100 = 0;
- mac->link.speed1000 = XGMAC_CONFIG_SS_1000;
- mac->link.speed2500 = XGMAC_CONFIG_SS_2500;
- mac->link.speed10000 = XGMAC_CONFIG_SS_10000;
+ mac->link.speed10 = XGMAC_CONFIG_SS_10_MII;
+ mac->link.speed100 = XGMAC_CONFIG_SS_100_MII;
+ mac->link.speed1000 = XGMAC_CONFIG_SS_1000_GMII;
+ mac->link.speed2500 = XGMAC_CONFIG_SS_2500_GMII;
+ mac->link.xgmii.speed2500 = XGMAC_CONFIG_SS_2500;
+ mac->link.xgmii.speed5000 = XGMAC_CONFIG_SS_5000;
+ mac->link.xgmii.speed10000 = XGMAC_CONFIG_SS_10000;
mac->link.speed_mask = XGMAC_CONFIG_SS_MASK;
mac->mii.addr = XGMAC_MDIO_ADDR;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 40408a067400..4a5941caaadc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -805,14 +805,43 @@ static void stmmac_validate(struct phylink_config *config,
struct phylink_link_state *state)
{
struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(mac_supported) = { 0, };
__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
int tx_cnt = priv->plat->tx_queues_to_use;
int max_speed = priv->plat->max_speed;
+ phylink_set(mac_supported, 10baseT_Half);
+ phylink_set(mac_supported, 10baseT_Full);
+ phylink_set(mac_supported, 100baseT_Half);
+ phylink_set(mac_supported, 100baseT_Full);
+
+ phylink_set(mac_supported, Autoneg);
+ phylink_set(mac_supported, Pause);
+ phylink_set(mac_supported, Asym_Pause);
+ phylink_set_port_modes(mac_supported);
+
+ if (priv->plat->has_gmac ||
+ priv->plat->has_gmac4 ||
+ priv->plat->has_xgmac) {
+ phylink_set(mac_supported, 1000baseT_Half);
+ phylink_set(mac_supported, 1000baseT_Full);
+ phylink_set(mac_supported, 1000baseKX_Full);
+ }
+
/* Cut down 1G if asked to */
if ((max_speed > 0) && (max_speed < 1000)) {
phylink_set(mask, 1000baseT_Full);
phylink_set(mask, 1000baseX_Full);
+ } else if (priv->plat->has_xgmac) {
+ phylink_set(mac_supported, 2500baseT_Full);
+ phylink_set(mac_supported, 5000baseT_Full);
+ phylink_set(mac_supported, 10000baseSR_Full);
+ phylink_set(mac_supported, 10000baseLR_Full);
+ phylink_set(mac_supported, 10000baseER_Full);
+ phylink_set(mac_supported, 10000baseLRM_Full);
+ phylink_set(mac_supported, 10000baseT_Full);
+ phylink_set(mac_supported, 10000baseKX4_Full);
+ phylink_set(mac_supported, 10000baseKR_Full);
}
/* Half-Duplex can only work with single queue */
@@ -822,7 +851,12 @@ static void stmmac_validate(struct phylink_config *config,
phylink_set(mask, 1000baseT_Half);
}
- bitmap_andnot(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS);
+ bitmap_and(supported, supported, mac_supported,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+ bitmap_andnot(supported, supported, mask,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+ bitmap_and(state->advertising, state->advertising, mac_supported,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
bitmap_andnot(state->advertising, state->advertising, mask,
__ETHTOOL_LINK_MODE_MASK_NBITS);
}
@@ -842,18 +876,37 @@ static void stmmac_mac_config(struct phylink_config *config, unsigned int mode,
ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
ctrl &= ~priv->hw->link.speed_mask;
- switch (state->speed) {
- case SPEED_1000:
- ctrl |= priv->hw->link.speed1000;
- break;
- case SPEED_100:
- ctrl |= priv->hw->link.speed100;
- break;
- case SPEED_10:
- ctrl |= priv->hw->link.speed10;
- break;
- default:
- return;
+ if (state->interface == PHY_INTERFACE_MODE_USXGMII) {
+ switch (state->speed) {
+ case SPEED_10000:
+ ctrl |= priv->hw->link.xgmii.speed10000;
+ break;
+ case SPEED_5000:
+ ctrl |= priv->hw->link.xgmii.speed5000;
+ break;
+ case SPEED_2500:
+ ctrl |= priv->hw->link.xgmii.speed2500;
+ break;
+ default:
+ return;
+ }
+ } else {
+ switch (state->speed) {
+ case SPEED_2500:
+ ctrl |= priv->hw->link.speed2500;
+ break;
+ case SPEED_1000:
+ ctrl |= priv->hw->link.speed1000;
+ break;
+ case SPEED_100:
+ ctrl |= priv->hw->link.speed100;
+ break;
+ case SPEED_10:
+ ctrl |= priv->hw->link.speed10;
+ break;
+ default:
+ return;
+ }
}
priv->speed = state->speed;
--
2.7.4
^ permalink raw reply related
* Re: [RFC] longer netdev names proposal
From: Jiri Pirko @ 2019-06-28 7:29 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, davem, jakub.kicinski, sthemmin, mlxsw
In-Reply-To: <26b73332-9ea0-9d2c-9185-9de522c72bb9@gmail.com>
Thu, Jun 27, 2019 at 07:14:31PM CEST, dsahern@gmail.com wrote:
>On 6/27/19 3:43 AM, Jiri Pirko wrote:
>> Hi all.
>>
>> In the past, there was repeatedly discussed the IFNAMSIZ (16) limit for
>> netdevice name length. Now when we have PF and VF representors
>> with port names like "pfXvfY", it became quite common to hit this limit:
>> 0123456789012345
>> enp131s0f1npf0vf6
>> enp131s0f1npf0vf22
>
>QinQ (stacked vlans) is another example.
There are more usecases for this, yes.
>
>>
>> Since IFLA_NAME is just a string, I though it might be possible to use
>> it to carry longer names as it is. However, the userspace tools, like
>> iproute2, are doing checks before print out. So for example in output of
>> "ip addr" when IFLA_NAME is longer than IFNAMSIZE, the netdevice is
>> completely avoided.
>>
>> So here is a proposal that might work:
>> 1) Add a new attribute IFLA_NAME_EXT that could carry names longer than
>> IFNAMSIZE, say 64 bytes. The max size should be only defined in kernel,
>> user should be prepared for any string size.
>> 2) Add a file in sysfs that would indicate that NAME_EXT is supported by
>> the kernel.
>
>no sysfs files.
>
>Johannes added infrastructure to retrieve the policy. That is a more
>flexible and robust option for determining what the kernel supports.
Sure, udev can query rtnetlink. I just proposed it as an option, anyway,
it's implementation detail.
>
>
>> 3) Udev is going to look for the sysfs indication file. In case when
>> kernel supports long names, it will do rename to longer name, setting
>> IFLA_NAME_EXT. If not, it does what it does now - fail.
>> 4) There are two cases that can happen during rename:
>> A) The name is shorter than IFNAMSIZ
>> -> both IFLA_NAME and IFLA_NAME_EXT would contain the same string:
>> original IFLA_NAME = eth0
>> original IFLA_NAME_EXT = eth0
>> renamed IFLA_NAME = enp5s0f1npf0vf1
>> renamed IFLA_NAME_EXT = enp5s0f1npf0vf1
>> B) The name is longer tha IFNAMSIZ
>> -> IFLA_NAME would contain the original one, IFLA_NAME_EXT would
>> contain the new one:
>> original IFLA_NAME = eth0
>> original IFLA_NAME_EXT = eth0
>> renamed IFLA_NAME = eth0
>> renamed IFLA_NAME_EXT = enp131s0f1npf0vf22
>
>so kernel side there will be 2 names for the same net_device?
Yes. However, updated tools (which would be eventually all) are going to
show only the ext one.
>
>>
>> This would allow the old tools to work with "eth0" and the new
>> tools would work with "enp131s0f1npf0vf22". In sysfs, there would
>> be symlink from one name to another.
>
>I would prefer a solution that does not rely on sysfs hooks.
Please note that this /sys/class/net/ifacename dirs are already created.
What I propose is to have symlink from ext to the short name or vice
versa. The solution really does not "rely" on this...
>
>>
>> Also, there might be a warning added to kernel if someone works
>> with IFLA_NAME that the userspace tool should be upgraded.
>
>that seems like spam and confusion for the first few years of a new api.
Spam? warn_once?
>
>>
>> Eventually, only IFLA_NAME_EXT is going to be used by everyone.
>>
>> I'm aware there are other places where similar new attribute
>> would have to be introduced too (ip rule for example).
>> I'm not saying this is a simple work.
>>
>> Question is what to do with the ioctl api (get ifindex etc). I would
>> probably leave it as is and push tools to use rtnetlink instead.
>
>The ioctl API is going to be a limiter here. ifconfig is still quite
>prevalent and net-snmp still uses ioctl (as just 2 common examples).
>snmp showing one set of names and rtnetlink s/w showing another is going
>to be really confusing.
I don't see other way though, do you? The ioctl names are unextendable :/
^ permalink raw reply
* [PATCH net-next v2 06/10] net: stmmac: Do not disable interrupts when cleaning TX
From: Jose Abreu @ 2019-06-28 7:29 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>
This is a performance killer and anyways the interrupts are being
disabled by RX NAPI so no need to disable them again.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4a5941caaadc..e8f3e76889c8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2061,10 +2061,8 @@ static int stmmac_napi_check(struct stmmac_priv *priv, u32 chan)
napi_schedule_irqoff(&ch->rx_napi);
}
- if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use)) {
- stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
+ if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use))
napi_schedule_irqoff(&ch->tx_napi);
- }
return status;
}
@@ -3570,8 +3568,8 @@ static int stmmac_napi_poll_tx(struct napi_struct *napi, int budget)
work_done = stmmac_tx_clean(priv, DMA_TX_SIZE, chan);
work_done = min(work_done, budget);
- if (work_done < budget && napi_complete_done(napi, work_done))
- stmmac_enable_dma_irq(priv, priv->ioaddr, chan);
+ if (work_done < budget)
+ napi_complete_done(napi, work_done);
/* Force transmission restart */
tx_q = &priv->tx_queue[chan];
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 10/10] net: stmmac: Update Kconfig entry
From: Jose Abreu @ 2019-06-28 7:29 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>
We support more speeds now. Update the Kconfig entry.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index c43e2da4e7e3..2acb999b7f63 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
config STMMAC_ETH
- tristate "STMicroelectronics 10/100/1000/EQOS Ethernet driver"
+ tristate "STMicroelectronics 10/100/1000/EQOS/2500/5000/10000 Ethernet driver"
depends on HAS_IOMEM && HAS_DMA
select MII
select PHYLINK
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 08/10] net: stmmac: Update RX Tail Pointer to last free entry
From: Jose Abreu @ 2019-06-28 7:29 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>
Update the RX Tail Pointer to the last available SKB entry.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 620dd387e3b1..a629b3144a0d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3347,6 +3347,8 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
}
rx_q->dirty_rx = entry;
+ rx_q->rx_tail_addr = rx_q->dma_rx_phy +
+ (rx_q->dirty_rx * sizeof(struct dma_desc));
stmmac_set_rx_tail_ptr(priv, priv->ioaddr, rx_q->rx_tail_addr, queue);
}
--
2.7.4
^ permalink raw reply related
* RE: [PATCH net-next 10/10] net: stmmac: Try to get C45 PHY if everything else fails
From: Jose Abreu @ 2019-06-28 7:27 UTC (permalink / raw)
To: Andrew Lunn, Jose Abreu
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <20190627140316.GF31189@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
> On Thu, Jun 27, 2019 at 01:33:59PM +0000, Jose Abreu wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> >
> > > There have been some drivers gaining patches for ACPI. That is
> > > probably the better long term solution, ask ACPI where is the PHY and
> > > what MDIO protocol to use to talk to it.
> >
> > Hmmm, I'm not sure this is going to work that way ...
> >
> > My setup is a PCI EP which is hot-pluggable and as far as I know ACPI
> > has only static content (????)
>
> I've wanted to improve the PHY probe code for a while. I was thinking
> we should add a flag to the MDIO bus driver structure indicating it
> can do C45. When that flag is present, we should also scan the bus for
> C45 devices, and register them as well.
>
> With that in place, i think your problem goes away. Architecturally, i
> think it is wrong that a MAC driver is registering PHY devices. The
> MDIO core should do this.
Ok, I will drop this patch from the series until we come up with a
better solution.
^ permalink raw reply
* [PATCH net-next v2] net: stmmac: Fix case when PHY handle is not present
From: Jose Abreu @ 2019-06-28 7:25 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Sergei Shtylyov
Some DT bindings do not have the PHY handle. Let's fallback to manually
discovery in case phylink_of_phy_connect() fails.
Changes from v1:
- Fixup comment style (Sergei)
Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
Reported-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
Tested-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 8f5ebd51859e..91f24b63ea16 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -953,9 +953,13 @@ static int stmmac_init_phy(struct net_device *dev)
node = priv->plat->phylink_node;
- if (node) {
+ if (node)
ret = phylink_of_phy_connect(priv->phylink, node, 0);
- } else {
+
+ /* Some DT bindings do not set-up the PHY handle. Let's try to
+ * manually parse it
+ */
+ if (!node || ret) {
int addr = priv->plat->phy_addr;
struct phy_device *phydev;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH bpf-next v5 2/3] bpf_xdp_redirect_map: Perform map lookup in eBPF helper
From: Toke Høiland-Jørgensen @ 2019-06-28 7:23 UTC (permalink / raw)
To: Daniel Borkmann, netdev
Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
Jonathan Lemon
In-Reply-To: <d0d5dbe3-d2bf-2284-b2a3-667c77487125@iogearbox.net>
Daniel Borkmann <daniel@iogearbox.net> writes:
> On 06/23/2019 04:17 AM, Toke Høiland-Jørgensen wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>>
>> The bpf_redirect_map() helper used by XDP programs doesn't return any
>> indication of whether it can successfully redirect to the map index it was
>> given. Instead, BPF programs have to track this themselves, leading to
>> programs using duplicate maps to track which entries are populated in the
>> devmap.
>>
>> This patch fixes this by moving the map lookup into the bpf_redirect_map()
>> helper, which makes it possible to return failure to the eBPF program. The
>> lower bits of the flags argument is used as the return code, which means
>> that existing users who pass a '0' flag argument will get XDP_ABORTED.
>>
>> With this, a BPF program can check the return code from the helper call and
>> react by, for instance, substituting a different redirect. This works for
>> any type of map used for redirect.
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> [...]
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 183bf4d8e301..a6779e1cc1b8 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -3605,17 +3605,13 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
>> struct bpf_redirect_info *ri)
>> {
>> u32 index = ri->ifindex;
>> - void *fwd = NULL;
>> + void *fwd = ri->item;
>> int err;
>>
>> ri->ifindex = 0;
>> + ri->item = NULL;
>> WRITE_ONCE(ri->map, NULL);
>>
>> - fwd = __xdp_map_lookup_elem(map, index);
>> - if (unlikely(!fwd)) {
>> - err = -EINVAL;
>> - goto err;
>> - }
>
> If you look at the _trace_xdp_redirect{,_err}(), we should also get rid of the
> extra NULL test in devmap_ifindex() which is not under tracepoint static key.
ACK, will add.
-Toke
^ permalink raw reply
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