* Re: Removing skb_orphan() from ip_rcv_core()
From: Eric Dumazet @ 2019-06-25 6:37 UTC (permalink / raw)
To: Joe Stringer, Florian Westphal
Cc: netdev, john fastabend, Daniel Borkmann, Lorenz Bauer,
Jakub Sitnicki, Paolo Abeni
In-Reply-To: <CAOftzPi5SO_tZeoEs1Apd5np=Sd2fFUPm1oome_31=rMqSD-=g@mail.gmail.com>
On 6/24/19 8:17 PM, Joe Stringer wrote:
> On Fri, Jun 21, 2019 at 1:59 PM Florian Westphal <fw@strlen.de> wrote:
>>
>> Joe Stringer <joe@wand.net.nz> wrote:
>>> As discussed during LSFMM, I've been looking at adding something like
>>> an `skb_sk_assign()` helper to BPF so that logic similar to TPROXY can
>>> be implemented with integration into other BPF logic, however
>>> currently any attempts to do so are blocked by the skb_orphan() call
>>> in ip_rcv_core() (which will effectively ignore any socket assign
>>> decision made by the TC BPF program).
>>>
>>> Recently I was attempting to remove the skb_orphan() call, and I've
>>> been trying different things but there seems to be some context I'm
>>> missing. Here's the core of the patch:
>>>
>>> diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
>>> index ed97724c5e33..16aea980318a 100644
>>> --- a/net/ipv4/ip_input.c
>>> +++ b/net/ipv4/ip_input.c
>>> @@ -500,8 +500,6 @@ static struct sk_buff *ip_rcv_core(struct sk_buff
>>> *skb, struct net *net)
>>> memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
>>> IPCB(skb)->iif = skb->skb_iif;
>>>
>>> - /* Must drop socket now because of tproxy. */
>>> - skb_orphan(skb);
>>>
>>> return skb;
>>>
>>> The statement that the socket must be dropped because of tproxy
>>> doesn't make sense to me, because the PRE_ROUTING hook is hit after
>>> this, which will call into the tproxy logic and eventually
>>> nf_tproxy_assign_sock() which already does the skb_orphan() itself.
>>
>> in comment: s/tproxy/skb_steal_sock/
>
> For reference, I was following the path like this:
>
> ip_rcv()
> ( -> ip_rcv_core() for skb_orphan)
> -> NF_INET_PRE_ROUTING hook
> (... invoke iptables hooks)
> -> iptable_mangle_hook()
> -> ipt_do_table()
> ... -> tproxy_tg4()
> ... -> nf_tproxy_assign_sock()
> -> skb_orphan()
> (... finish iptables processing)
> ( -> ip_rcv_finish())
> ( ... -> ip_rcv_finish_core() for early demux / route lookup )
> (... -> dst_input())
> (... -> tcp_v4_rcv())
> ( -> __inet_lookup_skb())
> ( -> skb_steal_sock() )
>
>> at least thats what I concluded a few years ago when I looked into
>> the skb_oprhan() need.
>>
>> IIRC some device drivers use skb->sk for backpressure, so without this
>> non-tcp socket would be stolen by skb_steal_sock.
>
> Do you happen to recall which device drivers? Or have some idea of a
> list I could try to go through? Are you referring to virtual drivers
> like veth or something else?
>
>> We also recently removed skb orphan when crossing netns:
>>
>> commit 9c4c325252c54b34d53b3d0ffd535182b744e03d
>> Author: Flavio Leitner <fbl@redhat.com>
>> skbuff: preserve sock reference when scrubbing the skb.
>>
>> So thats another case where this orphan is needed.
>
> Presumably the orphan is only needed in this case if the packet
> crosses a namespace and then is subsequently passed back into the
> stack?
Yes, I understand we do not want the skb_orphan() when 'srubing' the skb.
But we want the skb_orphan() right before the packet is reinjected in ingress path.
>
>> What could be done is adding some way to delay/defer the orphaning
>> further, but we would need at the very least some annotation for
>> skb_steal_sock to know when the skb->sk is really from TPROXY or
>> if it has to orphan.
>
> Eric mentions in another response to this thread that skb_orphan()
> should be called from any ndo_start_xmit() which sends traffic back
> into the stack. With that, presumably we would be pushing the
> orphaning earlier such that the only way that the skb->sk ref can be
> non-NULL around this point in receive would be because it was
> specifically set by some kind of tproxy logic?
>
>> Same for the safety check in the forwarding path.
>> Netfilter modules need o be audited as well, they might make assumptions
>> wrt. skb->sk being inet sockets (set by local stack or early demux).
>>
>>> However, if I drop these lines then I end up causing sockets to
>>> release references too many times. Seems like if we don't orphan the
>>> skb here, then later logic assumes that we have one more reference
>>> than we actually have, and decrements the count when it shouldn't
>>> (perhaps the skb_steal_sock() call in __inet_lookup_skb() which seems
>>> to assume we always have a reference to the socket?)
>>
>> We might be calling the wrong destructor (i.e., the one set by tcp
>> receive instead of the one set at tx time)?
>
> Hmm, interesting thought. Sure enough, with a bit of bpftrace
> debugging we find it's tcp_wfree():
>
> $ cat ip_rcv.bt
> #include <linux/skbuff.h>
>
> kprobe:ip_rcv {
> $sk = ((struct sk_buff *)arg0)->sk;
> $des = ((struct sk_buff *)arg0)->destructor;
> if ($sk) {
> if ($des) {
> printf("received %s on %s with sk destructor %s
> set\n", str(arg0), str(arg1), ksym($des));
> @ip4_stacks[kstack] = count();
> }
> }
> }
> $ sudo bpftrace ip_rcv.bt
> Attaching 1 probe...
> received on eth0 with sk destructor tcp_wfree set
> ^C
>
> @ip4_stacks[
> ip_rcv+1
> __netif_receive_skb+24
> process_backlog+179
> net_rx_action+304
> __do_softirq+220
> do_softirq_own_stack+42
> do_softirq.part.17+70
> __local_bh_enable_ip+101
> ip_finish_output2+421
> __ip_finish_output+187
> ip_finish_output+44
> ip_output+109
> ip_local_out+59
> __ip_queue_xmit+368
> ip_queue_xmit+16
> __tcp_transmit_skb+1303
> tcp_connect+2758
> tcp_v4_connect+1135
> __inet_stream_connect+214
> inet_stream_connect+59
> __sys_connect+237
> __x64_sys_connect+26
> do_syscall_64+90
> entry_SYSCALL_64_after_hwframe+68
> ]: 1
>
> Is there a solution here where we call the destructor if it's not
> sock_efree()? When the socket is later stolen, it will only return the
> reference via a call to sock_put(), so presumably at that point in the
> stack we already assume that the skb->destructor is not one of these
> other destructors (otherwise we wouldn't release the resources
> correctly).
>
What was the driver here ? In any case, the following patch should help.
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index eeacebd7debbe6a55daedb92f00afd48051ebaf8..5075b4b267af7057f69fcb935226fce097a920e2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3699,6 +3699,7 @@ static __always_inline int ____dev_forward_skb(struct net_device *dev,
return NET_RX_DROP;
}
+ skb_orphan(skb);
skb_scrub_packet(skb, true);
skb->priority = 0;
return 0;
^ permalink raw reply related
* [PATCH net-next] net: ipvlan: forward ingress packet to slave's l2 in l3s mode
From: Zhiyuan Hou @ 2019-06-25 6:42 UTC (permalink / raw)
To: zhiyuan2048, davem, idosch, daniel, petrm, jiri, tglx, linmiaohe
Cc: zhabin, caspar, netdev, linux-kernel
In ipvlan l3s mode, ingress packet is switched to slave interface and
delivers to l4 stack. This may cause two problems:
1. When slave is in an ns different from master, the behavior of stack
in slave ns may cause confusion for users. For example, iptables, tc,
and other l2/l3 functions are not available for ingress packet.
2. l3s mode is not used for tap device, and cannot support ipvtap. But
in VM or container based VM cases, tap device is a very common device.
In l3s mode's input nf_hook, this patch calles the skb_forward_dev() to
forward ingress packet to slave and uses nf_conntrack_confirm() to make
conntrack work with new mode.
Signed-off-by: Zha Bin <zhabin@linux.alibaba.com>
Signed-off-by: Zhiyuan Hou <zhiyuan2048@linux.alibaba.com>
---
drivers/net/ipvlan/ipvlan.h | 9 ++++++++-
drivers/net/ipvlan/ipvlan_l3s.c | 16 ++++++++++++++--
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index 3837c897832e..48c814e24c3f 100644
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h
@@ -172,6 +172,14 @@ void ipvlan_link_delete(struct net_device *dev, struct list_head *head);
void ipvlan_link_setup(struct net_device *dev);
int ipvlan_link_register(struct rtnl_link_ops *ops);
#ifdef CONFIG_IPVLAN_L3S
+
+#include <net/netfilter/nf_conntrack_core.h>
+
+static inline int ipvlan_confirm_conntrack(struct sk_buff *skb)
+{
+ return nf_conntrack_confirm(skb);
+}
+
int ipvlan_l3s_register(struct ipvl_port *port);
void ipvlan_l3s_unregister(struct ipvl_port *port);
void ipvlan_migrate_l3s_hook(struct net *oldnet, struct net *newnet);
@@ -206,5 +214,4 @@ static inline bool netif_is_ipvlan_port(const struct net_device *dev)
{
return rcu_access_pointer(dev->rx_handler) == ipvlan_handle_frame;
}
-
#endif /* __IPVLAN_H */
diff --git a/drivers/net/ipvlan/ipvlan_l3s.c b/drivers/net/ipvlan/ipvlan_l3s.c
index 943d26cbf39f..ed210002f593 100644
--- a/drivers/net/ipvlan/ipvlan_l3s.c
+++ b/drivers/net/ipvlan/ipvlan_l3s.c
@@ -95,14 +95,26 @@ static unsigned int ipvlan_nf_input(void *priv, struct sk_buff *skb,
{
struct ipvl_addr *addr;
unsigned int len;
+ int ret = NF_ACCEPT;
+ bool success;
addr = ipvlan_skb_to_addr(skb, skb->dev);
if (!addr)
goto out;
- skb->dev = addr->master->dev;
len = skb->len + ETH_HLEN;
- ipvlan_count_rx(addr->master, len, true, false);
+
+ ret = ipvlan_confirm_conntrack(skb);
+ if (ret != NF_ACCEPT) {
+ ipvlan_count_rx(addr->master, len, false, false);
+ goto out;
+ }
+
+ skb_push_rcsum(skb, ETH_HLEN);
+ success = dev_forward_skb(addr->master->dev, skb) == NET_RX_SUCCESS;
+ ipvlan_count_rx(addr->master, len, success, false);
+ return NF_STOLEN;
+
out:
return NF_ACCEPT;
}
--
2.18.0
^ permalink raw reply related
* Re: [PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
From: Davide Caratti @ 2019-06-25 7:02 UTC (permalink / raw)
To: Lucas Bates, davem
Cc: netdev, nicolas.dichtel, jhs, xiyou.wangcong, jiri, mleitner,
vladbu, kernel
In-Reply-To: <1561424427-9949-1-git-send-email-lucasb@mojatatu.com>
On Mon, 2019-06-24 at 21:00 -0400, Lucas Bates wrote:
> This patch restores the original behaviour for tdc prior to the
> introduction of the plugin system, where the network namespace
> functionality was split from the main script.
>
> It introduces the concept of required plugins for testcases,
> and will automatically load any plugin that isn't already
> enabled when said plugin is required by even one testcase.
>
> Additionally, the -n option for the nsPlugin is deprecated
> so the default action is to make use of the namespaces.
> Instead, we introduce -N to not use them, but still create
> the veth pair.
>
> buildebpfPlugin's -B option is also deprecated.
>
> If a test cases requires the features of a specific plugin
> in order to pass, it should instead include a new key/value
> pair describing plugin interactions:
>
> "plugins": {
> "requires": "buildebpfPlugin"
> },
>
> A test case can have more than one required plugin: a list
> can be inserted as the value for 'requires'.
>
> Signed-off-by: Lucas Bates <lucasb@mojatatu.com>
> ---
hi Lucas,
thanks a lot for including a fix for buildebpfPlugin!
Acked-by: Davide Caratti <dcaratti@redhat.com>
^ permalink raw reply
* Re: [RFC bpf-next 0/7] Programming socket lookup with BPF
From: Jakub Sitnicki @ 2019-06-25 7:28 UTC (permalink / raw)
To: Joe Stringer; +Cc: Florian Westphal, netdev, bpf, kernel-team
In-Reply-To: <CAOftzPj6NWyWnz4JL-mXBaQUKAvQDtKJTrjZmrN4W5rqoy-W0A@mail.gmail.com>
[Reposting with correct format this time. Sorry.]
On Fri, Jun 21, 2019 at 12:20 AM CEST, Joe Stringer wrote:
> On Wed, Jun 19, 2019 at 2:14 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>>
>> Hey Florian,
>>
>> Thanks for taking a look at it.
>>
>> On Tue, Jun 18, 2019 at 03:52 PM CEST, Florian Westphal wrote:
>> > Jakub Sitnicki <jakub@cloudflare.com> wrote:
>> >> - XDP programs using bpf_sk_lookup helpers, like load balancers, can't
>> >> find the listening socket to check for SYN cookies with TPROXY redirect.
>> >
>> > Sorry for the question, but where is the problem?
>> > (i.e., is it with TPROXY or bpf side)?
>>
>> The way I see it is that the problem is that we have mappings for
>> steering traffic into sockets split between two places: (1) the socket
>> lookup tables, and (2) the TPROXY rules.
>>
>> BPF programs that need to check if there is a socket the packet is
>> destined for have access to the socket lookup tables, via the mentioned
>> bpf_sk_lookup helper, but are unaware of TPROXY redirects.
>>
>> For TCP we're able to look up from BPF if there are any established,
>> request, and "normal" listening sockets. The listening sockets that
>> receive connections via TPROXY are invisible to BPF progs.
>>
>> Why are we interested in finding all listening sockets? To check if any
>> of them had SYN queue overflow recently and if we should honor SYN
>> cookies.
>
> Why are they invisible? Can't you look them up with bpf_skc_lookup_tcp()?
They are invisible in that sense that you can't look them up using the
packet 4-tuple. You have to somehow make the XDP/TC progs aware of the
TPROXY redirects to find the target sockets.
-Jakub
^ permalink raw reply
* RE: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic
From: Jose Abreu @ 2019-06-25 7:37 UTC (permalink / raw)
To: Jon Hunter, Jose Abreu, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Cc: Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Russell King, Andrew Lunn, Florian Fainelli,
Heiner Kallweit, linux-tegra
In-Reply-To: <7f0f2ed0-f47c-4670-d169-25f0413c1fd3@nvidia.com>
From: Jon Hunter <jonathanh@nvidia.com>
> Any further feedback? I am still seeing this issue on today's -next.
Apologies but I was in FTO.
Is there any possibility you can just disable the ethX configuration in
the rootfs mount and manually configure it after rootfs is done ?
I just want to make sure in which conditions this is happening (if in
ifdown or ifup).
Thanks,
Jose Miguel Abreu
^ permalink raw reply
* Re: [PATCH 2/3] module: Fix up module_notifier return values.
From: Peter Zijlstra @ 2019-06-25 7:42 UTC (permalink / raw)
To: Frank Ch. Eigler
Cc: Mathieu Desnoyers, Jessica Yu, linux-kernel, Josh Poimboeuf,
jikos, mbenes, Petr Mladek, Alexei Starovoitov, Daniel Borkmann,
Andrew Morton, Robert Richter, rostedt, Ingo Molnar,
Martin KaFai Lau, Song Liu, Yonghong Song, paulmck,
Joel Fernandes, Google, Ard Biesheuvel, Thomas Gleixner,
oprofile-list, netdev, bpf
In-Reply-To: <20190624205810.GD26422@redhat.com>
On Mon, Jun 24, 2019 at 04:58:10PM -0400, Frank Ch. Eigler wrote:
> Hi -
>
> > > While auditing all module notifiers I noticed a whole bunch of fail
> > > wrt the return value. Notifiers have a 'special' return semantics.
>
> From peterz's comments, the patches, it's not obvious to me how one is
> to choose between 0 (NOTIFY_DONE) and 1 (NOTIFY_OK) in the case of a
> routine success.
I'm not sure either; what I think I choice was:
- if I want to completely ignore the callback, use DONE (per the
"Don't care" comment).
- if we finished the notifier without error, use OK or
notifier_from_errno(0).
But yes, its a bit of a shit interface.
^ permalink raw reply
* RE: stmmac regression on ASUS TinkerBoard
From: Jose Abreu @ 2019-06-25 7:46 UTC (permalink / raw)
To: Katsuhiro Suzuki, Giuseppe Cavallaro, Alexandre Torgue,
Maxime Coquelin, netdev@vger.kernel.org
Cc: Andrew Lunn, Heiko Stuebner, linux-arm-kernel,
Linux Kernel Mailing List
In-Reply-To: <8fa9ce79-6aa2-d44d-e24d-09cc1b2b70a3@katsuster.net>
From: Katsuhiro Suzuki <katsuhiro@katsuster.net>
> I checked drivers/net/ethernet/stmicro/stmmac/stmmac_main.c and found
> stmmac_init_phy() is going to fail if ethernet device node does not
> have following property:
> - phy-handle
> - phy
> - phy-device
>
> This commit broke the device-trees such as TinkerBoard. The mdio
> subnode creating a mdio bus is changed to required or still optional?
Yeah, with PHYLINK the PHY binding is always required ...
How do you want to proceed ? I think DT bindings can never break between
releases so I will probably need to cook a patch for stmmac.
Thanks,
Jose Miguel Abreu
^ permalink raw reply
* Re: stmmac regression on ASUS TinkerBoard
From: Heiko Stübner @ 2019-06-25 7:50 UTC (permalink / raw)
To: Jose Abreu
Cc: Katsuhiro Suzuki, Giuseppe Cavallaro, Alexandre Torgue,
Maxime Coquelin, netdev@vger.kernel.org, Andrew Lunn,
linux-arm-kernel, Linux Kernel Mailing List
In-Reply-To: <78EB27739596EE489E55E81C33FEC33A0B9D7065@DE02WEMBXB.internal.synopsys.com>
Hi,
Am Dienstag, 25. Juni 2019, 09:46:12 CEST schrieb Jose Abreu:
> From: Katsuhiro Suzuki <katsuhiro@katsuster.net>
>
> > I checked drivers/net/ethernet/stmicro/stmmac/stmmac_main.c and found
> > stmmac_init_phy() is going to fail if ethernet device node does not
> > have following property:
> > - phy-handle
> > - phy
> > - phy-device
> >
> > This commit broke the device-trees such as TinkerBoard. The mdio
> > subnode creating a mdio bus is changed to required or still optional?
>
> Yeah, with PHYLINK the PHY binding is always required ...
>
> How do you want to proceed ? I think DT bindings can never break between
> releases so I will probably need to cook a patch for stmmac.
Correct ... old devicetrees on new kernels should not break.
Especially as this affects a big number of boards potentially loosing
network support and in the devicetree binding the phy property is also
marked as optional.
Heiko
^ permalink raw reply
* Re: [PATCH] net: stmmac: add sanity check to device_property_read_u32_array call
From: Colin Ian King @ 2019-06-25 7:58 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: <CAFBinCC-LLpfXQRFcKBbUpCfKc0S9Xtt60QrhEThsOFV-T7vFw@mail.gmail.com>
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?
I'm out of the office today. I'll get back to you on this tomorrow.
Colin
>
>
> Martin
>
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c?id=84ce4d0f9f55b4f4ca4d4edcbb54a23d9dad1aae
>
^ permalink raw reply
* Re: [PATCH bpf-next] libbpf: add xsk_ring_prod__free() function
From: Eelco Chaudron @ 2019-06-25 7:58 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Karlsson, Magnus, Networking, Alexei Starovoitov, Daniel Borkmann,
Martin Lau, Song Liu, Yonghong Song
In-Reply-To: <CAEf4Bzb7A5abJaxxrS5sudCE=Ca0C9rY0B23OjK8c2RCCx=Y6g@mail.gmail.com>
On 24 Jun 2019, at 18:42, Andrii Nakryiko wrote:
> On Mon, Jun 24, 2019 at 2:37 AM Eelco Chaudron <echaudro@redhat.com>
> wrote:
>>
>>
>>
>> On 21 Jun 2019, at 21:13, Andrii Nakryiko wrote:
>>
>>> On Fri, Jun 21, 2019 at 8:26 AM Eelco Chaudron <echaudro@redhat.com>
>>> wrote:
>>>>
>>>> When an AF_XDP application received X packets, it does not mean X
>>>> frames can be stuffed into the producer ring. To make it easier for
>>>> AF_XDP applications this API allows them to check how many frames
>>>> can
>>>> be added into the ring.
>>>>
>>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>>> ---
>>>> tools/lib/bpf/xsk.h | 6 ++++++
>>>> 1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
>>>> index 82ea71a0f3ec..86f3d485e957 100644
>>>> --- a/tools/lib/bpf/xsk.h
>>>> +++ b/tools/lib/bpf/xsk.h
>>>> @@ -95,6 +95,12 @@ static inline __u32 xsk_prod_nb_free(struct
>>>> xsk_ring_prod *r, __u32 nb)
>>>> return r->cached_cons - r->cached_prod;
>>>> }
>>>>
>>>> +static inline __u32 xsk_ring_prod__free(struct xsk_ring_prod *r)
>>>
>>> This is a very bad name choice. __free is used for functions that
>>> free
>>> memory and resources. One function below I see avail is used in the
>>> name, why not xsk_ring_prog__avail?
>>
>> Must agree that free sound like you are freeing entries… However, I
>> just kept the naming already in the API/file (see above,
>> xsk_prod_nb_free()).
>> Reading the code there is a difference as xx_avail() means available
>> filled entries, where xx_free() means available free entries.
>>
>> So I could rename it to xsk_ring_prod__nb_free() maybe?
>
> I'm fine with __nb_free, yes. Thanks!
Ok, will rework the patch and use xsk_ring_prod__nb_free(). Will also
take Magnus suggestion into account, and create a cached version (and
use it internally).
>>
>> Forgot to include Magnus in the email, so copied him in, for some
>> comments.
>>
>>>> +{
>>>> + r->cached_cons = *r->consumer + r->size;
>>>> + return r->cached_cons - r->cached_prod;
>>>> +}
>>>> +
>>>> static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r,
>>>> __u32
>>>> nb)
>>>> {
>>>> __u32 entries = r->cached_prod - r->cached_cons;
>>>> --
>>>> 2.20.1
>>>>
^ permalink raw reply
* Re: [RFC bpf-next 0/7] Programming socket lookup with BPF
From: Jakub Sitnicki @ 2019-06-25 8:11 UTC (permalink / raw)
To: Joe Stringer; +Cc: Florian Westphal, netdev, bpf, kernel-team
In-Reply-To: <CAOftzPhGVeLpqbffLwBP8JCvY1t65-uXztEsZV0qJEQapywRgg@mail.gmail.com>
On Fri, Jun 21, 2019 at 06:50 PM CEST, Joe Stringer wrote:
> On Fri, Jun 21, 2019 at 1:44 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>>
>> On Fri, Jun 21, 2019, 00:20 Joe Stringer <joe@wand.net.nz> wrote:
>>>
>>> On Wed, Jun 19, 2019 at 2:14 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>>> >
>>> > Hey Florian,
>>> >
>>> > Thanks for taking a look at it.
>>> >
>>> > On Tue, Jun 18, 2019 at 03:52 PM CEST, Florian Westphal wrote:
>>> > > Jakub Sitnicki <jakub@cloudflare.com> wrote:
>>> > >> - XDP programs using bpf_sk_lookup helpers, like load balancers, can't
>>> > >> find the listening socket to check for SYN cookies with TPROXY redirect.
>>> > >
>>> > > Sorry for the question, but where is the problem?
>>> > > (i.e., is it with TPROXY or bpf side)?
>>> >
>>> > The way I see it is that the problem is that we have mappings for
>>> > steering traffic into sockets split between two places: (1) the socket
>>> > lookup tables, and (2) the TPROXY rules.
>>> >
>>> > BPF programs that need to check if there is a socket the packet is
>>> > destined for have access to the socket lookup tables, via the mentioned
>>> > bpf_sk_lookup helper, but are unaware of TPROXY redirects.
>>> >
>>> > For TCP we're able to look up from BPF if there are any established,
>>> > request, and "normal" listening sockets. The listening sockets that
>>> > receive connections via TPROXY are invisible to BPF progs.
>>> >
>>> > Why are we interested in finding all listening sockets? To check if any
>>> > of them had SYN queue overflow recently and if we should honor SYN
>>> > cookies.
>>>
>>> Why are they invisible? Can't you look them up with bpf_skc_lookup_tcp()?
>>
>>
>> They are invisible in that sense that you can't look them up using the packet 4-tuple. You have to somehow make the XDP/TC progs aware of the TPROXY redirects to find the target sockets.
>
> Isn't that what you're doing in the example from the cover letter
> (reincluded below for reference), except with the new program type
> rather than XDP/TC progs?
>
> switch (bpf_ntohl(ctx->local_ip4) >> 8) {
> case NET1:
> ctx->local_ip4 = bpf_htonl(IP4(127, 0, 0, 1));
> ctx->local_port = 81;
> return BPF_REDIRECT;
> case NET2:
> ctx->local_ip4 = bpf_htonl(IP4(127, 0, 0, 1));
> ctx->local_port = 82;
> return BPF_REDIRECT;
> }
>
> That said, I appreciate that even if you find the sockets from XDP,
> you'd presumably need some way to retain the socket reference beyond
> XDP execution to convince the stack to guide the traffic into that
> socket, which would be a whole other effort. For your use case it may
> or may not make the most sense.
Granted we're just moving steering logic from one place to another, that
is from TPROXY rules to a BPF program.
The key here is that the BPF prog runs during inet_lookup. This let's
"lower level" BPF progs like XDP or TC check if there is a destination
socket, without having to know about steering rules.
If there is a local socket, we don't need to do socket dispatch from
BPF. Just pass the packet up the stack.
-Jakub
^ permalink raw reply
* Re: [PATCH net-next 11/12] net: flow_offload: don't allow block sharing until drivers support this
From: Jiri Pirko @ 2019-06-25 8:16 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, netfilter-devel, davem, thomas.lendacky, f.fainelli,
ariel.elior, michael.chan, santosh, madalin.bucur, yisen.zhuang,
salil.mehta, jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch,
jakub.kicinski, peppe.cavallaro, grygorii.strashko, andrew,
vivien.didelot, alexandre.torgue, joabreu, linux-net-drivers,
ganeshgr, ogerlitz, Manish.Chopra, marcelo.leitner, mkubecek,
venkatkumar.duvvuru, cphealy
In-Reply-To: <20190620194917.2298-12-pablo@netfilter.org>
I don't understand the purpose of this patch. Could you please provide
some description about what this is about. mlxsw supports block sharing
between ports. Or what kind of "sharing" do you have in mind?
^ permalink raw reply
* Re: [PATCH v3] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: Pablo Neira Ayuso @ 2019-06-25 8:17 UTC (permalink / raw)
To: linmiaohe
Cc: kadlec@blackhole.kfki.hu, fw@strlen.de, davem@davemloft.net,
kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org,
netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
dsahern@gmail.com, Mingfangsen
In-Reply-To: <30442ee669c44d9db01fb374b73fd2dd@huawei.com>
On Wed, Jun 19, 2019 at 09:49:04AM +0000, linmiaohe wrote:
>
> On 2019/6/18 23:58, Pablo Neira Ayuso wrote:
> > On Thu, Apr 25, 2019 at 09:43:53PM +0800, linmiaohe wrote:
> >> From: Miaohe Lin <linmiaohe@huawei.com>
> >>
> >> When firewalld is enabled with ipv4/ipv6 rpfilter, vrf
> >> ipv4/ipv6 packets will be dropped because in device is vrf but out
> >> device is an enslaved device. So failed with the check of the
> >> rpfilter.
> >>
> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> >> ---
> >> --- a/net/ipv4/netfilter/ipt_rpfilter.c
> >> +++ b/net/ipv4/netfilter/ipt_rpfilter.c
> >> @@ -81,6 +81,7 @@ static bool rpfilter_mt(const struct sk_buff *skb, struct xt_action_param *par)
> >> flow.flowi4_mark = info->flags & XT_RPFILTER_VALID_MARK ? skb->mark : 0;
> >> flow.flowi4_tos = RT_TOS(iph->tos);
> >> flow.flowi4_scope = RT_SCOPE_UNIVERSE;
> >> + flow.flowi4_oif = l3mdev_master_ifindex_rcu(xt_in(par));
> >>
> >> return rpfilter_lookup_reverse(xt_net(par), &flow, xt_in(par),
> >> --- a/net/ipv6/netfilter/ip6t_rpfilter.c
> >> +++ b/net/ipv6/netfilter/ip6t_rpfilter.c
> >> @@ -58,7 +58,9 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
> >> if (rpfilter_addr_linklocal(&iph->saddr)) {
> >> lookup_flags |= RT6_LOOKUP_F_IFACE;
> >> fl6.flowi6_oif = dev->ifindex;
> >> - } else if ((flags & XT_RPFILTER_LOOSE) == 0)
> >> + } else if (((flags & XT_RPFILTER_LOOSE) == 0) ||
> >> + (netif_is_l3_master(dev)) ||
> >> + (netif_is_l3_slave(dev)))
> >> fl6.flowi6_oif = dev->ifindex;
> >>
> >> rt = (void *)ip6_route_lookup(net, &fl6, skb, lookup_flags); @@
> >> -73,6 +75,12 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
> >> goto out;
> >> }
> >>
> >> + if (netif_is_l3_master(dev)) {
> >> + dev = dev_get_by_index_rcu(dev_net(dev), IP6CB(skb)->iif);
> >> + if (!dev)
> >> + goto out;
> >> + }
> >
> > So, for the l3 device cases this makes:
> >
> > #1 ip6_route_lookup() to fetch the route, using the device in xt_in()
> > (the _LOOSE flag is ignored for the l3 device case).
> >
> > #2 If this is a l3dev master, then you make a global lookup for the
> > device using IP6CB(skb)->iif.
> >
> > #3 You check if route matches with the device, using the new device
> > from the lookup:
> >
> > if (rt->rt6i_idev->dev == dev ...
> >
> > If there is no other way to fix this, OK, that's fair enough.
> >
> > Still this fix looks a bit tricky to me.
> >
> > And this assymmetric between the IPv4 and IPv6 codebase looks rare.
> >
> > Probably someone can explain me this in more detail? I'd appreciate.
> >
> > Thanks!
> >
> Thanks for your reply. I will try to explain this in more detail.
> Vrf device will pass through netfilter hook twice. One with skb->dev=l3mdev
> Slave device and another one with skb->dev=l3mdev master deivce.
> If a device is an l3mdev, l3mdev_master_ifindex_rcu will return l3mdev
> master device ifindex otherwise 0 . So for non l3mdev cases, v4 version is
> as same as the previous one. And for l3mdev cases, flow.flowi4_oif
> will be l3mdev master device ifindex, so we can do a fib lookup in l3mdev
> domain as expected. Since fib_info_nh_uses_dev help us handle the case with
> dev=l3mdev slave or master and XT_RPFILTER_LOOSE do not lookup route
> table, we finish v4.
> For v6 version we need to set fl6.flowi6_oif as we are supposed to lookup
> fib in l3mdev domain even in XT_RPFILTER_LOOSE mode.
> And fib result rt->rt6i_idev->dev is l3mdev slave device, we need change
> dev to enslaved l3mdev device when dev passed in is l3mdev master device.
> The key is l3mdev will pass through netfilter hook twice with skb dev is l3mdev slave
> and master . And we need to set flowi6_oif as fib lookup should in the l3mdev
> domain.
Thanks for explaining.
Something must be wrong in all these helper function logic because
this new code logic is hard to follow for the IPv6 chunk...
Can you explore a more readable fix?
So I'm not inclined to quickly take this patch.
Thanks.
^ permalink raw reply
* [PATCH net-next] net: stmmac: Fix the case when PHY handle is not present
From: Jose Abreu @ 2019-06-25 8:19 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
Some DT bindings do not have the PHY handle. Let's fallback to manually
discovery in case phylink_of_phy_connect() fails.
Reported-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
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>
---
Hello Katsuhiro,
Can you please test this patch ?
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a48751989fa6..f4593d2d9d20 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -950,9 +950,12 @@ 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 net-next 11/12] net: flow_offload: don't allow block sharing until drivers support this
From: Pablo Neira Ayuso @ 2019-06-25 8:22 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, netfilter-devel, davem, thomas.lendacky, f.fainelli,
ariel.elior, michael.chan, santosh, madalin.bucur, yisen.zhuang,
salil.mehta, jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch,
jakub.kicinski, peppe.cavallaro, grygorii.strashko, andrew,
vivien.didelot, alexandre.torgue, joabreu, linux-net-drivers,
ganeshgr, ogerlitz, Manish.Chopra, marcelo.leitner, mkubecek,
venkatkumar.duvvuru, cphealy
In-Reply-To: <20190625081627.GA2630@nanopsycho>
On Tue, Jun 25, 2019 at 10:16:27AM +0200, Jiri Pirko wrote:
> I don't understand the purpose of this patch. Could you please provide
> some description about what this is about. mlxsw supports block sharing
> between ports. Or what kind of "sharing" do you have in mind?
I'm refering to ethtool, tc and netfilter potentially using the same
tc setup infrastructure infrastructure.
At this stage, if one of them sets up the flow block, then, other
subsystems will hit busy.
^ permalink raw reply
* Re: [PATCH RFC net-next 5/5] net: dsa: mt7530: Add mediatek,ephy-handle to isolate external phy
From: René van Dorst @ 2019-06-25 8:24 UTC (permalink / raw)
To: Andrew Lunn
Cc: sean.wang, f.fainelli, linux, davem, matthias.bgg, vivien.didelot,
frank-w, netdev, linux-mediatek, linux-mips
In-Reply-To: <20190624215248.GC31306@lunn.ch>
Quoting Andrew Lunn <andrew@lunn.ch>:
Hi Andrew,
>> +static int mt7530_isolate_ephy(struct dsa_switch *ds,
>> + struct device_node *ephy_node)
>> +{
>> + struct phy_device *phydev = of_phy_find_device(ephy_node);
>> + int ret;
>> +
>> + if (!phydev)
>> + return 0;
>> +
>> + ret = phy_modify(phydev, MII_BMCR, 0, (BMCR_ISOLATE | BMCR_PDOWN));
>
> genphy_suspend() does what you want.
In case my device has AT8033 PHY which act as a RGMII-to-SGMII
converter for the
SFP cage.
Qoute of the AR8031/33 datasheet:
The AR8033 device supports the low power mode with software power-down.
To enter the standard IEEE power-down mode, set the bit[11] POWER_DOWN of
Control register - copper page or Control register — fiber page to 1.
In this mode, AR8033 ignores all MAC interface signals except the MDC/MDIO and
does not respond to any activity on the media side. AR8033 cannot wake
up on its
own and is only waken up by setting the POWER_DOWN bit to 0.
Does "standard IEEE power-down mode" describ this behavior that in power-down
mode the RGMII are also put in tri-state?
Reading the datasheet does not give me any clues.
Putting RGMII signals in tri-state is important in this case.
>
>> + if (ret)
>> + dev_err(ds->dev, "Failed to put phy %s in isolation mode!\n",
>> + ephy_node->full_name);
>> + else
>> + dev_info(ds->dev, "Phy %s in isolation mode!\n",
>> + ephy_node->full_name);
>
> No need to clog up the system with yet more kernel messages.
OK, I remove it.
>
> Andrew
Greats,
René
^ permalink raw reply
* RE: [PATCH v4 4/5] net: macb: add support for high speed interface
From: Parshuram Raju Thombare @ 2019-06-25 8:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
f.fainelli@gmail.com, linux@armlinux.org.uk,
netdev@vger.kernel.org, hkallweit1@gmail.com,
linux-kernel@vger.kernel.org, Rafal Ciepiela, Anil Joy Varughese,
Piotr Sroka
In-Reply-To: <20190624131307.GA17872@lunn.ch>
Hi Andrew,
>What i'm saying is that the USXGMII rate is fixed. So why do you need a device
>tree property for the SERDES rate?
This is based on Cisco USXGMII specification, it specify USXGMII 5G and USXGMII 10G.
Sorry I can't share that document here.
Regards,
Parshuram Thombare
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: sched: protect against stack overflow in TC act_mirred
From: Eyal Birger @ 2019-06-25 8:30 UTC (permalink / raw)
To: John Hurley
Cc: netdev, davem, fw, jhs, simon.horman, jakub.kicinski, oss-drivers,
shmulik
In-Reply-To: <1561414416-29732-3-git-send-email-john.hurley@netronome.com>
Hi John,
On Mon, 24 Jun 2019 23:13:36 +0100
John Hurley <john.hurley@netronome.com> wrote:
> TC hooks allow the application of filters and actions to packets at
> both ingress and egress of the network stack. It is possible, with
> poor configuration, that this can produce loops whereby an ingress
> hook calls a mirred egress action that has an egress hook that
> redirects back to the first ingress etc. The TC core classifier
> protects against loops when doing reclassifies but there is no
> protection against a packet looping between multiple hooks and
> recursively calling act_mirred. This can lead to stack overflow
> panics.
>
> Add a per CPU counter to act_mirred that is incremented for each
> recursive call of the action function when processing a packet. If a
> limit is passed then the packet is dropped and CPU counter reset.
>
> Note that this patch does not protect against loops in TC datapaths.
> Its aim is to prevent stack overflow kernel panics that can be a
> consequence of such loops.
>
> Signed-off-by: John Hurley <john.hurley@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
> ---
> net/sched/act_mirred.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index 8c1d736..c3fce36 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -27,6 +27,9 @@
> static LIST_HEAD(mirred_list);
> static DEFINE_SPINLOCK(mirred_list_lock);
>
> +#define MIRRED_RECURSION_LIMIT 4
Could you increase the limit to maybe 6 or 8? I am aware of cases where
mirred ingress is used for cascading several layers of logical network
interfaces and 4 seems a little limiting.
Thanks,
Eyal.
^ permalink raw reply
* Re: [PATCH net-next 04/12] net: sched: add tcf_block_setup()
From: Pablo Neira Ayuso @ 2019-06-25 8:31 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, netfilter-devel, davem, thomas.lendacky, f.fainelli,
ariel.elior, michael.chan, santosh, madalin.bucur, yisen.zhuang,
salil.mehta, jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch,
jakub.kicinski, peppe.cavallaro, grygorii.strashko, andrew,
vivien.didelot, alexandre.torgue, joabreu, linux-net-drivers,
ganeshgr, ogerlitz, Manish.Chopra, marcelo.leitner, mkubecek,
venkatkumar.duvvuru, cphealy
In-Reply-To: <20190621171603.GF2414@nanopsycho.orion>
On Fri, Jun 21, 2019 at 07:16:03PM +0200, Jiri Pirko wrote:
> Thu, Jun 20, 2019 at 09:49:09PM CEST, pablo@netfilter.org wrote:
>
> [...]
>
> >
> >+static LIST_HEAD(tcf_block_cb_list);
>
> I still don't like the global list. Have to go throught the code more
> carefully, but why you can't pass the priv/ctx from tc/netfilter. From
> tc it would be tcf_block as it is now, from netfilter something else.
This tcf_block_cb_list should go away at some point, once drivers know
how to deal with multiple subsystems using the setup block
infrastructure. As I said in my previous email, only one can set up
the block at this stage, the ones coming later will hit busy.
^ permalink raw reply
* [PATCH v1 1/2] dt-bindings: net: dsa: qca8k: document reset-gpios property
From: Christian Lamparter @ 2019-06-25 8:41 UTC (permalink / raw)
To: netdev
Cc: Mark Rutland, Rob Herring, David S . Miller, Florian Fainelli,
Vivien Didelot, Andrew Lunn
This patch documents the qca8k's reset-gpios property that
can be used if the QCA8337N ends up in a bad state during
reset.
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
Documentation/devicetree/bindings/net/dsa/qca8k.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
index 93a7469e70d4..ccbc6d89325d 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
@@ -9,6 +9,10 @@ Required properties:
- #size-cells: must be 0
- #address-cells: must be 1
+Optional properties:
+
+- reset-gpios: GPIO to be used to reset the whole device
+
Subnodes:
The integrated switch subnode should be specified according to the binding
@@ -66,6 +70,7 @@ for the external mdio-bus configuration:
#address-cells = <1>;
#size-cells = <0>;
+ reset-gpios = <&gpio 42 GPIO_ACTIVE_LOW>;
reg = <0x10>;
ports {
@@ -123,6 +128,7 @@ for the internal master mdio-bus configuration:
#address-cells = <1>;
#size-cells = <0>;
+ reset-gpios = <&gpio 42 GPIO_ACTIVE_LOW>;
reg = <0x10>;
ports {
--
2.20.1
^ permalink raw reply related
* [PATCH v1 2/2] net: dsa: qca8k: introduce reset via gpio feature
From: Christian Lamparter @ 2019-06-25 8:41 UTC (permalink / raw)
To: netdev
Cc: Mark Rutland, Rob Herring, David S . Miller, Florian Fainelli,
Vivien Didelot, Andrew Lunn
In-Reply-To: <08e0fd513620f03a2207b9f32637cdb434ed8def.1561452044.git.chunkeey@gmail.com>
The QCA8337(N) has a RESETn signal on Pin B42 that
triggers a chip reset if the line is pulled low.
The datasheet says that: "The active low duration
must be greater than 10 ms".
This can hopefully fix some of the issues related
to pin strapping in OpenWrt for the EA8500 which
suffers from detection issues after a SoC reset.
Please note that the qca8k_probe() function does
currently require to read the chip's revision
register for identification purposes.
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
drivers/net/dsa/qca8k.c | 15 +++++++++++++++
drivers/net/dsa/qca8k.h | 2 ++
2 files changed, 17 insertions(+)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index c4fa400efdcc..27709f866c23 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -14,6 +14,7 @@
#include <linux/of_platform.h>
#include <linux/if_bridge.h>
#include <linux/mdio.h>
+#include <linux/gpio.h>
#include <linux/etherdevice.h>
#include "qca8k.h"
@@ -1046,6 +1047,20 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
priv->bus = mdiodev->bus;
priv->dev = &mdiodev->dev;
+ priv->reset_gpio = devm_gpiod_get_optional(priv->dev, "reset",
+ GPIOD_ASIS);
+ if (IS_ERR(priv->reset_gpio))
+ return PTR_ERR(priv->reset_gpio);
+
+ if (priv->reset_gpio) {
+ gpiod_set_value_cansleep(priv->reset_gpio, 1);
+ /* The active low duration must be greater than 10 ms
+ * and checkpatch.pl wants 20 ms.
+ */
+ msleep(20);
+ gpiod_set_value_cansleep(priv->reset_gpio, 0);
+ }
+
/* read the switches ID register */
id = qca8k_read(priv, QCA8K_REG_MASK_CTRL);
id >>= QCA8K_MASK_CTRL_ID_S;
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 91557433ce2f..42d6ea24eb14 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -10,6 +10,7 @@
#include <linux/delay.h>
#include <linux/regmap.h>
+#include <linux/gpio.h>
#define QCA8K_NUM_PORTS 7
@@ -174,6 +175,7 @@ struct qca8k_priv {
struct mutex reg_mutex;
struct device *dev;
struct dsa_switch_ops ops;
+ struct gpio_desc *reset_gpio;
};
struct qca8k_mib_desc {
--
2.20.1
^ permalink raw reply related
* [PATCH v2 2/2] net: macb: Kconfig: Rename Atmel to Cadence
From: Palmer Dabbelt @ 2019-06-25 8:48 UTC (permalink / raw)
To: nicolas.ferre, harinik; +Cc: davem, netdev, linux-kernel, Palmer Dabbelt
In-Reply-To: <20190625084828.540-1-palmer@sifive.com>
The help text makes it look like NET_VENDOR_CADENCE enables support for
Atmel devices, when in reality it's a driver written by Atmel that
supports Cadence devices. This may confuse users that have this device
on a non-Atmel SoC.
The fix is just s/Atmel/Cadence/, but I did go and re-wrap the Kconfig
help text as that change caused it to go over 80 characters.
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
drivers/net/ethernet/cadence/Kconfig | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 64d8d6ee7739..f4b3bd85dfe3 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
#
-# Atmel device configuration
+# Cadence device configuration
#
config NET_VENDOR_CADENCE
@@ -13,8 +13,8 @@ config NET_VENDOR_CADENCE
If unsure, say Y.
Note that the answer to this question doesn't directly affect the
- kernel: saying N will just cause the configurator to skip all
- the remaining Atmel network card questions. If you say Y, you will be
+ kernel: saying N will just cause the configurator to skip all the
+ remaining Cadence network card questions. If you say Y, you will be
asked for your specific card in the following questions.
if NET_VENDOR_CADENCE
--
2.21.0
^ permalink raw reply related
* [PATCH v2 1/2] net: macb: Kconfig: Make MACB depend on COMMON_CLK
From: Palmer Dabbelt @ 2019-06-25 8:48 UTC (permalink / raw)
To: nicolas.ferre, harinik; +Cc: davem, netdev, linux-kernel, Palmer Dabbelt
In-Reply-To: <20190625084828.540-1-palmer@sifive.com>
commit c218ad559020 ("macb: Add support for SiFive FU540-C000") added a
dependency on the common clock framework to the macb driver, but didn't
express that dependency in Kconfig. As a result macb now fails to
compile on systems without COMMON_CLK, which specifically causes a build
failure on powerpc allyesconfig.
This patch adds the dependency, which results in the macb driver no
longer being selectable on systems without the common clock framework.
All known systems that have this device already support the common clock
framework, so this should not cause trouble for any uses. Supporting
both the FU540-C000 and systems without COMMON_CLK is quite ugly.
I've build tested this on powerpc allyesconfig and RISC-V defconfig
(which selects MACB), but I have not even booted the resulting kernels.
Fixes: c218ad559020 ("macb: Add support for SiFive FU540-C000")
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
drivers/net/ethernet/cadence/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 1766697c9c5a..64d8d6ee7739 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -21,7 +21,7 @@ if NET_VENDOR_CADENCE
config MACB
tristate "Cadence MACB/GEM support"
- depends on HAS_DMA
+ depends on HAS_DMA && COMMON_CLK
select PHYLIB
---help---
The Cadence MACB ethernet interface is found on many Atmel AT32 and
@@ -42,7 +42,7 @@ config MACB_USE_HWSTAMP
config MACB_PCI
tristate "Cadence PCI MACB/GEM support"
- depends on MACB && PCI && COMMON_CLK
+ depends on MACB && PCI
---help---
This is PCI wrapper for MACB driver.
--
2.21.0
^ permalink raw reply related
* net: macb: Fix compilation on systems without COMMON_CLK, v2
From: Palmer Dabbelt @ 2019-06-25 8:48 UTC (permalink / raw)
To: nicolas.ferre, harinik; +Cc: davem, netdev, linux-kernel
Our patch to add support for the FU540-C000 broke compilation on at
least powerpc allyesconfig, which was found as part of the linux-next
build regression tests. This must have somehow slipped through the
cracks, as the patch has been reverted in linux-next for a while now.
This patch applies on top of the offending commit, which is the only one
I've even tried it on as I'm not sure how this subsystem makes it to
Linus.
This patch set fixes the issue by adding a dependency of COMMON_CLK to
the MACB Kconfig entry, which avoids the build failure by disabling MACB
on systems where it wouldn't compile. All known users of MACB have
COMMON_CLK, so this shouldn't cause any issues. This is a significantly
simpler approach than disabling just the FU540-C000 support.
I've also included a second patch to indicate this is a driver for a
Cadence device that was originally written by an engineer at Atmel. The
only relation is that I stumbled across it when writing the first patch.
Changes since v1 <20190624061603.1704-1-palmer@sifive.com>:
* Disable MACB on systems without COMMON_CLK, instead of just disabling
the FU540-C000 support on these systems.
* Update the commit message to reflect the driver was written by Atmel.
^ permalink raw reply
* Re: [PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
From: Nicolas Dichtel @ 2019-06-25 8:49 UTC (permalink / raw)
To: Lucas Bates, davem
Cc: netdev, jhs, xiyou.wangcong, jiri, mleitner, vladbu, dcaratti,
kernel
In-Reply-To: <1561424427-9949-1-git-send-email-lucasb@mojatatu.com>
Le 25/06/2019 à 03:00, Lucas Bates a écrit :
> This patch restores the original behaviour for tdc prior to the
> introduction of the plugin system, where the network namespace
> functionality was split from the main script.
>
> It introduces the concept of required plugins for testcases,
> and will automatically load any plugin that isn't already
> enabled when said plugin is required by even one testcase.
>
> Additionally, the -n option for the nsPlugin is deprecated
> so the default action is to make use of the namespaces.
> Instead, we introduce -N to not use them, but still create
> the veth pair.
>
> buildebpfPlugin's -B option is also deprecated.
>
> If a test cases requires the features of a specific plugin
> in order to pass, it should instead include a new key/value
> pair describing plugin interactions:
>
> "plugins": {
> "requires": "buildebpfPlugin"
> },
>
> A test case can have more than one required plugin: a list
> can be inserted as the value for 'requires'.
>
> Signed-off-by: Lucas Bates <lucasb@mojatatu.com>
Thank you for the follow up!
Tested-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
[snip]
> @@ -550,6 +614,7 @@ def filter_tests_by_category(args, testlist):
>
> return answer
>
> +
> def get_test_cases(args):
> """
> If a test case file is specified, retrieve tests from that file.
nit: this new line is probably a leftover of a previous version ;-)
^ 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