* [patch net-next repost 1/8] devlink: Add IPv6 header for dpipe
From: Jiri Pirko @ 2017-08-31 15:59 UTC (permalink / raw)
To: netdev; +Cc: davem, arkadis, idosch, mlxsw
In-Reply-To: <20170831155919.1366-1-jiri@resnulli.us>
From: Arkadi Sharshevsky <arkadis@mellanox.com>
This will be used by the IPv6 host table which will be introduced in the
following patches. The fields in the header are added per-use. This header
is global and can be reused by many drivers.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/devlink.h | 1 +
include/uapi/linux/devlink.h | 5 +++++
net/core/devlink.c | 17 +++++++++++++++++
3 files changed, 23 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index aaf7178..b9654e1 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -330,6 +330,7 @@ int devlink_dpipe_match_put(struct sk_buff *skb,
struct devlink_dpipe_match *match);
extern struct devlink_dpipe_header devlink_dpipe_header_ethernet;
extern struct devlink_dpipe_header devlink_dpipe_header_ipv4;
+extern struct devlink_dpipe_header devlink_dpipe_header_ipv6;
#else
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 6c17254..0cbca96 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -234,9 +234,14 @@ enum devlink_dpipe_field_ipv4_id {
DEVLINK_DPIPE_FIELD_IPV4_DST_IP,
};
+enum devlink_dpipe_field_ipv6_id {
+ DEVLINK_DPIPE_FIELD_IPV6_DST_IP,
+};
+
enum devlink_dpipe_header_id {
DEVLINK_DPIPE_HEADER_ETHERNET,
DEVLINK_DPIPE_HEADER_IPV4,
+ DEVLINK_DPIPE_HEADER_IPV6,
};
#endif /* _UAPI_LINUX_DEVLINK_H_ */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index cbc4b04..7d430c1 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -63,6 +63,23 @@ struct devlink_dpipe_header devlink_dpipe_header_ipv4 = {
};
EXPORT_SYMBOL(devlink_dpipe_header_ipv4);
+static struct devlink_dpipe_field devlink_dpipe_fields_ipv6[] = {
+ {
+ .name = "destination ip",
+ .id = DEVLINK_DPIPE_FIELD_IPV6_DST_IP,
+ .bitwidth = 128,
+ },
+};
+
+struct devlink_dpipe_header devlink_dpipe_header_ipv6 = {
+ .name = "ipv6",
+ .id = DEVLINK_DPIPE_HEADER_IPV6,
+ .fields = devlink_dpipe_fields_ipv6,
+ .fields_count = ARRAY_SIZE(devlink_dpipe_fields_ipv6),
+ .global = true,
+};
+EXPORT_SYMBOL(devlink_dpipe_header_ipv6);
+
EXPORT_TRACEPOINT_SYMBOL_GPL(devlink_hwmsg);
static LIST_HEAD(devlink_list);
--
2.9.3
^ permalink raw reply related
* [patch net-next repost 0/8] mlxsw: Add IPv6 host dpipe table
From: Jiri Pirko @ 2017-08-31 15:59 UTC (permalink / raw)
To: netdev; +Cc: davem, arkadis, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
This patchset adds IPv6 host dpipe table support. This will provide the
ability to observe the hardware offloaded IPv6 neighbors.
Arkadi Sharshevsky (8):
devlink: Add IPv6 header for dpipe
mlxsw: spectrum_router: Export IPv6 link local address check helper
mlxsw: spectrum_dpipe: Add IPv6 host table initial support
mlxsw: spectrum_router: Add IPv6 neighbor access helper
mlxsw: spectrum_dpipe: Make host entry fill handler more generic
mlxsw: spectrum_dpipe: Add support for IPv6 host table dump
mlxsw: spectrum_router: Add support for setting counters on IPv6
neighbors
mlxsw: spectrum_dpipe: Add support for controlling IPv6 neighbor
counters
.../net/ethernet/mellanox/mlxsw/spectrum_dpipe.c | 182 ++++++++++++++++++---
.../net/ethernet/mellanox/mlxsw/spectrum_dpipe.h | 1 +
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 37 ++++-
.../net/ethernet/mellanox/mlxsw/spectrum_router.h | 3 +
include/net/devlink.h | 1 +
include/uapi/linux/devlink.h | 5 +
net/core/devlink.c | 17 ++
7 files changed, 219 insertions(+), 27 deletions(-)
--
2.9.3
^ permalink raw reply
* Re: [RFC PATCH] net: frag limit checks need to use percpu_counter_compare
From: Stephen Hemminger @ 2017-08-31 15:58 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: liujian56, netdev, Florian Westphal
In-Reply-To: <150417481955.28907.15567119824187929000.stgit@firesoul>
On Thu, 31 Aug 2017 12:20:19 +0200
Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> +static inline bool frag_mem_over_limit(struct netns_frags *nf, int thresh)
> {
> - return percpu_counter_read(&nf->mem);
> + /* When reading counter here, __percpu_counter_compare() call
> + * will invoke __percpu_counter_sum() when needed. Which
> + * depend on num_online_cpus()*batch size, as each CPU can
> + * potentential can hold a batch count.
> + *
> + * With many CPUs this heavier sum operation will
> + * unfortunately always occur.
> + */
> + if (__percpu_counter_compare(&nf->mem, thresh,
> + frag_percpu_counter_batch) > 0)
> + return true;
> + else
> + return false;
You don't need an if() here.
return __percpu_counter_compare(&nf->mem, thresh,
frag_percpu_counter_batch) > 0;
^ permalink raw reply
* Re: [PATCH net-next] samples/bpf: Fix compilation issue in redirect dummy program
From: Y Song @ 2017-08-31 15:54 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Jesper Dangaard Brouer, Tariq Toukan, David S. Miller, netdev,
Eran Ben Elisha, Alexei Starovoitov
In-Reply-To: <59A7F658.4020606@iogearbox.net>
On Thu, Aug 31, 2017 at 4:43 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 08/31/2017 01:27 PM, Jesper Dangaard Brouer wrote:
>>
>> On Thu, 31 Aug 2017 14:16:39 +0300
>> Tariq Toukan <tariqt@mellanox.com> wrote:
>>
>>> Fix compilation error below:
>>>
>>> $ make samples/bpf/
>>>
>>> LLVM ERROR: 'xdp_redirect_dummy' label emitted multiple times to
>>> assembly file
>>> make[1]: *** [samples/bpf/xdp_redirect_kern.o] Error 1
>>> make: *** [samples/bpf/] Error 2
>>>
>>> Fixes: 306da4e685b4 ("samples/bpf: xdp_redirect load XDP dummy prog on TX
>>> device")
>>> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
>>> ---
>>
>>
>> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
>
>> What LLVM/clang version do you use?
>>
>> I don't see this compile error, and I have:
>> $ clang --version
>> clang version 3.9.1 (tags/RELEA
>
>
> I'm seeing the error as well with a fairly recent LLVM from git
> tree (6.0.0git-2d810c2).
>
> Looks like the llvm error is triggered when section name and
> the function name for XDP prog is the same. Changing either the
> function or the section name right above resolves the issue. If
> such error didn't trigger on older versions, people could be
> using such naming scheme as done here, so seems to me like a
> regression on LLVM side we might need to look at ...
Martin fixed a similar bug earlier:
=====
commit a2e8bbd2ef5457485f00b6b947bbbfa2778e5b1e
Author: Martin KaFai Lau <kafai@fb.com>
Date: Thu Jun 8 22:30:17 2017 -0700
bpf: Fix test_obj_id.c for llvm 5.0
llvm 5.0 does not like the section name and the function name
to be the same:
...
=====
gcc also has this behavior. Section name is treated as global
and hence cannot collide with a function name...
^ permalink raw reply
* Re: [PATCH v2 net-next 4/6] udp: flow dissector offload
From: Willem de Bruijn @ 2017-08-31 15:53 UTC (permalink / raw)
To: Paolo Abeni; +Cc: Tom Herbert, David Miller, Network Development
In-Reply-To: <1504089372.2480.55.camel@redhat.com>
On Wed, Aug 30, 2017 at 6:36 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> On Tue, 2017-08-29 at 16:27 -0700, Tom Herbert wrote:
>> Add support to perform UDP specific flow dissection. This is
>> primarily intended for dissecting encapsulated packets in UDP
>> encapsulation.
>>
>> This patch adds a flow_dissect offload for UDP4 and UDP6. The backend
>> function performs a socket lookup and calls the flow_dissect function
>> if a socket is found.
>>
>> Signed-off-by: Tom Herbert <tom@quantonium.net>
>> ---
>> include/linux/udp.h | 8 ++++++++
>> include/net/udp.h | 8 ++++++++
>> include/net/udp_tunnel.h | 8 ++++++++
>> net/ipv4/udp_offload.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>> net/ipv4/udp_tunnel.c | 1 +
>> net/ipv6/udp_offload.c | 13 +++++++++++++
>> 6 files changed, 83 insertions(+)
>>
>> diff --git a/include/linux/udp.h b/include/linux/udp.h
>> index eaea63bc79bb..2e90b189ef6a 100644
>> --- a/include/linux/udp.h
>> +++ b/include/linux/udp.h
>> @@ -79,6 +79,14 @@ struct udp_sock {
>> int (*gro_complete)(struct sock *sk,
>> struct sk_buff *skb,
>> int nhoff);
>> + /* Flow dissector function for a UDP socket */
>> + enum flow_dissect_ret (*flow_dissect)(struct sock *sk,
>> + const struct sk_buff *skb,
>> + struct flow_dissector_key_control *key_control,
>> + struct flow_dissector *flow_dissector,
>> + void *target_container, void *data,
>> + __be16 *p_proto, u8 *p_ip_proto, int *p_nhoff,
>> + int *p_hlen, unsigned int flags);
>>
>> /* udp_recvmsg try to use this before splicing sk_receive_queue */
>> struct sk_buff_head reader_queue ____cacheline_aligned_in_smp;
>> diff --git a/include/net/udp.h b/include/net/udp.h
>> index f3d1de6f0983..499e4faf8b14 100644
>> --- a/include/net/udp.h
>> +++ b/include/net/udp.h
>> @@ -174,6 +174,14 @@ struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb,
>> struct udphdr *uh, udp_lookup_t lookup);
>> int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup);
>>
>> +enum flow_dissect_ret udp_flow_dissect(const struct sk_buff *skb,
>> + udp_lookup_t lookup,
>> + struct flow_dissector_key_control *key_control,
>> + struct flow_dissector *flow_dissector,
>> + void *target_container, void *data,
>> + __be16 *p_proto, u8 *p_ip_proto, int *p_nhoff,
>> + int *p_hlen, unsigned int flags);
>> +
>> static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
>> {
>> struct udphdr *uh;
>> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
>> index 10cce0dd4450..b7102e0f41a9 100644
>> --- a/include/net/udp_tunnel.h
>> +++ b/include/net/udp_tunnel.h
>> @@ -69,6 +69,13 @@ typedef struct sk_buff **(*udp_tunnel_gro_receive_t)(struct sock *sk,
>> struct sk_buff *skb);
>> typedef int (*udp_tunnel_gro_complete_t)(struct sock *sk, struct sk_buff *skb,
>> int nhoff);
>> +typedef enum flow_dissect_ret (*udp_tunnel_flow_dissect_t)(struct sock *sk,
>> + const struct sk_buff *skb,
>> + struct flow_dissector_key_control *key_control,
>> + struct flow_dissector *flow_dissector,
>> + void *target_container, void *data,
>> + __be16 *p_proto, u8 *p_ip_proto, int *p_nhoff,
>> + int *p_hlen, unsigned int flags);
>>
>> struct udp_tunnel_sock_cfg {
>> void *sk_user_data; /* user data used by encap_rcv call back */
>> @@ -78,6 +85,7 @@ struct udp_tunnel_sock_cfg {
>> udp_tunnel_encap_destroy_t encap_destroy;
>> udp_tunnel_gro_receive_t gro_receive;
>> udp_tunnel_gro_complete_t gro_complete;
>> + udp_tunnel_flow_dissect_t flow_dissect;
>> };
>>
>> /* Setup the given (UDP) sock to receive UDP encapsulated packets */
>> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
>> index 97658bfc1b58..7f0a7ed4a6f7 100644
>> --- a/net/ipv4/udp_offload.c
>> +++ b/net/ipv4/udp_offload.c
>> @@ -328,11 +328,56 @@ static int udp4_gro_complete(struct sk_buff *skb, int nhoff)
>> return udp_gro_complete(skb, nhoff, udp4_lib_lookup_skb);
>> }
>>
>> +enum flow_dissect_ret udp_flow_dissect(const struct sk_buff *skb,
>> + udp_lookup_t lookup,
>> + struct flow_dissector_key_control *key_control,
>> + struct flow_dissector *flow_dissector,
>> + void *target_container, void *data,
>> + __be16 *p_proto, u8 *p_ip_proto, int *p_nhoff,
>> + int *p_hlen, unsigned int flags)
>> +{
>> + enum flow_dissect_ret ret = FLOW_DISSECT_RET_CONTINUE;
>> + struct udphdr *uh, _uh;
>> + struct sock *sk;
>> +
>> + uh = __skb_header_pointer(skb, *p_nhoff, sizeof(_uh), data,
>> + *p_hlen, &_uh);
>> + if (!uh)
>> + return FLOW_DISSECT_RET_OUT_BAD;
>> +
>> + rcu_read_lock();
>> +
>> + sk = (*lookup)(skb, uh->source, uh->dest);
>> +
>> + if (sk && udp_sk(sk)->flow_dissect)
>> + ret = udp_sk(sk)->flow_dissect(sk, skb, key_control,
>> + flow_dissector, target_container,
>> + data, p_proto, p_ip_proto,
>> + p_nhoff, p_hlen, flags);
>> + rcu_read_unlock();
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(udp_flow_dissect);
>
> If I read the above correctly, this is going to add another full UDP
> lookup per UDP packet, can we avoid it with some static key enabled by
> vxlan/fou/etc. ?
That would also limit the exposure if a bug is discovered. The
vulnerability of this critical path also came up with the recent
flow dissector expansion for ipv6 neighbor discovery.
http://patchwork.ozlabs.org/patch/722957/
Ideally, flow dissector support for less common protocols can be
disabled administratively.
That said, the parser code looks great to me.
^ permalink raw reply
* Re: DSA mv88e6xxx RX frame errors and TCP/IP RX failure
From: Tim Harvey @ 2017-08-31 15:37 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, Vivien Didelot, linux-kernel@vger.kernel.org, Fugang Duan,
Ilia Mirkin
In-Reply-To: <20170831014624.GA18554@lunn.ch>
On Wed, Aug 30, 2017 at 6:46 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > /* Report late collisions as a frame error. */
>> > if (status & (BD_ENET_RX_NO | BD_ENET_RX_CL))
>> > ndev->stats.rx_frame_errors++;
>> >
>> > I don't see anywhere else frame errors are counted, but it would be
>> > good to prove we are looking in the right place.
>> >
>>
>> Andrew,
>>
>> (adding IMX FEC driver maintainer to CC)
>>
>> Yes, that's one of them being hit. It looks like ifconfig reports
>> 'frame' as the accumulation of a few stats so here are some more
>> specifics from /sys/class/net/eth0/statistics:
>>
>> root@xenial:/sys/devices/soc0/soc/2100000.aips-bus/2188000.ethernet/net/eth0/statistics#
>> for i in `ls rx_*`; do echo $i:$(cat $i); done
>> rx_bytes:103229
>> rx_compressed:0
>> rx_crc_errors:22
>> rx_dropped:0
>> rx_errors:22
>> rx_fifo_errors:0
>> rx_frame_errors:22
>> rx_length_errors:22
>> rx_missed_errors:0
>> rx_nohandler:0
>> rx_over_errors:0
>> rx_packets:1174
>> root@xenial:/sys/devices/soc0/soc/2100000.aips-bus/2188000.ethernet/net/eth0/statistics#
>> ifconfig eth0
>> eth0 Link encap:Ethernet HWaddr 00:D0:12:41:F3:E7
>> inet6 addr: fe80::2d0:12ff:fe41:f3e7/64 Scope:Link
>> UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1
>> RX packets:1207 errors:22 dropped:0 overruns:0 frame:66
>> TX packets:42 errors:0 dropped:0 overruns:0 carrier:0
>> collisions:0 txqueuelen:1000
>> RX bytes:106009 (103.5 KiB) TX bytes:4604 (4.4 KiB)
>>
>> Instrumenting fec driver I see the following getting hit:
>>
>> status & BD_ENET_RX_LG /* rx_length_errors: Frame too long */
>> status & BD_ENET_RX_CR /* rx_crc_errors: CRC Error */
>> status & BD_ENET_RX_CL /* rx_frame_errors: Collision? */
>>
>> Is this a frame size issue where the MV88E6176 is sending frames down
>> that exceed the MTU because of headers added?
>
> I did fix an issue recently with that. See
>
> commit fbbeefdd21049fcf9437c809da3828b210577f36
> Author: Andrew Lunn <andrew@lunn.ch>
> Date: Sun Jul 30 19:36:05 2017 +0200
>
> net: fec: Allow reception of frames bigger than 1522 bytes
>
> The FEC Receive Control Register has a 14 bit field indicating the
> longest frame that may be received. It is being set to 1522. Frames
> longer than this are discarded, but counted as being in error.
>
> When using DSA, frames from the switch has an additional header,
> either 4 or 8 bytes if a Marvell switch is used. Thus a full MTU frame
> of 1522 bytes received by the switch on a port becomes 1530 bytes when
> passed to the host via the FEC interface.
>
> Change the maximum receive size to 2048 - 64, where 64 is the maximum
> rx_alignment applied on the receive buffer for AVB capable FEC
> cores. Use this value also for the maximum receive buffer size. The
> driver is already allocating a receive SKB of 2048 bytes, so this
> change should not have any significant effects.
>
> Tested on imx51, imx6, vf610.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
>
> However, this is was of an all/nothing problem. All frames with the
> full MTU were getting dropped, where as i think you are only seeing a
> few dropped?
>
Andrew,
Indeed this does resolve the issue. I see a burst of FIFO overruns
initially when receiving an iperf bandwidth test but that would be
caused by the IMX6 errata and should be mitigated via pause frames.
After that short burst I see no other errors and iperf works fine.
Should we get this patch in the linux-stable tree for the 4.9 kernel?
Thanks!
Tim
^ permalink raw reply
* Re: [PATCH net-next] bridge: add tracepoint in br_fdb_update
From: David Ahern @ 2017-08-31 15:30 UTC (permalink / raw)
To: Roopa Prabhu, Jesper Dangaard Brouer
Cc: davem@davemloft.net, netdev@vger.kernel.org, Nikolay Aleksandrov,
Florian Fainelli, Andrew Lunn, bridge
In-Reply-To: <CAJieiUjZ5dTNpBFb8c2KMRgjXRCKVGhJsTfQcpiw+p6fyNVwDw@mail.gmail.com>
On 8/31/17 9:21 AM, Roopa Prabhu wrote:
> On Thu, Aug 31, 2017 at 5:38 AM, Jesper Dangaard Brouer
> <brouer@redhat.com> wrote:
>> On Wed, 30 Aug 2017 22:18:13 -0700
>> Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>> This extends bridge fdb table tracepoints to also cover
>>> learned fdb entries in the br_fdb_update path. Note that
>>> unlike other tracepoints I have moved this to when the fdb
>>> is modified because this is in the datapath and can generate
>>> a lot of noise in the trace output. br_fdb_update is also called
>>> from added_by_user context in the NTF_USE case which is already
>>> traced ..hence the !added_by_user check.
>>>
>>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>> ---
>>> include/trace/events/bridge.h | 31 +++++++++++++++++++++++++++++++
>>> net/bridge/br_fdb.c | 5 ++++-
>>> net/core/net-traces.c | 1 +
>>> 3 files changed, 36 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/trace/events/bridge.h b/include/trace/events/bridge.h
>>> index 0f1cde0..1bee3e7 100644
>>> --- a/include/trace/events/bridge.h
>>> +++ b/include/trace/events/bridge.h
>>> @@ -92,6 +92,37 @@ TRACE_EVENT(fdb_delete,
>>> __entry->addr[4], __entry->addr[5], __entry->vid)
>>> );
>>>
>>> +TRACE_EVENT(br_fdb_update,
>>> +
>>> + TP_PROTO(struct net_bridge *br, struct net_bridge_port *source,
>>> + const unsigned char *addr, u16 vid, bool added_by_user),
>>> +
>>> + TP_ARGS(br, source, addr, vid, added_by_user),
>>> +
>>> + TP_STRUCT__entry(
>>> + __string(br_dev, br->dev->name)
>>> + __string(dev, source->dev->name)
>>
>> I have found that using the device string name is
>>
>> (1) slow as it involves strcpy+strlen
>>
>> See [1]+[2] where a single dev-name costed me 16 ns, and the base
>> overhead of a bpf attached tracepoint is 25 ns (see [3]).
>>
>> [1] https://git.kernel.org/davem/net-next/c/e7d12ce121a
>> [2] https://git.kernel.org/davem/net-next/c/315ec3990ef
>> [3] https://git.kernel.org/davem/net-next/c/25d4dae1a64
>>
>> (2) strings are also harder to work-with/extract when attaching a bpf_prog
>>
>> See the trouble I'm in accessing a dev string here napi:napi_poll here:
>> https://github.com/netoptimizer/prototype-kernel/blob/103b955a080/kernel/samples/bpf/napi_monitor_kern.c#L52-L58
>>
>> Using ifindex'es in userspace is fairly easy see man if_indextoname(3).
>>
>
> Jesper thanks for the data!. GTK. Looking at include/trace/events,
> currently almost all tracepoints use dev->name.
> These bridge tracepoints in context are primarily for debugging fdb
> updates only, not for every packet and hence not in the performance
> path.
> In large scale deployments with thousands of bridge ports and fdb
> entries, dev->name will definately make it easier to trouble-shoot.
> So, I did like to leave these with dev->name unless there are strong objections.
+1 for user friendliness for debugging tracepoints. The device name is
also more user friendly when adding filters to the data collection.
Being able to add bpf everywhere certainly changes the game a bit, but
we should not relinquish ease of use and understanding for the potential
that someone might want to put a bpf program on the tracepoint and want
to maintain high performance.
^ permalink raw reply
* Re: [PATCH] rtlwifi: refactor code in halbtcoutsrc module
From: Larry Finger @ 2017-08-31 15:25 UTC (permalink / raw)
To: Gustavo A. R. Silva, Chaoming Li, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20170830164602.GA16413@embeddedgus>
On 08/30/2017 11:46 AM, Gustavo A. R. Silva wrote:
> Function halbtc_get_wifi_rssi always returns rtlpriv->dm.undec_sm_pwdb.
> So this function can be removed and the value of
> rtlpriv->dm.undec_sm_pwdb assigned to *s32_tmp directly.
>
> This issue was first reported by Coverity as "identical code for different
> branches" in function halbtc_get_wifi_rssi.
>
> Addresses-Coverity-ID: 1226793
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> This code was reported by Coverity and it was tested by compilation only.
> Chances are this may be a copy/paste error in function
> halbtc_get_wifi_rssi. Please, verify.
> Also, notice this code has been there since 2014.
The value of *s32_tmp is not dependent on the link state, thus this patch is
correct, but I request that it be changed. Future developments will modify
halbtc_get_wifi_rssi() making it more complicated and not as easily inlined as
this patch. In short, if you remove it here, we will have to add it later.
Thanks,
Larrt
>
> .../net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> index c1eacd8..2a47b97 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> @@ -373,17 +373,6 @@ u32 halbtc_get_wifi_link_status(struct btc_coexist *btcoexist)
> return ret_val;
> }
>
> -static s32 halbtc_get_wifi_rssi(struct rtl_priv *rtlpriv)
> -{
> - int undec_sm_pwdb = 0;
> -
> - if (rtlpriv->mac80211.link_state >= MAC80211_LINKED)
> - undec_sm_pwdb = rtlpriv->dm.undec_sm_pwdb;
> - else /* associated entry pwdb */
> - undec_sm_pwdb = rtlpriv->dm.undec_sm_pwdb;
> - return undec_sm_pwdb;
> -}
> -
> static bool halbtc_get(void *void_btcoexist, u8 get_type, void *out_buf)
> {
> struct btc_coexist *btcoexist = (struct btc_coexist *)void_btcoexist;
> @@ -479,7 +468,7 @@ static bool halbtc_get(void *void_btcoexist, u8 get_type, void *out_buf)
> *bool_tmp = false;
> break;
> case BTC_GET_S4_WIFI_RSSI:
> - *s32_tmp = halbtc_get_wifi_rssi(rtlpriv);
> + *s32_tmp = rtlpriv->dm.undec_sm_pwdb;
> break;
> case BTC_GET_S4_HS_RSSI:
> *s32_tmp = 0;
>
^ permalink raw reply
* Re: [PATCH net-next] net/ncsi: Define {add, kill}_vid callbacks for !CONFIG_NET_NCSI
From: Vernon Mauery @ 2017-08-31 15:24 UTC (permalink / raw)
To: Samuel Mendoza-Jonas
Cc: David S . Miller, netdev, linux-kernel, OpenBMC Maillist,
Gavin Shan
In-Reply-To: <20170831033846.23538-1-sam@mendozajonas.com>
On 31-Aug-2017 01:38 PM, Samuel Mendoza-Jonas wrote:
> Patch "net/ncsi: Configure VLAN tag filter" defined two new callback
> functions in include/net/ncsi.h, but neglected the !CONFIG_NET_NCSI
> case. This can cause a build error if these are referenced elsewhere
> without NCSI enabled, for example in ftgmac100:
>
> >>> ERROR: "ncsi_vlan_rx_kill_vid" [drivers/net/ethernet/faraday/ftgmac100.ko] undefined!
> >>> ERROR: "ncsi_vlan_rx_add_vid" [drivers/net/ethernet/faraday/ftgmac100.ko] undefined!
>
> Add definitions for !CONFIG_NET_NCSI to bring it into line with the rest
> of ncsi.h
>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> ---
> include/net/ncsi.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/net/ncsi.h b/include/net/ncsi.h
> index 1f96af46df49..2b13b6b91a4d 100644
> --- a/include/net/ncsi.h
> +++ b/include/net/ncsi.h
> @@ -36,6 +36,14 @@ int ncsi_start_dev(struct ncsi_dev *nd);
> void ncsi_stop_dev(struct ncsi_dev *nd);
> void ncsi_unregister_dev(struct ncsi_dev *nd);
> #else /* !CONFIG_NET_NCSI */
> +int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
> +{
> + return -ENOTTY;
> +}
> +int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
> +{
> + return -ENOTTY;
> +}
These should be static functions because they are defined in the header
file or you will get multiple symbol definitions.
--Vernon
> static inline struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
> void (*notifier)(struct ncsi_dev *nd))
> {
> --
> 2.14.1
>
^ permalink raw reply
* Re: [PATCH net-next] bridge: add tracepoint in br_fdb_update
From: Roopa Prabhu @ 2017-08-31 15:21 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: davem@davemloft.net, netdev@vger.kernel.org, Nikolay Aleksandrov,
Florian Fainelli, Andrew Lunn, bridge
In-Reply-To: <20170831143814.68709334@redhat.com>
On Thu, Aug 31, 2017 at 5:38 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Wed, 30 Aug 2017 22:18:13 -0700
> Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This extends bridge fdb table tracepoints to also cover
>> learned fdb entries in the br_fdb_update path. Note that
>> unlike other tracepoints I have moved this to when the fdb
>> is modified because this is in the datapath and can generate
>> a lot of noise in the trace output. br_fdb_update is also called
>> from added_by_user context in the NTF_USE case which is already
>> traced ..hence the !added_by_user check.
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>> include/trace/events/bridge.h | 31 +++++++++++++++++++++++++++++++
>> net/bridge/br_fdb.c | 5 ++++-
>> net/core/net-traces.c | 1 +
>> 3 files changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/trace/events/bridge.h b/include/trace/events/bridge.h
>> index 0f1cde0..1bee3e7 100644
>> --- a/include/trace/events/bridge.h
>> +++ b/include/trace/events/bridge.h
>> @@ -92,6 +92,37 @@ TRACE_EVENT(fdb_delete,
>> __entry->addr[4], __entry->addr[5], __entry->vid)
>> );
>>
>> +TRACE_EVENT(br_fdb_update,
>> +
>> + TP_PROTO(struct net_bridge *br, struct net_bridge_port *source,
>> + const unsigned char *addr, u16 vid, bool added_by_user),
>> +
>> + TP_ARGS(br, source, addr, vid, added_by_user),
>> +
>> + TP_STRUCT__entry(
>> + __string(br_dev, br->dev->name)
>> + __string(dev, source->dev->name)
>
> I have found that using the device string name is
>
> (1) slow as it involves strcpy+strlen
>
> See [1]+[2] where a single dev-name costed me 16 ns, and the base
> overhead of a bpf attached tracepoint is 25 ns (see [3]).
>
> [1] https://git.kernel.org/davem/net-next/c/e7d12ce121a
> [2] https://git.kernel.org/davem/net-next/c/315ec3990ef
> [3] https://git.kernel.org/davem/net-next/c/25d4dae1a64
>
> (2) strings are also harder to work-with/extract when attaching a bpf_prog
>
> See the trouble I'm in accessing a dev string here napi:napi_poll here:
> https://github.com/netoptimizer/prototype-kernel/blob/103b955a080/kernel/samples/bpf/napi_monitor_kern.c#L52-L58
>
> Using ifindex'es in userspace is fairly easy see man if_indextoname(3).
>
Jesper thanks for the data!. GTK. Looking at include/trace/events,
currently almost all tracepoints use dev->name.
These bridge tracepoints in context are primarily for debugging fdb
updates only, not for every packet and hence not in the performance
path.
In large scale deployments with thousands of bridge ports and fdb
entries, dev->name will definately make it easier to trouble-shoot.
So, I did like to leave these with dev->name unless there are strong objections.
thanks.
^ permalink raw reply
* [PATCH] wl1251: add a missing spin_lock_init()
From: Pavel Machek @ 2017-08-31 14:47 UTC (permalink / raw)
To: Kalle Valo, Linus Torvalds
Cc: David Miller, xiyou.wangcong, torvalds, akpm, netdev,
linux-kernel, linux-wireless
In-Reply-To: <87y3q0q7ln.fsf@kamboji.qca.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 3388 bytes --]
From: Cong Wang <xiyou.wangcong@gmail.com>
wl1251: add a missing spin_lock_init()
This fixes the following kernel warning:
[ 5668.771453] BUG: spinlock bad magic on CPU#0, kworker/u2:3/9745
[ 5668.771850] lock: 0xce63ef20, .magic: 00000000, .owner: <none>/-1,
.owner_cpu: 0
[ 5668.772277] CPU: 0 PID: 9745 Comm: kworker/u2:3 Tainted: G W
4.12.0-03002-gec979a4-dirty #40
[ 5668.772796] Hardware name: Nokia RX-51 board
[ 5668.773071] Workqueue: phy1 wl1251_irq_work
[ 5668.773345] [<c010c9e4>] (unwind_backtrace) from [<c010a274>]
(show_stack+0x10/0x14)
[ 5668.773803] [<c010a274>] (show_stack) from [<c01545a4>]
(do_raw_spin_lock+0x6c/0xa0)
[ 5668.774230] [<c01545a4>] (do_raw_spin_lock) from [<c06ca578>]
(_raw_spin_lock_irqsave+0x10/0x18)
[ 5668.774658] [<c06ca578>] (_raw_spin_lock_irqsave) from [<c048c010>]
(wl1251_op_tx+0x38/0x5c)
[ 5668.775115] [<c048c010>] (wl1251_op_tx) from [<c06a12e8>]
(ieee80211_tx_frags+0x188/0x1c0)
[ 5668.775543] [<c06a12e8>] (ieee80211_tx_frags) from [<c06a138c>]
(__ieee80211_tx+0x6c/0x130)
[ 5668.775970] [<c06a138c>] (__ieee80211_tx) from [<c06a3dbc>]
(ieee80211_tx+0xdc/0x104)
[ 5668.776367] [<c06a3dbc>] (ieee80211_tx) from [<c06a4af0>]
(__ieee80211_subif_start_xmit+0x454/0x8c8)
[ 5668.776824] [<c06a4af0>] (__ieee80211_subif_start_xmit) from
[<c06a4f94>] (ieee80211_subif_start_xmit+0x30/0x2fc)
[ 5668.777343] [<c06a4f94>] (ieee80211_subif_start_xmit) from
[<c0578848>] (dev_hard_start_xmit+0x80/0x118)
...
by adding the missing spin_lock_init().
Reported-by: Pavel Machek <pavel@ucw.cz>
Cc: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
Cc: stable@kernel.org
---
> >> Yeah, you are right there. I did actually ponder which I tree should
> >> commit it back in July but due to various reasons decided differently.
> >
> > Can we still get the fix to v4.13-final? :-).
>
> I'm not planning to submit pull requests to 4.13 anymore. If you think
> this is so important that it needs to be applied in the last minute (I
> don't) you could always try to convince Dave to take it directly.
>
> Or better yet, push it to the stable tree. If the merge window opens on
> Sunday I suspect that the commit will be in Linus' tree sometime next
> week. Then you can submit the request to the stable team to take it.
I don't think we should use stable tree as an excuse for not fixing
the bugs in mainline. Original patch is from Jul 6, thats 7 weeks ago.
Dave, Linus -- can you still take the patch?
Thanks,
Pavel
diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
index 08f0477..9915d83 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -1571,6 +1571,7 @@ struct ieee80211_hw *wl1251_alloc_hw(void)
wl->state = WL1251_STATE_OFF;
mutex_init(&wl->mutex);
+ spin_lock_init(&wl->wl_lock);
wl->tx_mgmt_frm_rate = DEFAULT_HW_GEN_TX_RATE;
wl->tx_mgmt_frm_mod = DEFAULT_HW_GEN_MODULATION_TYPE;
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply related
* Re: [PATCH net-next] x86: bpf_jit: small optimization in emit_bpf_tail_call()
From: Alexei Starovoitov @ 2017-08-31 14:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <1504180422.15310.12.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Aug 31, 2017 at 04:53:42AM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Saves 4 bytes replacing following instructions :
>
> lea rax, [rsi + rdx * 8 + offsetof(...)]
> mov rax, qword ptr [rax]
> cmp rax, 0
>
> by :
>
> mov rax, [rsi + rdx * 8 + offsetof(...)]
> test rax, rax
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Nicely spotted. Much appreciate it!
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: invoke zerocopy callback on xmit path if no tx napi
From: Willem de Bruijn @ 2017-08-31 14:30 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Koichiro Den, Jason Wang, virtualization, Network Development
In-Reply-To: <CAF=yD-+Wk9sc9dXMUq1+x_hh=3ThTXa6BnZkygP3tgVpjbp93g@mail.gmail.com>
On Tue, Aug 29, 2017 at 3:35 PM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
> On Fri, Aug 25, 2017 at 9:03 PM, Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
>> On Fri, Aug 25, 2017 at 7:32 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> On Fri, Aug 25, 2017 at 06:44:36PM -0400, Willem de Bruijn wrote:
>>>> >> >> > We don't enable network watchdog on virtio but we could and maybe
>>>> >> >> > should.
>>>> >> >>
>>>> >> >> Can you elaborate?
>>>> >> >
>>>> >> > The issue is that holding onto buffers for very long times makes guests
>>>> >> > think they are stuck. This is funamentally because from guest point of
>>>> >> > view this is a NIC, so it is supposed to transmit things out in
>>>> >> > a timely manner. If host backs the virtual NIC by something that is not
>>>> >> > a NIC, with traffic shaping etc introducing unbounded latencies,
>>>> >> > guest will be confused.
>>>> >>
>>>> >> That assumes that guests are fragile in this regard. A linux guest
>>>> >> does not make such assumptions.
>>>> >
>>>> > Yes it does. Examples above:
>>>> > > > - a single slow flow can occupy the whole ring, you will not
>>>> > > > be able to make any new buffers available for the fast flow
>>>>
>>>> Oh, right. Though those are due to vring_desc pool exhaustion
>>>> rather than an upper bound on latency of any single packet.
>>>>
>>>> Limiting the number of zerocopy packets in flight to some fraction
>>>> of the ring ensures that fast flows can always grab a slot.
>>>> Running
>>>> out of ubuf_info slots reverts to copy, so indirectly does this. But
>>>> I read it correclty the zerocopy pool may be equal to or larger than
>>>> the descriptor pool. Should we refine the zcopy_used test
>>>>
>>>> (nvq->upend_idx + 1) % UIO_MAXIOV != nvq->done_idx
>>>>
>>>> to also return false if the number of outstanding ubuf_info is greater
>>>> than, say, vq->num >> 1?
>>>
>>>
>>> We'll need to think about where to put the threshold, but I think it's
>>> a good idea.
>>>
>>> Maybe even a fixed number, e.g. max(vq->num >> 1, X) to limit host
>>> resources.
>>>
>>> In a sense it still means once you run out of slots zcopt gets disabled possibly permanently.
>>>
>>> Need to experiment with some numbers.
>>
>> I can take a stab with two flows, one delayed in a deep host qdisc
>> queue. See how this change affects the other flow and also how
>> sensitive that is to the chosen threshold value.
>
> Incomplete results at this stage, but I do see this correlation between
> flows. It occurs even while not running out of zerocopy descriptors,
> which I cannot yet explain.
>
> Running two threads in a guest, each with a udp socket, each
> sending up to 100 datagrams, or until EAGAIN, every msec.
>
> Sender A sends 1B datagrams.
> Sender B sends VHOST_GOODCOPY_LEN, which is enough
> to trigger zcopy_used in vhost net.
>
> A local receive process on the host receives both flows. To avoid
> a deep copy when looping the packet onto the receive path,
> changed skb_orphan_frags_rx to always return false (gross hack).
>
> The flow with the larger packets is redirected through netem on ifb0:
>
> modprobe ifb
> ip link set dev ifb0 up
> tc qdisc add dev ifb0 root netem limit $LIMIT rate 1MBit
>
> tc qdisc add dev tap0 ingress
> tc filter add dev tap0 parent ffff: protocol ip \
> u32 match ip dport 8000 0xffff \
> action mirred egress redirect dev ifb0
>
> For 10 second run, packet count with various ifb0 queue lengths $LIMIT:
>
> no filter
> rx.A: ~840,000
> rx.B: ~840,000
>
> limit 1
> rx.A: ~500,000
> rx.B: ~3100
> ifb0: 3273 sent, 371141 dropped
>
> limit 100
> rx.A: ~9000
> rx.B: ~4200
> ifb0: 4630 sent, 1491 dropped
>
> limit 1000
> rx.A: ~6800
> rx.B: ~4200
> ifb0: 4651 sent, 0 dropped
>
> Sender B is always correctly rate limited to 1 MBps or less. With a
> short queue, it ends up dropping a lot and sending even less.
>
> When a queue builds up for sender B, sender A throughput is strongly
> correlated with queue length. With queue length 1, it can send almost
> at unthrottled speed. But even at limit 100 its throughput is on the
> same order as sender B.
>
> What is surprising to me is that this happens even though the number
> of ubuf_info in use at limit 100 is around 100 at all times. In other words,
> it does not exhaust the pool.
>
> When forcing zcopy_used to be false for all packets, this effect of
> sender A throughput being correlated with sender B does not happen.
>
> no filter
> rx.A: ~850,000
> rx.B: ~850,000
>
> limit 100
> rx.A: ~850,000
> rx.B: ~4200
> ifb0: 4518 sent, 876182 dropped
>
> Also relevant is that with zerocopy, the sender processes back off
> and report the same count as the receiver. Without zerocopy,
> both senders send at full speed, even if only 4200 packets from flow
> B arrive at the receiver.
>
> This is with the default virtio_net driver, so without napi-tx.
>
> It appears that the zerocopy notifications are pausing the guest.
> Will look at that now.
It was indeed as simple as that. With 256 descriptors, queuing even
a hundred or so packets causes the guest to stall the device as soon
as the qdisc is installed.
Adding this check
+ in_use = nvq->upend_idx - nvq->done_idx;
+ if (nvq->upend_idx < nvq->done_idx)
+ in_use += UIO_MAXIOV;
+
+ if (in_use > (vq->num >> 2))
+ zcopy_used = false;
Has the desired behavior of reverting zerocopy requests to copying.
Without this change, the result is, as previously reported, throughput
dropping to hundreds of packets per second on both flows.
With the change, pps as observed for a few seconds at handle_tx is
zerocopy=165 copy=168435
zerocopy=0 copy=168500
zerocopy=65 copy=168535
Both flows continue to send at more or less normal rate, with only
sender B observing massive drops at the netem.
With the queue removed the rate reverts to
zerocopy=58878 copy=110239
zerocopy=58833 copy=110207
This is not a 50/50 split, which implies that some packets from the large
packet flow are still converted to copying. Without the change the rate
without queue was 80k zerocopy vs 80k copy, so this choice of
(vq->num >> 2) appears too conservative.
However, testing with (vq->num >> 1) was not as effective at mitigating
stalls. I did not save that data, unfortunately. Can run more tests on fine
tuning this variable, if the idea sounds good.
I also compiled qemu with 1024 descriptors in the tx ring instead of 256.
In that case the test
if (unlikely(vhost_exceeds_maxpend(net)))
break;
which is
return (nvq->upend_idx + vq->num - VHOST_MAX_PEND) % UIO_MAXIOV
== nvq->done_idx;
Is hit before my proposed change, again stalling the device instead of
reverting to copying.
^ permalink raw reply
* Re: [PATCH v2 net-next 1/8] bpf: Add support for recursively running cgroup sock filters
From: Tejun Heo @ 2017-08-31 14:22 UTC (permalink / raw)
To: David Ahern; +Cc: Alexei Starovoitov, netdev, daniel, ast, davem
In-Reply-To: <bb4398e5-55a3-d63e-ac4d-3f9d7e2efc02@gmail.com>
Hello, David, Alexei.
Sorry about late reply.
On Sun, Aug 27, 2017 at 08:49:23AM -0600, David Ahern wrote:
> On 8/25/17 8:49 PM, Alexei Starovoitov wrote:
> >
> >> + if (prog && curr_recursive && !new_recursive)
> >> + /* if a parent has recursive prog attached, only
> >> + * allow recursive programs in descendent cgroup
> >> + */
> >> + return -EINVAL;
> >> +
> >> old_prog = cgrp->bpf.prog[type];
> >
> > ... I'm struggling to completely understand how it interacts
> > with BPF_F_ALLOW_OVERRIDE.
>
> The 2 flags are completely independent. The existing override logic is
> unchanged. If a program can not be overridden, then the new recursive
> flag is irrelevant.
I'm not sure all four combo of the two flags makes sense. Can't we
have something simpler like the following?
1. None: No further bpf programs allowed in the subtree.
2. Overridable: If a sub-cgroup installs the same bpf program, this
one yields to that one.
3. Recursive: If a sub-cgroup installs the same bpf program, that
cgroup program gets run in addition to this one.
Note that we can have combinations of overridables and recursives -
both allow further programs in the sub-hierarchy and the only
distinction is whether that specific program behaves when that
happens.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
From: Marc Gonzalez @ 2017-08-31 14:21 UTC (permalink / raw)
To: Florian Fainelli, Mans Rullgard
Cc: David Daney, netdev, Geert Uytterhoeven, David Miller,
Andrew Lunn, Mason
In-Reply-To: <f4bb5ac8-dae8-c0af-7aa6-e546fc0783fa@sigmadesigns.com>
On 31/08/2017 14:29, Marc Gonzalez wrote:
> On 31/08/2017 02:49, Florian Fainelli wrote:
>
>> The original motivation for this change originated from Marc Gonzalez
>> indicating that his network driver did not have its adjust_link callback
>> executing with phydev->link = 0 while he was expecting it.
>
> I expect the core to call phy_adjust_link() for link changes.
> This used to work back in 3.4 and was broken somewhere along
> the way.
>
>> PHYLIB has never made any such guarantees ever because phy_stop() merely
>> just tells the workqueue to move into PHY_HALTED state which will happen
>> asynchronously.
>
> My original proposal was to fix the issue in the driver.
> I'll try locating it in my archives.
The original proposal was:
(I.e. basically a copy of phy_state_machine()'s PHY_HALTED case)
Is this what I need to submit, now that the synchronous call to
phy_state_machine() has been removed?
diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index 607064a6d7a1..8b9a981c55c1 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -1017,6 +1017,10 @@ static int nb8800_stop(struct net_device *dev)
phy_disconnect(phydev);
+ phydev->link = 0;
+ netif_carrier_off(dev);
+ nb8800_link_reconfigure(dev);
+
free_irq(dev->irq, dev);
nb8800_dma_free(dev);
^ permalink raw reply related
* Re: [PATCH net-next] bridge: add tracepoint in br_fdb_update
From: Nikolay Aleksandrov @ 2017-08-31 14:19 UTC (permalink / raw)
To: Roopa Prabhu, davem; +Cc: netdev, f.fainelli, andrew, bridge
In-Reply-To: <1504156693-24692-1-git-send-email-roopa@cumulusnetworks.com>
On 31/08/17 08:18, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This extends bridge fdb table tracepoints to also cover
> learned fdb entries in the br_fdb_update path. Note that
> unlike other tracepoints I have moved this to when the fdb
> is modified because this is in the datapath and can generate
> a lot of noise in the trace output. br_fdb_update is also called
> from added_by_user context in the NTF_USE case which is already
> traced ..hence the !added_by_user check.
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> include/trace/events/bridge.h | 31 +++++++++++++++++++++++++++++++
> net/bridge/br_fdb.c | 5 ++++-
> net/core/net-traces.c | 1 +
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* [PATCH] ipv6: sr: Use ARRAY_SIZE macro
From: Thomas Meyer @ 2017-08-31 14:18 UTC (permalink / raw)
To: davem, netdev, linux-kernel; +Cc: Thomas Meyer
Grepping for "sizeof\(.+\) / sizeof\(" found this as one of the first
candidates.
Maybe a coccinelle can catch all of those.
Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
---
net/ipv6/seg6_hmac.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index f950cb53d5e3..33fb35cbfac1 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -12,6 +12,7 @@
*/
#include <linux/errno.h>
+#include <linux/kernel.h>
#include <linux/types.h>
#include <linux/socket.h>
#include <linux/sockios.h>
@@ -110,7 +111,7 @@ static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
struct seg6_hmac_algo *algo;
int i, alg_count;
- alg_count = sizeof(hmac_algos) / sizeof(struct seg6_hmac_algo);
+ alg_count = ARRAY_SIZE(hmac_algos);
for (i = 0; i < alg_count; i++) {
algo = &hmac_algos[i];
if (algo->alg_id == alg_id)
@@ -360,7 +361,7 @@ static int seg6_hmac_init_algo(void)
struct shash_desc *shash;
int i, alg_count, cpu;
- alg_count = sizeof(hmac_algos) / sizeof(struct seg6_hmac_algo);
+ alg_count = ARRAY_SIZE(hmac_algos);
for (i = 0; i < alg_count; i++) {
struct crypto_shash **p_tfm;
@@ -421,7 +422,7 @@ void seg6_hmac_exit(void)
struct seg6_hmac_algo *algo = NULL;
int i, alg_count, cpu;
- alg_count = sizeof(hmac_algos) / sizeof(struct seg6_hmac_algo);
+ alg_count = ARRAY_SIZE(hmac_algos);
for (i = 0; i < alg_count; i++) {
algo = &hmac_algos[i];
for_each_possible_cpu(cpu) {
--
2.11.0
^ permalink raw reply related
* [PATCH][net-next] net: qualcomm: rmnet: remove unused variable priv
From: Colin King @ 2017-08-31 14:07 UTC (permalink / raw)
To: Subash Abhinov Kasiviswanathan, David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
priv is being assigned but is never used, so remove it.
Cleans up clang build warning:
"warning: Value stored to 'priv' is never read"
Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial implementation")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index c8b573d28dcf..bf7455fdafcc 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -73,9 +73,6 @@ static const struct net_device_ops rmnet_vnd_ops = {
*/
void rmnet_vnd_setup(struct net_device *rmnet_dev)
{
- struct rmnet_priv *priv;
-
- priv = netdev_priv(rmnet_dev);
netdev_dbg(rmnet_dev, "Setting up device %s\n", rmnet_dev->name);
rmnet_dev->netdev_ops = &rmnet_vnd_ops;
--
2.14.1
^ permalink raw reply related
* Re: tip -ENOBOOT - bisected to locking/refcounts, x86/asm: Implement fast refcount overflow protection
From: Mike Galbraith @ 2017-08-31 13:58 UTC (permalink / raw)
To: Kees Cook, David S. Miller, Peter Zijlstra
Cc: LKML, Ingo Molnar, Reshetova, Elena, Network Development
In-Reply-To: <CAGXu5jKYPp5D+YxtqrJXsi48UUbjfeREngsys-dU1eP4RnnL+w@mail.gmail.com>
On Wed, 2017-08-30 at 21:10 -0700, Kees Cook wrote:
> On Wed, Aug 30, 2017 at 9:01 PM, Kees Cook <keescook@chromium.org> wrote:
> > On Wed, Aug 30, 2017 at 8:12 PM, Mike Galbraith <efault@gmx.de> wrote:
> >> On Wed, 2017-08-30 at 19:27 -0700, Kees Cook wrote:
> >>
> >>> Interesting! Can you try with 633547973ffc3 ("net: convert
> >>> sk_buff.users from atomic_t to refcount_t") reverted? I'll see if
> >>> running haveged will help me trigger this on my system...
> >>
> >> With that (plus 230cd1279d001 fix to it) reverted, vbox boots.
> >
> > Wonderful! Thank you so much for helping track this down.
> >
> > So, it seems that sk_buff.users will need some more special attention
> > before we can convert it to refcount.
> >
> > x86-refcount will saturate with refcount_dec_and_test() if the result
> > is negative. But that would mean at least starting at 0. FULL should
> > have WARNed in this case, so I remain slightly confused why it was
> > missed by FULL.
>
> Actually, if this is a race condition it's possible that FULL is slow
> enough to miss it...
>
> I bet something briefly takes the refcount negative, and with
> unchecked atomics, it come back up positive again during the race.
> FULL may miss the race, and x86-refcount will catch it and saturate...
(gdb) list *in6_dev_get+0x1e
0xffffffff8166d3de is in in6_dev_get (./arch/x86/include/asm/refcount.h:52).
47 : "cc", "cx");
48 }
49
50 static __always_inline void refcount_inc(refcount_t *r)
51 {
52 asm volatile(LOCK_PREFIX "incl %0\n\t"
53 REFCOUNT_CHECK_LT_ZERO
54 : [counter] "+m" (r->refs.counter)
55 : : "cc", "cx");
56
gdb) list *in6_dev_get+0x10
0xffffffff8166d3d0 is in in6_dev_get (./include/net/addrconf.h:318).
313 {
314 struct inet6_dev *idev;
315
316 rcu_read_lock();
317 idev = rcu_dereference(dev->ip6_ptr);
318 if (idev)
319 refcount_inc(&idev->refcnt);
320 rcu_read_unlock();
321 return idev;
322
That's from kernel with no revert, but your silent saturation patch
still applied, AND built with gcc-6.3.1. Kernel traps, but it boots
and works, as does kernel built with gcc-7.0.1. Remove your silent
saturation patch, kernel doesn't notice a thing, just works.
With gcc-4.8.5, trap means you're as good as dead, with the other two,
trap means the intended. Compiler, constraints, dark elves.. pick one.
Full first splat from bootable gcc-6.3.1 built kernel.
[ 1.293962] NET: Registered protocol family 10
[ 1.294635] refcount_t silent saturation at in6_dev_get+0x25/0x104 in swapper/0[1], uid/euid: 0/0
[ 1.295616] ------------[ cut here ]------------
[ 1.296120] WARNING: CPU: 0 PID: 1 at kernel/panic.c:612 refcount_error_report+0x94/0x9e
[ 1.296950] Modules linked in:
[ 1.297276] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.13.0.g152d54a-tip-default #53
[ 1.299179] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014
[ 1.300743] task: ffff88013ab84040 task.stack: ffffc9000062c000
[ 1.301825] RIP: 0010:refcount_error_report+0x94/0x9e
[ 1.302804] RSP: 0018:ffffc9000062fc10 EFLAGS: 00010282
[ 1.303791] RAX: 0000000000000055 RBX: ffffffff81a34274 RCX: ffffffff81c605e8
[ 1.304991] RDX: 0000000000000001 RSI: 0000000000000096 RDI: 0000000000000246
[ 1.306189] RBP: ffffc9000062fd58 R08: 0000000000000000 R09: 0000000000000175
[ 1.307392] R10: 0000000000000000 R11: 0000000000000001 R12: ffff88013ab84040
[ 1.308583] R13: 0000000000000000 R14: 0000000000000004 R15: ffffffff81a256c8
[ 1.309768] FS: 0000000000000000(0000) GS:ffff88013fc00000(0000) knlGS:0000000000000000
[ 1.311052] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1.312100] CR2: 00007f4631fe8df0 CR3: 0000000137d09003 CR4: 00000000001606f0
[ 1.313301] Call Trace:
[ 1.314012] ex_handler_refcount+0x63/0x70
[ 1.314893] fixup_exception+0x32/0x40
[ 1.315737] do_trap+0x8c/0x170
[ 1.316519] do_error_trap+0x70/0xd0
[ 1.317340] ? in6_dev_get+0x23/0x104
[ 1.318172] ? netlink_broadcast_filtered+0x2bd/0x430
[ 1.319156] ? kmem_cache_alloc_trace+0xce/0x5d0
[ 1.320098] ? set_debug_rodata+0x11/0x11
[ 1.320964] invalid_op+0x1e/0x30
[ 1.322520] RIP: 0010:in6_dev_get+0x25/0x104
[ 1.323631] RSP: 0018:ffffc9000062fe00 EFLAGS: 00010202
[ 1.324614] RAX: ffff880137de2400 RBX: ffff880137df4600 RCX: ffff880137de24f0
[ 1.325793] RDX: ffff88013a5e4000 RSI: 00000000fffffe00 RDI: ffff88013a5e4000
[ 1.326964] RBP: 00000000000000d1 R08: 0000000000000000 R09: ffff880137de7600
[ 1.328150] R10: 0000000000000000 R11: ffff8801398a4df8 R12: 0000000000000000
[ 1.329374] R13: ffffffff82137872 R14: 014200ca00000000 R15: 0000000000000000
[ 1.330547] ? set_debug_rodata+0x11/0x11
[ 1.331392] ip6_route_init_special_entries+0x2a/0x89
[ 1.332369] addrconf_init+0x9e/0x203
[ 1.333173] inet6_init+0x1af/0x365
[ 1.333956] ? af_unix_init+0x4e/0x4e
[ 1.334753] do_one_initcall+0x4e/0x190
[ 1.335555] ? set_debug_rodata+0x11/0x11
[ 1.336369] kernel_init_freeable+0x189/0x20e
[ 1.337230] ? rest_init+0xd0/0xd0
[ 1.337999] kernel_init+0xa/0xf7
[ 1.338744] ret_from_fork+0x25/0x30
[ 1.339500] Code: 48 8b 95 80 00 00 00 41 55 49 8d 8c 24 f0 0a 00 00 45 8b 84 24 10 09 00 00 41 89 c1 48 89 de 48 c7 c7 60 7a a3 81 e8 07 de 05 00 <0f> ff 58 5b 5d 41 5c 41 5d c3 0f 1f 44 00 00 55 48 89 e5 41 56
[ 1.342243] ---[ end trace b5d40c0fccce776c ]---
^ permalink raw reply
* [PATCH] net: phy: bcm7xxx: make array bcm7xxx_suspend_cfg static, reduces object code size
From: Colin King @ 2017-08-31 13:57 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Don't populate the array bcm7xxx_suspend_cfg A on the stack, instead
make it static. Makes the object code smaller by over 300 bytes:
Before:
text data bss dec hex filename
6351 8146 0 14497 38a1 drivers/net/phy/bcm7xxx.o
After:
text data bss dec hex filename
5986 8210 0 14196 3774 drivers/net/phy/bcm7xxx.o
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/phy/bcm7xxx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index caa9f6e17f34..8b33f688ac8a 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -511,7 +511,7 @@ static int bcm7xxx_config_init(struct phy_device *phydev)
static int bcm7xxx_suspend(struct phy_device *phydev)
{
int ret;
- const struct bcm7xxx_regs {
+ static const struct bcm7xxx_regs {
int reg;
u16 value;
} bcm7xxx_suspend_cfg[] = {
--
2.14.1
^ permalink raw reply related
* Re: [PATCH 24/25] net/cdc_ncm: Replace tasklet with softirq hrtimer
From: Bjørn Mork @ 2017-08-31 13:57 UTC (permalink / raw)
To: Anna-Maria Gleixner
Cc: LKML, Peter Zijlstra, Ingo Molnar, Christoph Hellwig, keescook,
John Stultz, Thomas Gleixner, Oliver Neukum, Greg Kroah-Hartman,
linux-usb, netdev
In-Reply-To: <20170831105827.479650817@linutronix.de>
Anna-Maria Gleixner <anna-maria@linutronix.de> writes:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> The bh tasklet is used in invoke the hrtimer (cdc_ncm_tx_timer_cb) in
> softirq context. This can be also achieved without the tasklet but with
> CLOCK_MONOTONIC_SOFT as hrtimer base.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> Cc: Oliver Neukum <oliver@neukum.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Cc: netdev@vger.kernel.org
> ---
> drivers/net/usb/cdc_ncm.c | 37 ++++++++++++++++---------------------
> include/linux/usb/cdc_ncm.h | 2 +-
> 2 files changed, 17 insertions(+), 22 deletions(-)
>
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -61,7 +61,6 @@ static bool prefer_mbim;
> module_param(prefer_mbim, bool, S_IRUGO | S_IWUSR);
> MODULE_PARM_DESC(prefer_mbim, "Prefer MBIM setting on dual NCM/MBIM functions");
>
> -static void cdc_ncm_txpath_bh(unsigned long param);
> static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
> static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
> static struct usb_driver cdc_ncm_driver;
> @@ -777,10 +776,9 @@ int cdc_ncm_bind_common(struct usbnet *d
> if (!ctx)
> return -ENOMEM;
>
> - hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> + hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC_SOFT, HRTIMER_MODE_REL);
> ctx->tx_timer.function = &cdc_ncm_tx_timer_cb;
> - ctx->bh.data = (unsigned long)dev;
> - ctx->bh.func = cdc_ncm_txpath_bh;
> + ctx->usbnet = dev;
> atomic_set(&ctx->stop, 0);
> spin_lock_init(&ctx->mtx);
>
> @@ -967,10 +965,7 @@ void cdc_ncm_unbind(struct usbnet *dev,
>
> atomic_set(&ctx->stop, 1);
>
> - if (hrtimer_active(&ctx->tx_timer))
> - hrtimer_cancel(&ctx->tx_timer);
> -
> - tasklet_kill(&ctx->bh);
> + hrtimer_cancel(&ctx->tx_timer);
>
> /* handle devices with combined control and data interface */
> if (ctx->control == ctx->data)
> @@ -1348,20 +1343,9 @@ static void cdc_ncm_tx_timeout_start(str
> HRTIMER_MODE_REL);
> }
>
> -static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
> +static void cdc_ncm_txpath_bh(struct cdc_ncm_ctx *ctx)
> {
> - struct cdc_ncm_ctx *ctx =
> - container_of(timer, struct cdc_ncm_ctx, tx_timer);
> -
> - if (!atomic_read(&ctx->stop))
> - tasklet_schedule(&ctx->bh);
> - return HRTIMER_NORESTART;
> -}
> -
> -static void cdc_ncm_txpath_bh(unsigned long param)
> -{
> - struct usbnet *dev = (struct usbnet *)param;
> - struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
> + struct usbnet *dev = ctx->usbnet;
>
> spin_lock_bh(&ctx->mtx);
> if (ctx->tx_timer_pending != 0) {
> @@ -1379,6 +1363,17 @@ static void cdc_ncm_txpath_bh(unsigned l
> }
> }
>
> +static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
> +{
> + struct cdc_ncm_ctx *ctx =
> + container_of(timer, struct cdc_ncm_ctx, tx_timer);
> +
> + if (!atomic_read(&ctx->stop))
> + cdc_ncm_txpath_bh(ctx);
> +
> + return HRTIMER_NORESTART;
> +}
> +
> struct sk_buff *
> cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
> {
> --- a/include/linux/usb/cdc_ncm.h
> +++ b/include/linux/usb/cdc_ncm.h
> @@ -92,7 +92,6 @@
> struct cdc_ncm_ctx {
> struct usb_cdc_ncm_ntb_parameters ncm_parm;
> struct hrtimer tx_timer;
> - struct tasklet_struct bh;
>
> const struct usb_cdc_ncm_desc *func_desc;
> const struct usb_cdc_mbim_desc *mbim_desc;
> @@ -101,6 +100,7 @@ struct cdc_ncm_ctx {
>
> struct usb_interface *control;
> struct usb_interface *data;
> + struct usbnet *usbnet;
>
> struct sk_buff *tx_curr_skb;
> struct sk_buff *tx_rem_skb;
I believe the struct usbnet pointer is redundant. We already have lots
of pointers back and forth here. This should work, but is not tested:
struct usbnet *dev = usb_get_intfdata(ctx->control):
Bjørn
^ permalink raw reply
* [PATCH net-next v6] net: stmmac: Delete dead code for MDIO registration
From: Romain Perier @ 2017-08-31 13:53 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Andrew Lunn,
Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Romain Perier
This code is no longer used, the logging function was changed by commit
fbca164776e4 ("net: stmmac: Use the right logging function in stmmac_mdio_register").
It was previously showing information about the type of the IRQ, if it's
polled, ignored or a normal interrupt. As we don't want information loss,
I have moved this code to phy_attached_print().
Fixes: fbca164776e4 ("net: stmmac: Use the right logging function in stmmac_mdio_register")
Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
Hello,
This is the continuity of "[PATCH v2 0/2] net: stmmac: phy logging fixes",
see https://lkml.org/lkml/2017/8/21/288
Changes in v6:
- Rebased onto net-next
Changes in v5:
- Don't truncater commit fbca164776e4 in the message of *this* commit
- Fixed typo
Changes in v4:
- Don't truncate the commit subject for "Fixes"
- Fixed invalid sizeof() used with snprintf
- Added "net-next" prefix to the title of the commit
Changes in v3:
- Removed patch 2/2: "net: phy: Don't use drv when it is NULL in phy_attached_print",
fixed upstream by fcd03e362b1c
("net: phy: Deal with unbound PHY driver in phy_attached_print()")
- Moved this code to phy_attached_print(), so we have more informations
about the IRQ (poll, ignore or normal irq) and no information are loss.
- Re-worded commit message
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 16 ----------------
drivers/net/phy/phy_device.c | 22 +++++++++++++++++++---
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index 72ec711fcba2..f5f37bfa1d58 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -248,9 +248,6 @@ int stmmac_mdio_register(struct net_device *ndev)
found = 0;
for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
struct phy_device *phydev = mdiobus_get_phy(new_bus, addr);
- int act = 0;
- char irq_num[4];
- char *irq_str;
if (!phydev)
continue;
@@ -273,19 +270,6 @@ int stmmac_mdio_register(struct net_device *ndev)
if (priv->plat->phy_addr == -1)
priv->plat->phy_addr = addr;
- act = (priv->plat->phy_addr == addr);
- switch (phydev->irq) {
- case PHY_POLL:
- irq_str = "POLL";
- break;
- case PHY_IGNORE_INTERRUPT:
- irq_str = "IGNORE";
- break;
- default:
- sprintf(irq_num, "%d", phydev->irq);
- irq_str = irq_num;
- break;
- }
phy_attached_info(phydev);
found = 1;
}
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9493fb369682..c2f46e5053f7 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -874,19 +874,35 @@ void phy_attached_info(struct phy_device *phydev)
}
EXPORT_SYMBOL(phy_attached_info);
-#define ATTACHED_FMT "attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)"
+#define ATTACHED_FMT "attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%s)"
void phy_attached_print(struct phy_device *phydev, const char *fmt, ...)
{
+ char *irq_str;
+ char irq_num[4];
+
+ switch(phydev->irq) {
+ case PHY_POLL:
+ irq_str = "POLL";
+ break;
+ case PHY_IGNORE_INTERRUPT:
+ irq_str = "IGNORE";
+ break;
+ default:
+ snprintf(irq_num, sizeof(irq_num), "%d", phydev->irq);
+ irq_str = irq_num;
+ break;
+ }
+
if (!fmt) {
dev_info(&phydev->mdio.dev, ATTACHED_FMT "\n",
phydev->drv->name, phydev_name(phydev),
- phydev->irq);
+ irq_str);
} else {
va_list ap;
dev_info(&phydev->mdio.dev, ATTACHED_FMT,
phydev->drv->name, phydev_name(phydev),
- phydev->irq);
+ irq_str);
va_start(ap, fmt);
vprintk(fmt, ap);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next 2/3] net: mvpp2: use the GoP interrupt for link status changes
From: Antoine Tenart @ 2017-08-31 13:52 UTC (permalink / raw)
To: Andrew Lunn
Cc: Antoine Tenart, davem, gregory.clement, thomas.petazzoni, nadavh,
linux, linux-kernel, mw, stefanc, miquel.raynal, netdev
In-Reply-To: <20170831134724.GO22289@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 729 bytes --]
Hi Andrew,
On Thu, Aug 31, 2017 at 03:47:24PM +0200, Andrew Lunn wrote:
> On Thu, Aug 31, 2017 at 09:12:55AM +0200, Antoine Tenart wrote:
> > +static void mvpp22_gop_unmask_irq(struct mvpp2_port *port)
> > +{
> > + u32 val;
> > +
> > + if (port->phy_interface == PHY_INTERFACE_MODE_RGMII ||
> > + port->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > + port->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> > + port->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID ||
>
> phy_interface_mode_is_rgmii()
Right, and you already made that comment before... I'll fix it.
Thanks!
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [pull request][net-next 0/3] Mellanox, mlx5 GRE tunnel offloads
From: Hannes Frederic Sowa @ 2017-08-31 13:51 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: David S. Miller, netdev
In-Reply-To: <20170830230409.15176-1-saeedm@mellanox.com>
Saeed Mahameed <saeedm@mellanox.com> writes:
> The first patch from Gal and Ariel provides the mlx5 driver support for
> ConnectX capability to perform IP version identification and matching in
> order to distinguish between IPv4 and IPv6 without the need to specify the
> encapsulation type, thus perform RSS in MPLS automatically without
> specifying MPLS ethertyoe. This patch will also serve for inner GRE IPv4/6
> classification for inner GRE RSS.
I don't think this is legal at all or did I misunderstood something?
<https://tools.ietf.org/html/rfc3032#section-2.2>
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: mvpp2: use the GoP interrupt for link status changes
From: Andrew Lunn @ 2017-08-31 13:47 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, gregory.clement, thomas.petazzoni, nadavh, linux,
linux-kernel, mw, stefanc, miquel.raynal, netdev
In-Reply-To: <20170831071256.18416-3-antoine.tenart@free-electrons.com>
On Thu, Aug 31, 2017 at 09:12:55AM +0200, Antoine Tenart wrote:
> +static void mvpp22_gop_unmask_irq(struct mvpp2_port *port)
> +{
> + u32 val;
> +
> + if (port->phy_interface == PHY_INTERFACE_MODE_RGMII ||
> + port->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + port->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> + port->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID ||
Hi Antoine
phy_interface_mode_is_rgmii()
Andrew
^ 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