* Re: [PATCH iproute2] ip: add handling for new CAN netlink interface
From: Stephen Hemminger @ 2017-05-22 20:37 UTC (permalink / raw)
To: Remigiusz Kołłątaj; +Cc: netdev, linux-can
In-Reply-To: <20170519125449.7214-1-remigiusz.kollataj@mobica.com>
On Fri, 19 May 2017 14:54:49 +0200
Remigiusz Kołłątaj <remigiusz.kollataj@mobica.com> wrote:
> This patch adds handling for new CAN netlink interface introduced in
> 4.11 kernel:
> - IFLA_CAN_TERMINATION,
> - IFLA_CAN_TERMINATION_CONST,
> - IFLA_CAN_BITRATE_CONST,
> - IFLA_CAN_DATA_BITRATE_CONST
>
> Output example:
> $ip -d link show can0
> 6: can0: <NOARP,ECHO> mtu 16 qdisc noop state DOWN mode DEFAULT group default qlen 10
> link/can promiscuity 0
> can state STOPPED (berr-counter tx 0 rx 0) restart-ms 0
> bitrate 80000
> [ 20000, 33333, 50000, 80000, 83333, 100000,
> 125000, 150000, 175000, 200000, 225000, 250000,
> 275000, 300000, 500000, 625000, 800000, 1000000 ]
> termination 0 [ 0, 120 ]
> clock 0numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
>
> Signed-off-by: Remigiusz Kołłątaj <remigiusz.kollataj@mobica.com>
What is output without the -d flag?
In general iproute2 show commands are designed to be invertable.
I.e the show command looks like the same command to set.
Printing the bitrates and the format of the state output looks quite different
than the set command. Is it limited to detail (-d)?
^ permalink raw reply
* Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers
From: Jesper Dangaard Brouer @ 2017-05-22 20:42 UTC (permalink / raw)
To: Tom Herbert
Cc: Daniel Borkmann, Alexei Starovoitov,
Linux Kernel Network Developers, brouer
In-Reply-To: <20170522083935.4d82174f@redhat.com>
On Mon, 22 May 2017 08:39:35 +0200
Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> On Sun, 21 May 2017 15:10:29 -0700
> Tom Herbert <tom@herbertland.com> wrote:
>
> > On Sun, May 21, 2017 at 9:04 AM, Jesper Dangaard Brouer
> > <brouer@redhat.com> wrote:
> > > On Sat, 20 May 2017 09:16:09 -0700
> > > Tom Herbert <tom@herbertland.com> wrote:
> > >
> > >> > +/* XDP rxhash have an associated type, which is related to the RSS
> > >> > + * (Receive Side Scaling) standard, but NIC HW have different mapping
> > >> > + * and support. Thus, create mapping that is interesting for XDP. XDP
> > >> > + * would primarly want insight into L3 and L4 protocol info.
> > >> > + *
> > >> > + * TODO: Likely need to get extended with "L3_IPV6_EX" due RSS standard
> > >> > + *
> > >> > + * The HASH_TYPE will be returned from bpf helper as the top 32-bit of
> > >> > + * the 64-bit rxhash (internally type stored in xdp_buff->flags).
> > >> > + */
> > >> > +#define XDP_HASH(x) ((x) & ((1ULL << 32)-1))
> > >> > +#define XDP_HASH_TYPE(x) ((x) >> 32)
> > >> > +
> > >> > +#define XDP_HASH_TYPE_L3_SHIFT 0
> > >> > +#define XDP_HASH_TYPE_L3_BITS 3
> > >> > +#define XDP_HASH_TYPE_L3_MASK ((1ULL << XDP_HASH_TYPE_L3_BITS)-1)
> > >> > +#define XDP_HASH_TYPE_L3(x) ((x) & XDP_HASH_TYPE_L3_MASK)
> > >> > +enum {
> > >> > + XDP_HASH_TYPE_L3_IPV4 = 1,
> > >> > + XDP_HASH_TYPE_L3_IPV6,
> > >> > +};
> > >> > +
> > >> > +#define XDP_HASH_TYPE_L4_SHIFT XDP_HASH_TYPE_L3_BITS
> > >> > +#define XDP_HASH_TYPE_L4_BITS 5
> > >> > +#define XDP_HASH_TYPE_L4_MASK \
> > >> > + (((1ULL << XDP_HASH_TYPE_L4_BITS)-1) << XDP_HASH_TYPE_L4_SHIFT)
> > >> > +#define XDP_HASH_TYPE_L4(x) ((x) & XDP_HASH_TYPE_L4_MASK)
> > >> > +enum {
> > >> > + _XDP_HASH_TYPE_L4_TCP = 1,
> > >> > + _XDP_HASH_TYPE_L4_UDP,
> > >> > +};
> > >> > +#define XDP_HASH_TYPE_L4_TCP (_XDP_HASH_TYPE_L4_TCP << XDP_HASH_TYPE_L4_SHIFT)
> > >> > +#define XDP_HASH_TYPE_L4_UDP (_XDP_HASH_TYPE_L4_UDP << XDP_HASH_TYPE_L4_SHIFT)
> > >> > +
> > >> Hi Jesper,
> > >>
> > >> Why do we need these indicators for protocol specific hash? It seems
> > >> like L4 and L3 is useful differentiation and protocol agnostic (I'm
> > >> still holding out hope that SCTP will be deployed some day ;-) )
> > >
> > > I'm not sure I understood the question fully, but let me try to answer
> > > anyway. To me it seems obvious that you would want to know the
> > > protocol/L4 type, as this helps avoid hash collisions between UDP and
> > > TCP flows. I can easily imagine someone constructing an UDP packet
> > > that could hash collide with a given TCP flow.
> > >
> > > And yes, i40 support matching SCTP, and we will create a
> > > XDP_HASH_TYPE_L4_SCTP when adding XDP rxhash support for that driver.
> > >
> > But where would this information be used? We don't save it in skbuff,
> > don't use it in RPS, RFS. RSS doesn't use it for packet steering so
> > the hash collision problem already exists at the device level. If
> > there is a collision problem between two protocols then maybe hash
> > should be over 5-tuple instead...
>
> One use-case (I heard at a customer) was that they had (web-)servers
> that didn't serve any UDP traffic, thus they simply block/drop all
> incoming UDP on the service NIC (as an ACL in the switch). (The servers
> own DNS lookups and NTP goes through the management NIC to internal
> DNS/NTP servers).
>
> Another use-case: Inside an XDP/bpf program is can be used for
> splitting protocol processing, into different tail calls, before even
> touching packet-data. I can imagine the bpf TCP handling code is
> larger, thus an optimization is to have a separate tail call for the
> UDP protocol handling. One could also transfer/queue all TCP traffic
> to other CPU(s) like RPS, just without touching packet memory.
>
>
> This info is saved in the skb, but due to space constrains, it is
> reduced to a single bit, namely skb->l4_hash, iif some
> RSS-proto/XDP_HASH_TYPE_L4_* bit was set. And the network stack do use
> and react on this.
I also want to mention another real-customer use-case. Some
deployments have a VXLAN based networks, but some NICs cannot
understand VXLAN do cannot do proper RSS rx-hashing, which resulted in
bad CPU scaling as all VXLAN packets gets delivered to the same CPU.
Thus, I would like to implement recalculation of the RXHASH in XDP, as
that would save me implementing yet another extension to the flow
dissector, that the kernel would have to carry forever, while this is
just a matter of NIC hashing getting improved.
With the extra L3 and L4 info, I'm assuming that XDP_HASH_TYPE_L3(x)
and XDP_HASH_TYPE_L4(x) will be zero for VXLAN as the NIC cannot
identify this. Thus, I can at an early stage know which packets needs
to get a new rxhash.
I've seen a similar problem with Q-in-Q double tagged VLANs, failing
the RSS-hash distribution the same way...
I hope that explains what this can be use for(?)
--
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 net-next 17/20] net: dsa: add notifier for ageing time
From: Vivien Didelot @ 2017-05-22 20:45 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <3d37b2dd-476a-d339-d730-f14037140177@gmail.com>
Hi Florian,
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 05/19/2017 02:00 PM, Vivien Didelot wrote:
>> This patch keeps the port-wide ageing time handling code in
>> dsa_port_ageing_time, pushes the requested ageing time value in a new
>> switch fabric notification, and moves the switch-wide ageing time
>> handling code in dsa_switch_ageing_time.
>>
>> This has the effect that now not only the switch that the target port
>> belongs to can be programmed, but all switches composing the switch
>> fabric. For the moment, keep the current behavior and ignore other
>> switches.
>>
>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>
>> ---
>> net/dsa/dsa_priv.h | 8 ++++++++
>> net/dsa/port.c | 37 ++++++++-----------------------------
>> net/dsa/switch.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 62 insertions(+), 29 deletions(-)
>>
>> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
>> index c19241eb094b..becaf8a61b13 100644
>> --- a/net/dsa/dsa_priv.h
>> +++ b/net/dsa/dsa_priv.h
>> @@ -17,10 +17,18 @@
>> #include <net/dsa.h>
>>
>> enum {
>> + DSA_NOTIFIER_AGEING_TIME,
>> DSA_NOTIFIER_BRIDGE_JOIN,
>> DSA_NOTIFIER_BRIDGE_LEAVE,
>
> This is so we keep sorting notifier events alphabetically, right?
Correct. Thanks for your reviews and test!
Vivien
^ permalink raw reply
* Re: [patch net-next 2/2] net/sched: fix filter flushing
From: Cong Wang @ 2017-05-22 20:54 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Eric Dumazet, Daniel Borkmann, Simon Horman, mlxsw, Colin King
In-Reply-To: <20170521191941.GA4278@nanopsycho>
On Sun, May 21, 2017 at 12:19 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>You can't claim you really delete it as long as actions can still
>>see it and dump it.
>
> No, user just wants to delete all the filters. That is done. User does
> not care if the actual chain structure is there or not.
>
Hmm, so users see a chain with no filters... Fair enough.
^ permalink raw reply
* Re: [PATCH 1/5] sctp: Use kmalloc_array() in sctp_init()
From: Vlad Yasevich @ 2017-05-22 20:57 UTC (permalink / raw)
To: SF Markus Elfring, linux-sctp, netdev, David S. Miller,
Neil Horman, Vlad Yasevich
Cc: LKML, kernel-janitors
In-Reply-To: <24442429-99f7-7d83-5f56-f84776a81880@users.sourceforge.net>
On 05/22/2017 12:37 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 22 May 2017 17:20:11 +0200
>
> * A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "kmalloc_array".
>
> This issue was detected by using the Coccinelle software.
>
> * Replace the specification of a data structure by a pointer dereference
> to make the corresponding size determination a bit safer according to
> the Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/protocol.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 989a900383b5..2b1a6215bd2f 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -1442,6 +1442,6 @@ static __init int sctp_init(void)
>
> /* Allocate and initialize the endpoint hash table. */
> sctp_ep_hashsize = 64;
> - sctp_ep_hashtable =
> - kmalloc(64 * sizeof(struct sctp_hashbucket), GFP_KERNEL);
> + sctp_ep_hashtable = kmalloc_array(64, sizeof(*sctp_ep_hashtable),
> + GFP_KERNEL);
> if (!sctp_ep_hashtable) {
>
^ permalink raw reply
* Re: [PATCH 2/5] sctp: Delete an error message for a failed memory allocation in sctp_init()
From: Vlad Yasevich @ 2017-05-22 20:59 UTC (permalink / raw)
To: SF Markus Elfring, linux-sctp, netdev, David S. Miller,
Neil Horman, Vlad Yasevich
Cc: LKML, kernel-janitors
In-Reply-To: <7db0cb49-109b-88a6-532f-c591cc8ffa85@users.sourceforge.net>
On 05/22/2017 12:38 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 22 May 2017 17:28:14 +0200
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> net/sctp/protocol.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 2b1a6215bd2f..5e7c8a344770 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -1447,5 +1447,4 @@ static __init int sctp_init(void)
> if (!sctp_ep_hashtable) {
> - pr_err("Failed endpoint_hash alloc\n");
> status = -ENOMEM;
> goto err_ehash_alloc;
> }
>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
At the time this was written, it was patterned after TCP. Since then TCP changed
significantly. We can surely clean-up the pr_err() here and possibly update the
code as well later.
-vlad
^ permalink raw reply
* Re: [PATCH 3/5] sctp: Fix a typo in a comment line in sctp_init()
From: Vlad Yasevich @ 2017-05-22 21:00 UTC (permalink / raw)
To: SF Markus Elfring, linux-sctp, netdev, David S. Miller,
Neil Horman, Vlad Yasevich
Cc: LKML, kernel-janitors
In-Reply-To: <9dca8ae4-fd7f-d1de-af7b-87b92bf6411a@users.sourceforge.net>
On 05/22/2017 12:39 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 22 May 2017 17:43:44 +0200
>
> Add a missing character in this description.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> net/sctp/protocol.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 5e7c8a344770..64756c42cec9 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -1454,7 +1454,7 @@ static __init int sctp_init(void)
> }
>
> /* Allocate and initialize the SCTP port hash table.
> - * Note that order is initalized to start at the max sized
> + * Note that order is initialized to start at the max sized
> * table we want to support. If we can't get that many pages
> * reduce the order and try again
> */
>
Acked-by: Vlad Yasevich <vyasevich@gmail.com.
-vlad
^ permalink raw reply
* Re: [PATCH 4/5] sctp: Improve a size determination in sctp_inetaddr_event()
From: Vlad Yasevich @ 2017-05-22 21:00 UTC (permalink / raw)
To: SF Markus Elfring, linux-sctp, netdev, David S. Miller,
Neil Horman, Vlad Yasevich
Cc: LKML, kernel-janitors
In-Reply-To: <a96c883f-0495-ad8c-e3a4-c80f27b76ed2@users.sourceforge.net>
On 05/22/2017 12:40 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 22 May 2017 18:08:24 +0200
>
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> net/sctp/protocol.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 64756c42cec9..057479b7bd72 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -784,7 +784,7 @@ static int sctp_inetaddr_event(struct notifier_block *this, unsigned long ev,
>
> switch (ev) {
> case NETDEV_UP:
> - addr = kmalloc(sizeof(struct sctp_sockaddr_entry), GFP_ATOMIC);
> + addr = kmalloc(sizeof(*addr), GFP_ATOMIC);
> if (addr) {
> addr->a.v4.sin_family = AF_INET;
> addr->a.v4.sin_port = 0;
>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
^ permalink raw reply
* Re: [PATCH 5/5] sctp: Adjust one function call together with a variable assignment
From: Vlad Yasevich @ 2017-05-22 21:00 UTC (permalink / raw)
To: SF Markus Elfring, linux-sctp, netdev, David S. Miller,
Neil Horman, Vlad Yasevich
Cc: LKML, kernel-janitors
In-Reply-To: <197333e8-b528-ef34-bd16-01e415acdf52@users.sourceforge.net>
On 05/22/2017 12:41 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 22 May 2017 18:15:12 +0200
>
> The script "checkpatch.pl" pointed information out like the following.
>
> ERROR: do not use assignment in if condition
>
> Thus fix the affected source code place.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> net/sctp/protocol.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 057479b7bd72..be2fe3ebae78 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -141,7 +141,8 @@ static void sctp_v4_copy_addrlist(struct list_head *addrlist,
> struct sctp_sockaddr_entry *addr;
>
> rcu_read_lock();
> - if ((in_dev = __in_dev_get_rcu(dev)) == NULL) {
> + in_dev = __in_dev_get_rcu(dev);
> + if (!in_dev) {
> rcu_read_unlock();
> return;
> }
>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
^ permalink raw reply
* Re: [PATCH 2/2] at803x: double check SGMII side autoneg
From: Andrew Lunn @ 2017-05-22 21:02 UTC (permalink / raw)
To: Timur Tabi
Cc: Zefir Kurtisi, netdev, f.fainelli, David Miller, Manoj Iyer,
jhugo
In-Reply-To: <66de55da-7a5c-68b8-50d5-cab313ec0d6f@codeaurora.org>
> 2. I'm preparing a patch that adds a command-line parameter to at803x that
> makes this code conditional.
FYI:
A patch with a command line argument, i think you actually mean a
module argument, is very likely to be rejected.
Andrew
^ permalink raw reply
* Re: 4.12-RC2 BUG: scheduling while atomic: irq/47-iwlwifi
From: Arend Van Spriel @ 2017-05-22 21:02 UTC (permalink / raw)
To: Johannes Berg, Sander Eikelenboom
Cc: linux-wireless, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <764a929c-ce8a-c859-a49e-2f20cb05ae44-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
On 22-5-2017 14:09, Arend van Spriel wrote:
> On 5/22/2017 12:57 PM, Johannes Berg wrote:
>> On Mon, 2017-05-22 at 12:36 +0200, Sander Eikelenboom wrote:
>>> Hi,
>>>
>>> I encountered this splat with 4.12-RC2.
>>
>> Ugh, yeah, I should've seen that in the review.
>>
>> Arend, please take a look at this. cfg80211_sched_scan_results() cannot
>> sleep, so you can't rtnl_lock() in there. Looks like you can just rely
>> on RCU though?
>
> I see. I think you are right on RCU. Don't have the code in front of me
> now, but I think the lookup has an ASSERT_RTNL. Will look into it after
> my monday meeting :-p
I realized I have a laptop lying around with intel 3160 wifi chip and
tried to reproduce the issue. Did not run into the splat running
4.12-rc1 from wireless-drivers-next repo. I did not get the email from
Sander so I don't know any details.
Here is what I changed based on the info Johannes provided. Can you
please check if this get rid of the splat and let me know.
Regards,
Arend
---
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 14d5f0c..04833bb 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -322,9 +322,7 @@ static void cfg80211_del_sched_scan_req(struct
cfg80211_regi
{
struct cfg80211_sched_scan_request *pos;
- ASSERT_RTNL();
^ permalink raw reply related
* Re: 4.12-RC2 BUG: scheduling while atomic: irq/47-iwlwifi
From: Johannes Berg @ 2017-05-22 21:04 UTC (permalink / raw)
To: Arend Van Spriel, Sander Eikelenboom; +Cc: linux-wireless, netdev
In-Reply-To: <532c257e-52a0-18c1-1afe-04d37c28e072@broadcom.com>
Hi Arend,
Sorry, I forgot that the original message wasn't Cc'ed to the wireless
list, only netdev.
> +++ b/net/wireless/scan.c
> @@ -322,9 +322,7 @@ static void cfg80211_del_sched_scan_req(struct
> cfg80211_regi
> {
> struct cfg80211_sched_scan_request *pos;
>
> - ASSERT_RTNL();
> -
> - list_for_each_entry(pos, &rdev->sched_scan_req_list, list) {
> + list_for_each_entry_rcu(pos, &rdev->sched_scan_req_list,
> list) {
[snip]
This looks fine, but perhaps in the above we should have some kind of
locking assertion, e.g.
WARN_ON_ONCE(!rcu_read_lock_held() && !lockdep_rtnl_is_held());
johannes
^ permalink raw reply
* Re: [patch net-next 2/2] net/sched: fix filter flushing
From: Cong Wang @ 2017-05-22 21:04 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Eric Dumazet, Daniel Borkmann, Simon Horman, mlxsw, Colin King
In-Reply-To: <CAM_iQpUj8C+Rt_Jm8J_DHXedHAg-_GaKUYgQwQ28hN-=zcB-5Q@mail.gmail.com>
On Mon, May 22, 2017 at 1:54 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Sun, May 21, 2017 at 12:19 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>>You can't claim you really delete it as long as actions can still
>>>see it and dump it.
>>
>> No, user just wants to delete all the filters. That is done. User does
>> not care if the actual chain structure is there or not.
>>
>
> Hmm, so users see a chain with no filters... Fair enough.
But since you remove the chain from the chain_list, it means
users could not add new filters to this chain after flushing? And
users could create a new chain with the same index??
If so, you should instead keep it in the chain_list, although empty.
^ permalink raw reply
* Re: [PATCH 2/2] at803x: double check SGMII side autoneg
From: Andrew Lunn @ 2017-05-22 21:09 UTC (permalink / raw)
To: Timur Tabi
Cc: Zefir Kurtisi, netdev, f.fainelli, David Miller, Manoj Iyer,
jhugo
In-Reply-To: <66de55da-7a5c-68b8-50d5-cab313ec0d6f@codeaurora.org>
On Mon, May 22, 2017 at 03:12:03PM -0500, Timur Tabi wrote:
> On 10/24/2016 05:40 AM, Zefir Kurtisi wrote:
> > This commit adds a wrapper function for at8031
> > that in case of operating in SGMII mode double
> > checks SGMII link state when generic aneg_done()
> > succeeds. It prints a warning on failure but
> > intentionally does not try to recover from this
> > state. As a result, if you ever see a warning
> > '803x_aneg_done: SGMII link is not ok' you will
> > end up having an Ethernet link up but won't get
> > any data through. This should not happen, if it
> > does, please contact the module maintainer.
>
> I'm getting bitten by this one again. We're now have several systems that
> are reporting the link failure ("803x_aneg_done: SGMII link is not ok"), and
> the interface comes up but is not functional. I believe this is expected.
>
> The problem, however, is not because of the link failure. Instead, the
> problem is this:
>
> > + /* check if the SGMII link is OK. */
> > + if (!(phy_read(phydev, AT803X_PSSR) & AT803X_PSSR_MR_AN_COMPLETE)) {
> > + pr_warn("803x_aneg_done: SGMII link is not ok\n");
> > + aneg_done = 0;
>
> Returning zero is what breaks the interface. If I comment-out this last
> line, so that at803x_aneg_done() returns BMSR_ANEGCOMPLETE instead, then
> everything works.
Are you using interrupts? Or polling?
If polling, it should come back again 1 second later and see if
auto-neg has completed. Hopefully the SGMII side comes up eventually.
If you are using interrupts, you need another interrupt when the SGMII
side comes up, otherwise i think the state machine is stuck waiting.
Andrew
^ permalink raw reply
* Re: [PATCH 2/2] at803x: double check SGMII side autoneg
From: Florian Fainelli @ 2017-05-22 21:10 UTC (permalink / raw)
To: Andrew Lunn, Timur Tabi
Cc: Zefir Kurtisi, netdev, David Miller, Manoj Iyer, jhugo
In-Reply-To: <20170522210215.GQ29447@lunn.ch>
On 05/22/2017 02:02 PM, Andrew Lunn wrote:
>> 2. I'm preparing a patch that adds a command-line parameter to at803x that
>> makes this code conditional.
>
> FYI:
>
> A patch with a command line argument, i think you actually mean a
> module argument, is very likely to be rejected.
Even a module argument would be rejected. If you need platform/SoC
specific behavior propagated down to the PHY driver, several options exist:
- pass an agreed upon value for phy_flags to of_phy_connect() see
drivers/net/ethernet/broadcom/tg3.c and
drivers/net/ethernet/broadcom/genet/bcmgenet.c for instance and update
the driver to act on that "flags" see drivers/net/phy/broadcom.c and
drivers/net/phy/bcm7xxx.c
- register a PHY fixup which is specific to the board/SoC, and have the
PHY fixup do whatever is necessary for your platform (like setting
specific registers)
Preference goes for the first solution, but phy_flags is just a 32-bits
integer, so you could run out of bits.
--
Florian
^ permalink raw reply
* Re: [PATCH 2/2] at803x: double check SGMII side autoneg
From: Timur Tabi @ 2017-05-22 21:19 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn
Cc: Zefir Kurtisi, netdev, David Miller, Manoj Iyer, jhugo
In-Reply-To: <0528e1fa-2924-1286-578d-060419bfee49@gmail.com>
On 05/22/2017 04:10 PM, Florian Fainelli wrote:
> Even a module argument would be rejected. If you need platform/SoC
> specific behavior propagated down to the PHY driver, several options exist:
>
> - pass an agreed upon value for phy_flags to of_phy_connect() see
> drivers/net/ethernet/broadcom/tg3.c and
> drivers/net/ethernet/broadcom/genet/bcmgenet.c for instance and update
> the driver to act on that "flags" see drivers/net/phy/broadcom.c and
> drivers/net/phy/bcm7xxx.c
Will this work on ACPI systems as well? I call phy_connect_direct() instead
of of_phy_connect(). I see some drivers set phydev->dev_flags before
calling phy_connect_direct().
My concern is that this problem occurs only on an at8031 chip, so having my
network driver passing an at8031-specific flag seems out of place. What
happens if, on some other board, a different PHY is used, and that flag
means something else?
> - register a PHY fixup which is specific to the board/SoC, and have the
> PHY fixup do whatever is necessary for your platform (like setting
> specific registers)
Do you have an example of that?
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH net-next] net: make struct inet_frags::qsize unsigned
From: Alexey Dobriyan @ 2017-05-22 21:20 UTC (permalink / raw)
To: davem; +Cc: netdev
This field is sizeof of corresponding kmem_cache so it can't be negative.
Prepare for 32-bit kmem_cache_create().
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
include/net/inet_frag.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -92,7 +92,7 @@ struct inet_frags {
*/
u32 rnd;
seqlock_t rnd_seqlock;
- int qsize;
+ unsigned int qsize;
unsigned int (*hashfn)(const struct inet_frag_queue *);
bool (*match)(const struct inet_frag_queue *q,
^ permalink raw reply
* RE: [PATCH v3 net-next 3/5] dsa: add DSA switch driver for Microchip KSZ9477
From: Woojung.Huh @ 2017-05-22 21:21 UTC (permalink / raw)
To: andrew
Cc: f.fainelli, vivien.didelot, sergei.shtylyov, netdev, davem,
UNGLinuxDriver
In-Reply-To: <20170522143134.GC29447@lunn.ch>
Hi Andres,
> > +static struct {
> > + int index;
> > + char string[ETH_GSTRING_LEN];
>
> Hi Woojung
>
> Since you need to respin for the skb_put_padto(), please make this
> const.
OK.
> > +static int get_vlan_table(struct dsa_switch *ds, u16 vid, u32 *vlan_table)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > + u8 data;
> > + int timeout = 1000;
> > +
> > + ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid &
> VLAN_INDEX_M);
> > + ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_READ | VLAN_START);
> > +
> > + /* wait to be cleared */
> > + data = 0;
> > + do {
> > + ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
> > + if (!(data & VLAN_START))
> > + break;
> > + usleep_range(1, 10);
> > + } while (timeout-- > 0);
> > +
> > + if (!timeout)
> > + return -ETIMEDOUT;
> > +
> > + ksz_read32(dev, REG_SW_VLAN_ENTRY__4, &vlan_table[0]);
> > + ksz_read32(dev, REG_SW_VLAN_ENTRY_UNTAG__4,
> &vlan_table[1]);
> > + ksz_read32(dev, REG_SW_VLAN_ENTRY_PORTS__4, &vlan_table[2]);
> > +
> > + ksz_write8(dev, REG_SW_VLAN_CTRL, 0);
> > +
> > + return 0;
> > +}
> > +
> > +static int set_vlan_table(struct dsa_switch *ds, u16 vid, u32 *vlan_table)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > + u8 data;
> > + int timeout = 1000;
> > +
> > + ksz_write32(dev, REG_SW_VLAN_ENTRY__4, vlan_table[0]);
> > + ksz_write32(dev, REG_SW_VLAN_ENTRY_UNTAG__4, vlan_table[1]);
> > + ksz_write32(dev, REG_SW_VLAN_ENTRY_PORTS__4, vlan_table[2]);
> > +
> > + ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid &
> VLAN_INDEX_M);
> > + ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_START | VLAN_WRITE);
> > +
> > + do {
> > + ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
> > + if (!(data & VLAN_START))
> > + break;
> > + usleep_range(1, 10);
> > + } while (timeout-- > 0);
> > +
> > + if (!timeout)
> > + return -ETIMEDOUT;
> > +
> > + ksz_write8(dev, REG_SW_VLAN_CTRL, 0);
> > +
> > + mutex_lock(&dev->vlancache_mutex);
>
> Humm. I think this is wrong. Shouldn't you hold the mutex while you
> change the hardware as well as the cache. Otherwise there is a risk
> your cache could be different to the hardware when you get a race
> between two threads?
Thanks for pointing this out.
Rather than two separate mutex (H/W and vlancache), will put one HW access mutex
around get_vlan_table and set_vlan_table to cover vlancache access too. Even though
little bit overhead. How do you think?
- Woojung
^ permalink raw reply
* [PATCH] net: make struct request_sock_ops::obj_size unsigned
From: Alexey Dobriyan @ 2017-05-22 21:21 UTC (permalink / raw)
To: davem; +Cc: netdev
This field is sizeof of corresponding kmem_cache so it can't be negative.
Space will be saved after 32-bit kmem_cache_create() patch.
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
include/net/request_sock.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/include/net/request_sock.h
+++ b/include/net/request_sock.h
@@ -29,7 +29,7 @@ struct proto;
struct request_sock_ops {
int family;
- int obj_size;
+ unsigned int obj_size;
struct kmem_cache *slab;
char *slab_name;
int (*rtx_syn_ack)(const struct sock *sk,
^ permalink raw reply
* Re: [PATCH] net/core: remove explicit do_softirq() from busy_poll_stop()
From: Eric Dumazet @ 2017-05-22 21:26 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: netdev, David S. Miller, Thomas Gleixner
In-Reply-To: <20170522192641.hylqmhb7t2fykk5e@linutronix.de>
On Mon, May 22, 2017 at 12:26 PM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> Since commit 217f69743681 ("net: busy-poll: allow preemption in
> sk_busy_loop()") there is an explicit do_softirq() invocation after
> local_bh_enable() has been invoked.
> I don't understand why we need this because local_bh_enable() will
> invoke do_softirq() once the softirq counter reached zero and we have
> softirq-related work pending.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> net/core/dev.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index fca407b4a6ea..e84eb0ec5529 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5199,8 +5199,6 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock)
> if (rc == BUSY_POLL_BUDGET)
> __napi_schedule(napi);
> local_bh_enable();
> - if (local_softirq_pending())
> - do_softirq();
> }
preemption is disabled.
Look at netif_rx_ni() for a similar construct.
What exact problem do you have with existing code, that is worth
adding this change ?
Thanks.
^ permalink raw reply
* Re: [PATCH v3 net-next 3/5] dsa: add DSA switch driver for Microchip KSZ9477
From: Andrew Lunn @ 2017-05-22 21:27 UTC (permalink / raw)
To: Woojung.Huh
Cc: f.fainelli, vivien.didelot, sergei.shtylyov, netdev, davem,
UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A81210@CHN-SV-EXMX02.mchp-main.com>
On Mon, May 22, 2017 at 09:21:28PM +0000, Woojung.Huh@microchip.com wrote:
> Hi Andres,
>
> > > +static struct {
> > > + int index;
> > > + char string[ETH_GSTRING_LEN];
> >
> > Hi Woojung
> >
> > Since you need to respin for the skb_put_padto(), please make this
> > const.
> OK.
>
> > > +static int get_vlan_table(struct dsa_switch *ds, u16 vid, u32 *vlan_table)
> > > +{
> > > + struct ksz_device *dev = ds->priv;
> > > + u8 data;
> > > + int timeout = 1000;
> > > +
> > > + ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid &
> > VLAN_INDEX_M);
> > > + ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_READ | VLAN_START);
> > > +
> > > + /* wait to be cleared */
> > > + data = 0;
> > > + do {
> > > + ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
> > > + if (!(data & VLAN_START))
> > > + break;
> > > + usleep_range(1, 10);
> > > + } while (timeout-- > 0);
> > > +
> > > + if (!timeout)
> > > + return -ETIMEDOUT;
> > > +
> > > + ksz_read32(dev, REG_SW_VLAN_ENTRY__4, &vlan_table[0]);
> > > + ksz_read32(dev, REG_SW_VLAN_ENTRY_UNTAG__4,
> > &vlan_table[1]);
> > > + ksz_read32(dev, REG_SW_VLAN_ENTRY_PORTS__4, &vlan_table[2]);
> > > +
> > > + ksz_write8(dev, REG_SW_VLAN_CTRL, 0);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int set_vlan_table(struct dsa_switch *ds, u16 vid, u32 *vlan_table)
> > > +{
> > > + struct ksz_device *dev = ds->priv;
> > > + u8 data;
> > > + int timeout = 1000;
> > > +
> > > + ksz_write32(dev, REG_SW_VLAN_ENTRY__4, vlan_table[0]);
> > > + ksz_write32(dev, REG_SW_VLAN_ENTRY_UNTAG__4, vlan_table[1]);
> > > + ksz_write32(dev, REG_SW_VLAN_ENTRY_PORTS__4, vlan_table[2]);
> > > +
> > > + ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid &
> > VLAN_INDEX_M);
> > > + ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_START | VLAN_WRITE);
> > > +
> > > + do {
> > > + ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
> > > + if (!(data & VLAN_START))
> > > + break;
> > > + usleep_range(1, 10);
> > > + } while (timeout-- > 0);
> > > +
> > > + if (!timeout)
> > > + return -ETIMEDOUT;
> > > +
> > > + ksz_write8(dev, REG_SW_VLAN_CTRL, 0);
> > > +
> > > + mutex_lock(&dev->vlancache_mutex);
> >
> > Humm. I think this is wrong. Shouldn't you hold the mutex while you
> > change the hardware as well as the cache. Otherwise there is a risk
> > your cache could be different to the hardware when you get a race
> > between two threads?
> Thanks for pointing this out.
> Rather than two separate mutex (H/W and vlancache), will put one HW access mutex
> around get_vlan_table and set_vlan_table to cover vlancache access too. Even though
> little bit overhead. How do you think?
I would move the mutex_lock(&dev->vlancache_mutex) to be beginning of
the function. It then protects both the hardware and the vlan cache,
and keeps them synchronised.
Andrew
^ permalink raw reply
* Re: [PATCH 2/2] at803x: double check SGMII side autoneg
From: Timur Tabi @ 2017-05-22 21:29 UTC (permalink / raw)
To: Andrew Lunn
Cc: Zefir Kurtisi, netdev, f.fainelli, David Miller, Manoj Iyer,
jhugo
In-Reply-To: <20170522210927.GR29447@lunn.ch>
On 05/22/2017 04:09 PM, Andrew Lunn wrote:
> Are you using interrupts? Or polling?
adpt->phydev->irq = PHY_IGNORE_INTERRUPT;
ret = phy_connect_direct(netdev, adpt->phydev, emac_adjust_link,
PHY_INTERFACE_MODE_SGMII);
Technically it's polling, except that it's my NIC's hardware that is polling
the MDIO bus, and then generating an interrupt when there's a link state change.
When the link state changes, I call phy_mac_interrupt()
if (status & ISR_GPHY_LINK)
phy_mac_interrupt(adpt->phydev, !!(status & GPHY_LINK_UP_INT));
> If polling, it should come back again 1 second later and see if
> auto-neg has completed. Hopefully the SGMII side comes up eventually.
>
> If you are using interrupts, you need another interrupt when the SGMII
> side comes up, otherwise i think the state machine is stuck waiting.
I'll have to test this, but what do I do if I don't get another interrupt?
I have a suspicion that the link is actually okay, and that the error is bogus.
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers
From: Tom Herbert @ 2017-05-22 21:32 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Daniel Borkmann, Alexei Starovoitov,
Linux Kernel Network Developers
In-Reply-To: <20170522224229.540ce059@redhat.com>
On Mon, May 22, 2017 at 1:42 PM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Mon, 22 May 2017 08:39:35 +0200
> Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>
>> On Sun, 21 May 2017 15:10:29 -0700
>> Tom Herbert <tom@herbertland.com> wrote:
>>
>> > On Sun, May 21, 2017 at 9:04 AM, Jesper Dangaard Brouer
>> > <brouer@redhat.com> wrote:
>> > > On Sat, 20 May 2017 09:16:09 -0700
>> > > Tom Herbert <tom@herbertland.com> wrote:
>> > >
>> > >> > +/* XDP rxhash have an associated type, which is related to the RSS
>> > >> > + * (Receive Side Scaling) standard, but NIC HW have different mapping
>> > >> > + * and support. Thus, create mapping that is interesting for XDP. XDP
>> > >> > + * would primarly want insight into L3 and L4 protocol info.
>> > >> > + *
>> > >> > + * TODO: Likely need to get extended with "L3_IPV6_EX" due RSS standard
>> > >> > + *
>> > >> > + * The HASH_TYPE will be returned from bpf helper as the top 32-bit of
>> > >> > + * the 64-bit rxhash (internally type stored in xdp_buff->flags).
>> > >> > + */
>> > >> > +#define XDP_HASH(x) ((x) & ((1ULL << 32)-1))
>> > >> > +#define XDP_HASH_TYPE(x) ((x) >> 32)
>> > >> > +
>> > >> > +#define XDP_HASH_TYPE_L3_SHIFT 0
>> > >> > +#define XDP_HASH_TYPE_L3_BITS 3
>> > >> > +#define XDP_HASH_TYPE_L3_MASK ((1ULL << XDP_HASH_TYPE_L3_BITS)-1)
>> > >> > +#define XDP_HASH_TYPE_L3(x) ((x) & XDP_HASH_TYPE_L3_MASK)
>> > >> > +enum {
>> > >> > + XDP_HASH_TYPE_L3_IPV4 = 1,
>> > >> > + XDP_HASH_TYPE_L3_IPV6,
>> > >> > +};
>> > >> > +
>> > >> > +#define XDP_HASH_TYPE_L4_SHIFT XDP_HASH_TYPE_L3_BITS
>> > >> > +#define XDP_HASH_TYPE_L4_BITS 5
>> > >> > +#define XDP_HASH_TYPE_L4_MASK \
>> > >> > + (((1ULL << XDP_HASH_TYPE_L4_BITS)-1) << XDP_HASH_TYPE_L4_SHIFT)
>> > >> > +#define XDP_HASH_TYPE_L4(x) ((x) & XDP_HASH_TYPE_L4_MASK)
>> > >> > +enum {
>> > >> > + _XDP_HASH_TYPE_L4_TCP = 1,
>> > >> > + _XDP_HASH_TYPE_L4_UDP,
>> > >> > +};
>> > >> > +#define XDP_HASH_TYPE_L4_TCP (_XDP_HASH_TYPE_L4_TCP << XDP_HASH_TYPE_L4_SHIFT)
>> > >> > +#define XDP_HASH_TYPE_L4_UDP (_XDP_HASH_TYPE_L4_UDP << XDP_HASH_TYPE_L4_SHIFT)
>> > >> > +
>> > >> Hi Jesper,
>> > >>
>> > >> Why do we need these indicators for protocol specific hash? It seems
>> > >> like L4 and L3 is useful differentiation and protocol agnostic (I'm
>> > >> still holding out hope that SCTP will be deployed some day ;-) )
>> > >
>> > > I'm not sure I understood the question fully, but let me try to answer
>> > > anyway. To me it seems obvious that you would want to know the
>> > > protocol/L4 type, as this helps avoid hash collisions between UDP and
>> > > TCP flows. I can easily imagine someone constructing an UDP packet
>> > > that could hash collide with a given TCP flow.
>> > >
>> > > And yes, i40 support matching SCTP, and we will create a
>> > > XDP_HASH_TYPE_L4_SCTP when adding XDP rxhash support for that driver.
>> > >
>> > But where would this information be used? We don't save it in skbuff,
>> > don't use it in RPS, RFS. RSS doesn't use it for packet steering so
>> > the hash collision problem already exists at the device level. If
>> > there is a collision problem between two protocols then maybe hash
>> > should be over 5-tuple instead...
>>
>> One use-case (I heard at a customer) was that they had (web-)servers
>> that didn't serve any UDP traffic, thus they simply block/drop all
>> incoming UDP on the service NIC (as an ACL in the switch). (The servers
>> own DNS lookups and NTP goes through the management NIC to internal
>> DNS/NTP servers).
>>
>> Another use-case: Inside an XDP/bpf program is can be used for
>> splitting protocol processing, into different tail calls, before even
>> touching packet-data. I can imagine the bpf TCP handling code is
>> larger, thus an optimization is to have a separate tail call for the
>> UDP protocol handling. One could also transfer/queue all TCP traffic
>> to other CPU(s) like RPS, just without touching packet memory.
>>
>>
>> This info is saved in the skb, but due to space constrains, it is
>> reduced to a single bit, namely skb->l4_hash, iif some
>> RSS-proto/XDP_HASH_TYPE_L4_* bit was set. And the network stack do use
>> and react on this.
>
> I also want to mention another real-customer use-case. Some
> deployments have a VXLAN based networks, but some NICs cannot
> understand VXLAN do cannot do proper RSS rx-hashing, which resulted in
> bad CPU scaling as all VXLAN packets gets delivered to the same CPU.
>
They need to turn on RSS for UDP I think.
> Thus, I would like to implement recalculation of the RXHASH in XDP, as
> that would save me implementing yet another extension to the flow
> dissector, that the kernel would have to carry forever, while this is
> just a matter of NIC hashing getting improved.
>
> With the extra L3 and L4 info, I'm assuming that XDP_HASH_TYPE_L3(x)
> and XDP_HASH_TYPE_L4(x) will be zero for VXLAN as the NIC cannot
> identify this. Thus, I can at an early stage know which packets needs
> to get a new rxhash.
>
> I've seen a similar problem with Q-in-Q double tagged VLANs, failing
> the RSS-hash distribution the same way...
>
That's fine, but I'm still not seeing that differentiating the hash
created from different protocols is really helps. The most correct way
moving forward to generate a flow hash is use IPv6 flow label. This
eliminates either the device or software from needing to do DPI into
protocols, over extension headers, etc. In this case though we declare
a hash based on flow label as L4, but have no idea and really don't
care what protocol it's for (and in fact for something like IPsec we
can't even know).
The idea that the world is composed of just TCP and UDP is a delusion
espoused by NIC implementations, in SW we can and should do better
than that with protocol agnostic mechanisms.
Tom
> I hope that explains what this can be use for(?)
>
> --
> 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 2/2] at803x: double check SGMII side autoneg
From: Andrew Lunn @ 2017-05-22 21:32 UTC (permalink / raw)
To: Timur Tabi
Cc: Zefir Kurtisi, netdev, f.fainelli, David Miller, Manoj Iyer,
jhugo
In-Reply-To: <0d9b30e5-40f3-82ba-a44a-2eb25b8d3094@codeaurora.org>
> I'll have to test this, but what do I do if I don't get another interrupt?
It probably means interrupts cannot be used. Poll it.
Andrew
^ permalink raw reply
* Re: brcmfmac firmware issue on NanoPi K2
From: Franky Lin @ 2017-05-22 21:39 UTC (permalink / raw)
To: Andreas Färber
Cc: linux-wireless,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER, netdev,
technicalsupport, Arend Van Spriel, linux-amlogic
In-Reply-To: <ab713116-ab12-5f87-1ecf-cae2e0d68b93@suse.de>
Hi Andreas,
On Sun, May 21, 2017 at 9:20 AM, Andreas Färber <afaerber@suse.de> wrote:
> Hello,
>
> The NanoPi K2 has an Ampak AP6212 SDIO module. brcmfmac driver loads
> brcmfmac43430-sdio.bin.
>
> When using the firmware file from linux-firmware.git that openSUSE ships
> I get the following errors on 4.11.0 and next-20170519:
>
> [ 2103.618716] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000):
> clkctl 0x50
> [ 2104.668746] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000):
> clkctl 0x50
> [ 2105.678677] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000):
> clkctl 0x50
>
> If I overwrite /lib/firmware/brcm/bcm43430-sdio.bin with
> fw_bcm43438a0.bin from FriendlyARM's Android repository it suddenly works:
>
> [ +0.157738] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0:
> Jun 6 2014 14:50:39 version 7.10.226.49 (r) FWID 01-8962686a
> [ +0.160108] brcmfmac: brcmf_cfg80211_reg_notifier: not a ISO3166 code
> (0x30 0x30)
>
> I recall using the linux-firmware.git brcmfmac43430-sdio.bin file
> successfully on the Raspberry Pi 3 with a downstream (Leap 42.2) kernel.
>
> I've tested both nvram_ap6212.txt and nvram_ap6212a.txt, the latter has
> the following diff to nvram.txt:
>
> --- nvram_ap6212.txt 2017-05-21 04:24:40.372113426 +0200
> +++ nvram_ap6212a.txt 2017-05-21 04:24:49.852116599 +0200
> @@ -1,4 +1,4 @@
> -#AP6212_NVRAM_V1.0_20140603
> +#AP6212_NVRAM_V1.0.1_20160606
> # 2.4 GHz, 20 MHz BW mode
>
> # The following parameter values are just placeholders, need to be updated.
> @@ -51,4 +51,4 @@
> muxenab=0x10
> # CLDO PWM voltage settings - 0x4 - 1.1 volt
> #cldo_pwm=0x4
> -
> +glitch_based_crsmin=1
>
> https://github.com/friendlyarm/android_hardware_amlogic_wifi/tree/l-amlogic-gx-sync/bcm_ampak/config/6212
>
> * Does the linux-firmware.git brcmfmac43430-sdio.bin need a fix for AP6212?
> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/brcm
>
> * Does the brcmfmac driver need to distinguish revisions in sdio.c as
> done for 43241, plus a separate firmware file?
> BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43430_CHIP_ID, 0xFFFFFFFF, 43430),
It seems we are dealing with different revisions of 43430. The
firmware file you pointed to is for 43430a0 and raspberry pi 3 is
using 43430a1. So yes brcmfmac needs to load different firmware and
nvram for them just like 43241.
Could you please help try the attached patch? I don't have the
hardware to test it. Please rename the 43438 firmware and nvram to
brcmfmac43430a0-sdio.bin/txt.
Thanks,
Franky
---
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index a999f95..1b12ccb 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -608,6 +608,7 @@ BRCMF_FW_NVRAM_DEF(43340,
"brcmfmac43340-sdio.bin", "brcmfmac43340-sdio.txt");
BRCMF_FW_NVRAM_DEF(4335, "brcmfmac4335-sdio.bin", "brcmfmac4335-sdio.txt");
BRCMF_FW_NVRAM_DEF(43362, "brcmfmac43362-sdio.bin", "brcmfmac43362-sdio.txt");
BRCMF_FW_NVRAM_DEF(4339, "brcmfmac4339-sdio.bin", "brcmfmac4339-sdio.txt");
+BRCMF_FW_NVRAM_DEF(43430A0, "brcmfmac43430a0-sdio.bin",
"brcmfmac43430a0-sdio.txt");
BRCMF_FW_NVRAM_DEF(43430, "brcmfmac43430-sdio.bin", "brcmfmac43430-sdio.txt");
BRCMF_FW_NVRAM_DEF(43455, "brcmfmac43455-sdio.bin", "brcmfmac43455-sdio.txt");
BRCMF_FW_NVRAM_DEF(4354, "brcmfmac4354-sdio.bin", "brcmfmac4354-sdio.txt");
@@ -626,7 +627,8 @@ static struct brcmf_firmware_mapping
brcmf_sdio_fwnames[] = {
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4335_CHIP_ID, 0xFFFFFFFF, 4335),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43362_CHIP_ID, 0xFFFFFFFE, 43362),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4339_CHIP_ID, 0xFFFFFFFF, 4339),
- BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43430_CHIP_ID, 0xFFFFFFFF, 43430),
+ BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43430_CHIP_ID, 0x00000001, 43430A0),
+ BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43430_CHIP_ID, 0xFFFFFFFE, 43430),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4345_CHIP_ID, 0xFFFFFFC0, 43455),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4354_CHIP_ID, 0xFFFFFFFF, 4354),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4356_CHIP_ID, 0xFFFFFFFF, 4356)
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox