* [PATCH net-next] bnxt_en: Remove unnecessary unsigned integer comparison and initialize variable
From: Gustavo A. R. Silva @ 2018-10-05 20:12 UTC (permalink / raw)
To: Michael Chan, David S. Miller; +Cc: netdev, linux-kernel, Gustavo A. R. Silva
There is no need to compare *val.vu32* with < 0 because
such variable is of type u32 (32 bits, unsigned), making it
impossible to hold a negative value. Fix this by removing
such comparison.
Also, initialize variable *max_val* to -1, just in case
it is not initialized to either BNXT_MSIX_VEC_MAX or
BNXT_MSIX_VEC_MIN_MAX before using it in a comparison
with val.vu32 at line 159:
if (val.vu32 > max_val)
Addresses-Coverity-ID: 1473915 ("Unsigned compared against 0")
Addresses-Coverity-ID: 1473920 ("Uninitialized scalar variable")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 8a10e01..140dbd6 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -148,7 +148,7 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
union devlink_param_value val,
struct netlink_ext_ack *extack)
{
- int max_val;
+ int max_val = -1;
if (id == DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX)
max_val = BNXT_MSIX_VEC_MAX;
@@ -156,7 +156,7 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
if (id == DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN)
max_val = BNXT_MSIX_VEC_MIN_MAX;
- if (val.vu32 < 0 || val.vu32 > max_val) {
+ if (val.vu32 > max_val) {
NL_SET_ERR_MSG_MOD(extack, "MSIX value is exceeding the range");
return -EINVAL;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 0/5] net: Consolidate metrics handling for ipv4 and ipv6
From: Eric Dumazet @ 2018-10-05 13:08 UTC (permalink / raw)
To: Eric Dumazet, David Miller, dsahern
Cc: netdev, weiwan, sd, xiyou.wangcong, dsahern
In-Reply-To: <2b4d849f-9bd3-28aa-7a1c-ad61ab584041@gmail.com>
On 10/05/2018 05:17 AM, Eric Dumazet wrote:
>
>
> On 10/04/2018 09:55 PM, David Miller wrote:
>> From: David Ahern <dsahern@kernel.org>
>> Date: Thu, 4 Oct 2018 20:07:50 -0700
>>
>>> From: David Ahern <dsahern@gmail.com>
>>>
>>> As part of the IPv6 fib info refactoring, the intent was to make metrics
>>> handling for ipv6 identical to ipv4. One oversight in ip6_dst_destroy
>>> led to confusion and a couple of incomplete attempts at finding and
>>> fixing the resulting memory leak which was ultimately resolved by
>>> ce7ea4af0838 ("ipv6: fix memory leak on dst->_metrics").
>>>
>>> Refactor metrics hanlding make the code really identical for v4 and v6,
>>> and add a few test cases.
>>
>> Looks nice, series applied, thanks David.
>>
>
> Does not look well tested and reviewed to me.
For some reason I have not received the patch series in my inbox, I only got
your "series applied" message.
Commit 767a2217533fed6 ("net: common metrics init helper for FIB entries")
is not correct because we need to better deal with error paths.
I will submit this more formally when I can reach my workstation in a few minutes :
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 6c1d817151cae45421dc976c5ea082b4115650be..74d97addf1af20dda0c2b6a2018e88696f9f7d5a 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2976,6 +2976,8 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg,
rt->fib6_metrics = ip_fib_metrics_init(net, cfg->fc_mx, cfg->fc_mx_len);
if (IS_ERR(rt->fib6_metrics)) {
err = PTR_ERR(rt->fib6_metrics);
+ /* Do not leave garbage there. */
+ rt->fib6_metrics = (struct dst_metrics *)&dst_default_metrics;
goto out;
}
^ permalink raw reply related
* Re: [PATCH net-next,v2] IPv6 ifstats separation
From: Stephen Suryaputra @ 2018-10-05 13:00 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <cdf02f85-57ea-3371-cec3-ec155964f4d0@gmail.com>
On Thu, Oct 4, 2018 at 4:42 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> How have you decided some counters can be 'slow' and other 'fast' ?
>
> I can tell you I see many ultra-fast candidates in your 'slow' list :/
Based on what others have categorized based on what's in the code and
IMHO they make sense:
enum
{
IPSTATS_MIB_NUM = 0,
/* frequently written fields in fast path, kept in same cache line */
IPSTATS_MIB_INPKTS, /* InReceives */
IPSTATS_MIB_INOCTETS, /* InOctets */
IPSTATS_MIB_INDELIVERS, /* InDelivers */
IPSTATS_MIB_OUTFORWDATAGRAMS, /* OutForwDatagrams */
IPSTATS_MIB_OUTPKTS, /* OutRequests */
IPSTATS_MIB_OUTOCTETS, /* OutOctets */
/* other fields */
IPSTATS_MIB_INHDRERRORS, /* InHdrErrors */
...
__IPSTATS_MIB_MAX
};
>
> Also think about DDOS.
>
> After your patch, all these 'wrong packets' will incur an expensive
> operation on a shared and highly contented cache line,
> effectively making the attack easier to conduct.
>
I agree about it is becoming more expensive to hit the slow counters
due to the check whether they are enabled or not.
Do you think it should just be enabled always? Then the cost to hit
the slow counters is the same as what it is right now.
Stephen.
^ permalink raw reply
* Re: [PATCH net-next 0/5] net: Consolidate metrics handling for ipv4 and ipv6
From: Eric Dumazet @ 2018-10-05 12:17 UTC (permalink / raw)
To: David Miller, dsahern; +Cc: netdev, weiwan, sd, xiyou.wangcong, dsahern
In-Reply-To: <20181004.215507.1973705600397352485.davem@davemloft.net>
On 10/04/2018 09:55 PM, David Miller wrote:
> From: David Ahern <dsahern@kernel.org>
> Date: Thu, 4 Oct 2018 20:07:50 -0700
>
>> From: David Ahern <dsahern@gmail.com>
>>
>> As part of the IPv6 fib info refactoring, the intent was to make metrics
>> handling for ipv6 identical to ipv4. One oversight in ip6_dst_destroy
>> led to confusion and a couple of incomplete attempts at finding and
>> fixing the resulting memory leak which was ultimately resolved by
>> ce7ea4af0838 ("ipv6: fix memory leak on dst->_metrics").
>>
>> Refactor metrics hanlding make the code really identical for v4 and v6,
>> and add a few test cases.
>
> Looks nice, series applied, thanks David.
>
Does not look well tested and reviewed to me.
^ permalink raw reply
* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
From: Gustavo A. R. Silva @ 2018-10-05 19:15 UTC (permalink / raw)
To: Ben Greear, Kalle Valo, David S. Miller
Cc: ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <dd75a8aa-b87c-e325-cc1d-a0efe3664a32-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org>
On 10/5/18 9:14 PM, Gustavo A. R. Silva wrote:
>
>
> On 10/5/18 9:09 PM, Ben Greear wrote:
>> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>>> Currently, the error handling for the call to function
>>> ath10k_get_legacy_rate_idx() doesn't work because
>>> *rate_idx* is of type u8 (8 bits, unsigned), which
>>> makes it impossible for it to hold a value less
>>> than 0.
>>>
>>> Fix this by changing the type of variable *rate_idx*
>>> to s8 (8 bits, signed).
>>
>> There are more than 127 rates, are you sure this is doing
>> what you want?
>>
>
> Based on the following function, rate_idx can only hold values from 0 to 11
>
... and of course -EINVAL too
> static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
> {
> static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
> 18, 24, 36, 48, 54};
> int i;
>
> for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
> if (rate == legacy_rates[i])
> return i;
> }
>
> ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
> return -EINVAL;
> }
>
> Thanks
> --
> Gustavo
>
>> Thanks,
>> Ben
>>
>>>
>>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
>>> Signed-off-by: Gustavo A. R. Silva <gustavo-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org>
>>> ---
>>> drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> index f240525..edd0e74 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>>> struct ath10k_per_peer_tx_stats *peer_stats)
>>> {
>>> struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>>> - u8 rate = 0, rate_idx = 0, sgi;
>>> + u8 rate = 0, sgi;
>>> + s8 rate_idx = 0;
>>> struct rate_info txrate;
>>>
>>> lockdep_assert_held(&ar->data_lock);
>>>
>>
>>
^ permalink raw reply
* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
From: Gustavo A. R. Silva @ 2018-10-05 19:14 UTC (permalink / raw)
To: Ben Greear, Kalle Valo, David S. Miller
Cc: ath10k, linux-wireless, netdev, linux-kernel
In-Reply-To: <3a55bec6-d20d-f500-e741-b228a86b7117@candelatech.com>
On 10/5/18 9:09 PM, Ben Greear wrote:
> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>> Currently, the error handling for the call to function
>> ath10k_get_legacy_rate_idx() doesn't work because
>> *rate_idx* is of type u8 (8 bits, unsigned), which
>> makes it impossible for it to hold a value less
>> than 0.
>>
>> Fix this by changing the type of variable *rate_idx*
>> to s8 (8 bits, signed).
>
> There are more than 127 rates, are you sure this is doing
> what you want?
>
Based on the following function, rate_idx can only hold values from 0 to 11
static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
{
static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
18, 24, 36, 48, 54};
int i;
for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
if (rate == legacy_rates[i])
return i;
}
ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
return -EINVAL;
}
Thanks
--
Gustavo
> Thanks,
> Ben
>
>>
>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>> drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> index f240525..edd0e74 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>> struct ath10k_per_peer_tx_stats *peer_stats)
>> {
>> struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>> - u8 rate = 0, rate_idx = 0, sgi;
>> + u8 rate = 0, sgi;
>> + s8 rate_idx = 0;
>> struct rate_info txrate;
>>
>> lockdep_assert_held(&ar->data_lock);
>>
>
>
^ permalink raw reply
* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
From: Ben Greear @ 2018-10-05 19:09 UTC (permalink / raw)
To: Gustavo A. R. Silva, Kalle Valo, David S. Miller
Cc: ath10k, linux-wireless, netdev, linux-kernel
In-Reply-To: <20181005184245.GA11700@embeddedor.com>
On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
> Currently, the error handling for the call to function
> ath10k_get_legacy_rate_idx() doesn't work because
> *rate_idx* is of type u8 (8 bits, unsigned), which
> makes it impossible for it to hold a value less
> than 0.
>
> Fix this by changing the type of variable *rate_idx*
> to s8 (8 bits, signed).
There are more than 127 rates, are you sure this is doing
what you want?
Thanks,
Ben
>
> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index f240525..edd0e74 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
> struct ath10k_per_peer_tx_stats *peer_stats)
> {
> struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
> - u8 rate = 0, rate_idx = 0, sgi;
> + u8 rate = 0, sgi;
> + s8 rate_idx = 0;
> struct rate_info txrate;
>
> lockdep_assert_held(&ar->data_lock);
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] bpf: fix building without CONFIG_INET
From: Daniel Borkmann @ 2018-10-05 19:07 UTC (permalink / raw)
To: Song Liu, Arnd Bergmann
Cc: Alexei Starovoitov, David S . Miller, John Fastabend,
Martin KaFai Lau, makita.toshiaki, Lawrence Brakmo,
Andrey Ignatov, Jesper Dangaard Brouer, Jakub Kicinski,
Mathieu Xhonneux, dsahern, Networking, open list
In-Reply-To: <CAPhsuW7pg9Wp5am3Kc1t3UoAG1=EYzUeq8E2k7Pmv8yGu9JaEQ@mail.gmail.com>
On 10/05/2018 09:02 PM, Song Liu wrote:
[...]
> BPF_CALL_x() has static already (before this patch). We should not
> need change that
> for all the BPF_CALL_?(). Joe's version looks better to me.
My preference as well, thanks!
^ permalink raw reply
* Re: [PATCH] bpf: fix building without CONFIG_INET
From: Song Liu @ 2018-10-05 19:02 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Alexei Starovoitov, Daniel Borkmann, David S . Miller,
John Fastabend, Martin KaFai Lau, makita.toshiaki,
Lawrence Brakmo, Andrey Ignatov, Jesper Dangaard Brouer,
Jakub Kicinski, Mathieu Xhonneux, dsahern, Networking, open list
In-Reply-To: <20181005161526.843924-1-arnd@arndb.de>
On Fri, Oct 5, 2018 at 9:18 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> The newly added TCP and UDP handling fails to link when CONFIG_INET
> is disabled:
>
> net/core/filter.o: In function `sk_lookup':
> filter.c:(.text+0x7ff8): undefined reference to `tcp_hashinfo'
> filter.c:(.text+0x7ffc): undefined reference to `tcp_hashinfo'
> filter.c:(.text+0x8020): undefined reference to `__inet_lookup_established'
> filter.c:(.text+0x8058): undefined reference to `__inet_lookup_listener'
> filter.c:(.text+0x8068): undefined reference to `udp_table'
> filter.c:(.text+0x8070): undefined reference to `udp_table'
> filter.c:(.text+0x808c): undefined reference to `__udp4_lib_lookup'
> net/core/filter.o: In function `bpf_sk_release':
> filter.c:(.text+0x82e8): undefined reference to `sock_gen_put'
>
> The compiler can optimize it out and avoid those references for
> the most part, but we are missing a few steps here:
>
> - sk_lookup() should always have been marked 'static', this also
> avoids a warning about a missing prototype when building with
> 'make W=1'.
> - The BPF_CALL_x() macro needs a little change to allow marking
> the unneeded BPF call as 'static' and having the compiler
> drop them.
> - The reference to the bpf_func_proto must be made conditional.
>
> Fixes: 6acc9b432e67 ("bpf: Add helper to retrieve socket in BPF")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> include/linux/filter.h | 2 +-
> net/core/filter.c | 18 +++++++++++-------
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 6791a0ac0139..d9ec9d908bbe 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -428,9 +428,9 @@ struct sock_reuseport;
> u64, __ur_3, u64, __ur_4, u64, __ur_5)
>
> #define BPF_CALL_x(x, name, ...) \
> + u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__)); \
> static __always_inline \
> u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__)); \
> - u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__)); \
> u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__)) \
> { \
> return ____##name(__BPF_MAP(x,__BPF_CAST,__BPF_N,__VA_ARGS__));\
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 30c6b2d3ef16..dd5fe021f44c 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4817,7 +4817,7 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
> };
> #endif /* CONFIG_IPV6_SEG6_BPF */
>
> -struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> +static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> struct sk_buff *skb, u8 family, u8 proto)
> {
> int dif = skb->dev->ifindex;
> @@ -4902,13 +4902,13 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> return (unsigned long) sk;
> }
>
> -BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
> +static BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
> struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
> {
> return bpf_sk_lookup(skb, tuple, len, IPPROTO_TCP, netns_id, flags);
> }
BPF_CALL_x() has static already (before this patch). We should not
need change that
for all the BPF_CALL_?(). Joe's version looks better to me.
Thanks,
Song
>
> -static const struct bpf_func_proto bpf_sk_lookup_tcp_proto = {
> +static const __maybe_unused struct bpf_func_proto bpf_sk_lookup_tcp_proto = {
> .func = bpf_sk_lookup_tcp,
> .gpl_only = false,
> .pkt_access = true,
> @@ -4920,13 +4920,13 @@ static const struct bpf_func_proto bpf_sk_lookup_tcp_proto = {
> .arg5_type = ARG_ANYTHING,
> };
>
> -BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
> +static BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
> struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
> {
> return bpf_sk_lookup(skb, tuple, len, IPPROTO_UDP, netns_id, flags);
> }
>
> -static const struct bpf_func_proto bpf_sk_lookup_udp_proto = {
> +static const __maybe_unused struct bpf_func_proto bpf_sk_lookup_udp_proto = {
> .func = bpf_sk_lookup_udp,
> .gpl_only = false,
> .pkt_access = true,
> @@ -4938,14 +4938,14 @@ static const struct bpf_func_proto bpf_sk_lookup_udp_proto = {
> .arg5_type = ARG_ANYTHING,
> };
>
> -BPF_CALL_1(bpf_sk_release, struct sock *, sk)
> +static BPF_CALL_1(bpf_sk_release, struct sock *, sk)
> {
> if (!sock_flag(sk, SOCK_RCU_FREE))
> sock_gen_put(sk);
> return 0;
> }
>
> -static const struct bpf_func_proto bpf_sk_release_proto = {
> +static const __maybe_unused struct bpf_func_proto bpf_sk_release_proto = {
> .func = bpf_sk_release,
> .gpl_only = false,
> .ret_type = RET_INTEGER,
> @@ -5158,12 +5158,14 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> case BPF_FUNC_skb_ancestor_cgroup_id:
> return &bpf_skb_ancestor_cgroup_id_proto;
> #endif
> +#ifdef CONFIG_INET
> case BPF_FUNC_sk_lookup_tcp:
> return &bpf_sk_lookup_tcp_proto;
> case BPF_FUNC_sk_lookup_udp:
> return &bpf_sk_lookup_udp_proto;
> case BPF_FUNC_sk_release:
> return &bpf_sk_release_proto;
> +#endif
> default:
> return bpf_base_func_proto(func_id);
> }
> @@ -5264,12 +5266,14 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> return &bpf_sk_redirect_hash_proto;
> case BPF_FUNC_get_local_storage:
> return &bpf_get_local_storage_proto;
> +#ifdef CONFIG_INET
> case BPF_FUNC_sk_lookup_tcp:
> return &bpf_sk_lookup_tcp_proto;
> case BPF_FUNC_sk_lookup_udp:
> return &bpf_sk_lookup_udp_proto;
> case BPF_FUNC_sk_release:
> return &bpf_sk_release_proto;
> +#endif
> default:
> return bpf_base_func_proto(func_id);
> }
> --
> 2.18.0
>
^ permalink raw reply
* Re: [PATCH net-next 0/6] Fixes, minor changes & cleanups for the Unicast MAC VLAN table
From: David Miller @ 2018-10-05 19:02 UTC (permalink / raw)
To: salil.mehta
Cc: yisen.zhuang, lipeng321, mehta.salil, netdev, linux-kernel,
linuxarm
In-Reply-To: <20181005170329.6512-1-salil.mehta@huawei.com>
From: Salil Mehta <salil.mehta@huawei.com>
Date: Fri, 5 Oct 2018 18:03:23 +0100
> This patch-set presents necessary modifications, fixes & cleanups related
> to Unicast MAC Vlan Table to support revision 0x21 hardware.
Series applied.
^ permalink raw reply
* [PATCH net-next 1/2] net/ncsi: Add NCSI Broadcom OEM command
From: Vijay Khemka @ 2018-10-05 19:01 UTC (permalink / raw)
To: Samuel Mendoza-Jonas, David S. Miller, netdev, linux-kernel
Cc: vijaykhemka, openbmc @ lists . ozlabs . org,
Justin . Lee1 @ Dell . com, joel @ jms . id . au,
linux-aspeed @ lists . ozlabs . org, Sai Dasari,
christian @ cmd . nu
This patch adds OEM Broadcom commands and response handling. It also
defines OEM Get MAC Address handler to get and configure the device.
ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
getting mac address.
ncsi_rsp_handler_oem_bcm: This handles response received for all
broadcom OEM commands.
ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
set it to device.
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
net/ncsi/Kconfig | 6 ++++
net/ncsi/internal.h | 8 +++++
net/ncsi/ncsi-manage.c | 70 +++++++++++++++++++++++++++++++++++++++++-
net/ncsi/ncsi-pkt.h | 8 +++++
net/ncsi/ncsi-rsp.c | 42 ++++++++++++++++++++++++-
5 files changed, 132 insertions(+), 2 deletions(-)
diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
index 08a8a6031fd7..7f2b46108a24 100644
--- a/net/ncsi/Kconfig
+++ b/net/ncsi/Kconfig
@@ -10,3 +10,9 @@ config NET_NCSI
support. Enable this only if your system connects to a network
device via NCSI and the ethernet driver you're using supports
the protocol explicitly.
+config NCSI_OEM_CMD_GET_MAC
+ bool "Get NCSI OEM MAC Address"
+ depends on NET_NCSI
+ ---help---
+ This allows to get MAC address from NCSI firmware and set them back to
+ controller.
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 3d0a33b874f5..45883b32790e 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -71,6 +71,13 @@ enum {
/* OEM Vendor Manufacture ID */
#define NCSI_OEM_MFR_MLX_ID 0x8119
#define NCSI_OEM_MFR_BCM_ID 0x113d
+/* Broadcom specific OEM Command */
+#define NCSI_OEM_BCM_CMD_GMA 0x01 /* CMD ID for Get MAC */
+/* OEM Command payload lengths*/
+#define NCSI_OEM_BCM_CMD_GMA_LEN 12
+/* Mac address offset in OEM response */
+#define BCM_MAC_ADDR_OFFSET 28
+
struct ncsi_channel_version {
u32 version; /* Supported BCD encoded NCSI version */
@@ -240,6 +247,7 @@ enum {
ncsi_dev_state_probe_dp,
ncsi_dev_state_config_sp = 0x0301,
ncsi_dev_state_config_cis,
+ ncsi_dev_state_config_oem_gma,
ncsi_dev_state_config_clear_vids,
ncsi_dev_state_config_svf,
ncsi_dev_state_config_ev,
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 091284760d21..e5bfd9245b5d 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -635,6 +635,39 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
return 0;
}
+#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
+
+/* NCSI OEM Command APIs */
+static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
+{
+ int ret = 0;
+ unsigned char data[NCSI_OEM_BCM_CMD_GMA_LEN];
+
+ nca->payload = NCSI_OEM_BCM_CMD_GMA_LEN;
+
+ memset(data, 0, NCSI_OEM_BCM_CMD_GMA_LEN);
+ *(unsigned int *)data = ntohl(NCSI_OEM_MFR_BCM_ID);
+ data[5] = NCSI_OEM_BCM_CMD_GMA;
+
+ nca->data = data;
+
+ ret = ncsi_xmit_cmd(nca);
+ if (ret)
+ netdev_err(nca->ndp->ndev.dev,
+ "NCSI: Failed to transmit cmd 0x%x during configure\n",
+ nca->type);
+}
+
+/* OEM Command handlers initialization */
+static struct ncsi_oem_gma_handler {
+ unsigned int mfr_id;
+ void (*handler)(struct ncsi_cmd_arg *nca);
+} ncsi_oem_gma_handlers[] = {
+ { NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm }
+};
+
+#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
+
static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
{
struct ncsi_dev *nd = &ndp->ndev;
@@ -643,9 +676,10 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
struct ncsi_channel *nc = ndp->active_channel;
struct ncsi_channel *hot_nc = NULL;
struct ncsi_cmd_arg nca;
+ struct ncsi_oem_gma_handler *nch = NULL;
unsigned char index;
unsigned long flags;
- int ret;
+ int ret, i;
nca.ndp = ndp;
nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
@@ -685,6 +719,40 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
goto error;
}
+#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
+ nd->state = ncsi_dev_state_config_oem_gma;
+ break;
+ case ncsi_dev_state_config_oem_gma:
+ nca.type = NCSI_PKT_CMD_OEM;
+ nca.package = np->id;
+ nca.channel = nc->id;
+ ndp->pending_req_num = 1;
+
+ /* Check for manufacturer id and Find the handler */
+ for (i = 0; i < ARRAY_SIZE(ncsi_oem_gma_handlers); i++) {
+ if (ncsi_oem_gma_handlers[i].mfr_id ==
+ nc->version.mf_id) {
+ if (ncsi_oem_gma_handlers[i].handler)
+ nch = &ncsi_oem_gma_handlers[i];
+ else
+ nch = NULL;
+
+ break;
+ }
+ }
+
+ if (!nch) {
+ netdev_err(ndp->ndev.dev, "No handler available for GMA with MFR-ID (0x%x)\n",
+ nc->version.mf_id);
+ nd->state = ncsi_dev_state_config_clear_vids;
+ schedule_work(&ndp->work);
+ break;
+ }
+
+ /* Get Mac address from NCSI device */
+ nch->handler(&nca);
+#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
+
nd->state = ncsi_dev_state_config_clear_vids;
break;
case ncsi_dev_state_config_clear_vids:
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 0f2087c8d42a..4d3f06be38bd 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -165,6 +165,14 @@ struct ncsi_rsp_oem_pkt {
unsigned char data[]; /* Payload data */
};
+/* Broadcom Response Data */
+struct ncsi_rsp_oem_bcm_pkt {
+ unsigned char ver; /* Payload Version */
+ unsigned char type; /* OEM Command type */
+ __be16 len; /* Payload Length */
+ unsigned char data[]; /* Cmd specific Data */
+};
+
/* Get Link Status */
struct ncsi_rsp_gls_pkt {
struct ncsi_rsp_pkt_hdr rsp; /* Response header */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index d66b34749027..bc20f7036579 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -596,12 +596,52 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
return 0;
}
+/* Response handler for Broadcom command Get Mac Address */
+static int ncsi_rsp_handler_oem_bcm_gma(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_oem_pkt *rsp;
+ struct ncsi_dev_priv *ndp = nr->ndp;
+ struct net_device *ndev = ndp->ndev.dev;
+ int ret = 0;
+ const struct net_device_ops *ops = ndev->netdev_ops;
+ struct sockaddr saddr;
+
+ /* Get the response header */
+ rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+
+ saddr.sa_family = ndev->type;
+ ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+ memcpy(saddr.sa_data, &rsp->data[BCM_MAC_ADDR_OFFSET], ETH_ALEN);
+ /* Increase mac address by 1 for BMC's address */
+ saddr.sa_data[ETH_ALEN - 1]++;
+ ret = ops->ndo_set_mac_address(ndev, &saddr);
+ if (ret < 0)
+ netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");
+
+ return ret;
+}
+
+/* Response handler for Broadcom card */
+static int ncsi_rsp_handler_oem_bcm(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_oem_pkt *rsp;
+ struct ncsi_rsp_oem_bcm_pkt *bcm;
+
+ /* Get the response header */
+ rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+ bcm = (struct ncsi_rsp_oem_bcm_pkt *)(rsp->data);
+
+ if (bcm->type == NCSI_OEM_BCM_CMD_GMA)
+ return ncsi_rsp_handler_oem_bcm_gma(nr);
+ return 0;
+}
+
static struct ncsi_rsp_oem_handler {
unsigned int mfr_id;
int (*handler)(struct ncsi_request *nr);
} ncsi_rsp_oem_handlers[] = {
{ NCSI_OEM_MFR_MLX_ID, NULL },
- { NCSI_OEM_MFR_BCM_ID, NULL }
+ { NCSI_OEM_MFR_BCM_ID, ncsi_rsp_handler_oem_bcm }
};
/* Response handler for OEM command */
--
2.17.1
^ permalink raw reply related
* [PATCH] ath10k: remove unnecessary comparison of unsigned integer with < 0
From: Gustavo A. R. Silva @ 2018-10-05 18:56 UTC (permalink / raw)
To: Kalle Valo, David S. Miller
Cc: ath10k, linux-wireless, netdev, linux-kernel, Gustavo A. R. Silva
There is no need to compare *ps_state_enable* with < 0 because
such variable is of type u8 (8 bits, unsigned), making it
impossible to hold a negative value.
Fix this by removing such comparison.
Addresses-Coverity-ID: 1473921 ("Unsigned compared against 0")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/net/wireless/ath/ath10k/debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 2c0cb67..15964b3 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -2421,7 +2421,7 @@ static ssize_t ath10k_write_ps_state_enable(struct file *file,
if (kstrtou8_from_user(user_buf, count, 0, &ps_state_enable))
return -EINVAL;
- if (ps_state_enable > 1 || ps_state_enable < 0)
+ if (ps_state_enable > 1)
return -EINVAL;
mutex_lock(&ar->conf_mutex);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] yam: fix a missing-check bug
From: David Miller @ 2018-10-05 18:55 UTC (permalink / raw)
To: wang6495; +Cc: kjlu, jpr, linux-hams, netdev, linux-kernel
In-Reply-To: <1538755176-22355-1-git-send-email-wang6495@umn.edu>
From: Wenwen Wang <wang6495@umn.edu>
Date: Fri, 5 Oct 2018 10:59:36 -0500
> In yam_ioctl(), the concrete ioctl command is firstly copied from the
> user-space buffer 'ifr->ifr_data' to 'ioctl_cmd' and checked through the
> following switch statement. If the command is not as expected, an error
> code EINVAL is returned. In the following execution the buffer
> 'ifr->ifr_data' is copied again in the cases of the switch statement to
> specific data structures according to what kind of ioctl command is
> requested. However, after the second copy, no re-check is enforced on the
> newly-copied command. Given that the buffer 'ifr->ifr_data' is in the user
> space, a malicious user can race to change the command between the two
> copies. This way, the attacker can inject inconsistent data and cause
> undefined behavior.
>
> This patch adds a re-check in each case of the switch statement if there is
> a second copy in that case, to re-check whether the command obtained in the
> second copy is the same as the one in the first copy. If not, an error code
> EINVAL will be returned.
>
> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] net: bpfilter: Fix type cast and pointer warnings
From: David Miller @ 2018-10-05 18:51 UTC (permalink / raw)
To: shanthosh.rk; +Cc: netdev, linux-kernel
In-Reply-To: <20181005152748.gotzlu2lcvkeoogb@debian.domain>
From: Shanthosh RK <shanthosh.rk@gmail.com>
Date: Fri, 5 Oct 2018 20:57:48 +0530
> Fixes the following Sparse warnings:
>
> net/bpfilter/bpfilter_kern.c:62:21: warning: cast removes address space
> of expression
> net/bpfilter/bpfilter_kern.c:101:49: warning: Using plain integer as
> NULL pointer
>
> Signed-off-by: Shanthosh RK <shanthosh.rk@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] gigaset: asyncdata: mark expected switch fall-throughs
From: David Miller @ 2018-10-05 18:51 UTC (permalink / raw)
To: gustavo; +Cc: pebolle, isdn, gigaset307x-common, netdev, linux-kernel
In-Reply-To: <20181005132909.GA5506@embeddedor.com>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Fri, 5 Oct 2018 15:29:09 +0200
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Notice that in this particular case, I replaced the
> " --v-- fall through --v-- " comment with a proper
> "fall through", which is what GCC is expecting to find.
>
> Addresses-Coverity-ID: 1364476 ("Missing break in switch")
> Addresses-Coverity-ID: 1364477 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied.
^ permalink raw reply
* Re: [PATCH] wil6210: fix debugfs_simple_attr.cocci warnings
From: YueHaibing @ 2018-10-05 11:51 UTC (permalink / raw)
To: Kalle Valo; +Cc: Maya Erez, linux-wireless, wil6210, kernel-janitors, netdev
In-Reply-To: <877eiw1wol.fsf@codeaurora.org>
On 2018/10/5 19:04, Kalle Valo wrote:
> YueHaibing <yuehaibing@huawei.com> writes:
>
>> Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
>> for debugfs files.
>>
>> Semantic patch information:
>> Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
>> imposes some significant overhead as compared to
>> DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
>>
>> Generated by: scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
>
> Just out of curiosity, what kind of overhead are we talking about here?
commit 5103068eaca2 ("debugfs, coccinelle: check for obsolete DEFINE_SIMPLE_ATTRIBUTE() usage")
said this:
In order to protect against file removal races, debugfs files created via
debugfs_create_file() now get wrapped by a struct file_operations at their
opening.
If the original struct file_operations are known to be safe against removal
races by themselves already, the proxy creation may be bypassed by creating
the files through debugfs_create_file_unsafe().
In order to help debugfs users who use the common
DEFINE_SIMPLE_ATTRIBUTE() + debugfs_create_file()
idiom to transition to removal safe struct file_operations, the helper
macro DEFINE_DEBUGFS_ATTRIBUTE() has been introduced.
>
^ permalink raw reply
* Re: [PATCH] net: cxgb3_main: fix a missing-check bug
From: David Miller @ 2018-10-05 18:47 UTC (permalink / raw)
To: wang6495; +Cc: kjlu, santosh, netdev, linux-kernel
In-Reply-To: <1538747307-21403-1-git-send-email-wang6495@umn.edu>
From: Wenwen Wang <wang6495@umn.edu>
Date: Fri, 5 Oct 2018 08:48:27 -0500
> In cxgb_extension_ioctl(), the command of the ioctl is firstly copied from
> the user-space buffer 'useraddr' to 'cmd' and checked through the
> switch statement. If the command is not as expected, an error code
> EOPNOTSUPP is returned. In the following execution, i.e., the cases of the
> switch statement, the whole buffer of 'useraddr' is copied again to a
> specific data structure, according to what kind of command is requested.
> However, after the second copy, there is no re-check on the newly-copied
> command. Given that the buffer 'useraddr' is in the user space, a malicious
> user can race to change the command between the two copies. By doing so,
> the attacker can supply malicious data to the kernel and cause undefined
> behavior.
>
> This patch adds a re-check in each case of the switch statement if there is
> a second copy in that case, to re-check whether the command obtained in the
> second copy is the same as the one in the first copy. If not, an error code
> EINVAL is returned.
>
> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
Applied.
^ permalink raw reply
* [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
From: Gustavo A. R. Silva @ 2018-10-05 18:42 UTC (permalink / raw)
To: Kalle Valo, David S. Miller
Cc: netdev, linux-wireless, linux-kernel, ath10k, Gustavo A. R. Silva
Currently, the error handling for the call to function
ath10k_get_legacy_rate_idx() doesn't work because
*rate_idx* is of type u8 (8 bits, unsigned), which
makes it impossible for it to hold a value less
than 0.
Fix this by changing the type of variable *rate_idx*
to s8 (8 bits, signed).
Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index f240525..edd0e74 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
struct ath10k_per_peer_tx_stats *peer_stats)
{
struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
- u8 rate = 0, rate_idx = 0, sgi;
+ u8 rate = 0, sgi;
+ s8 rate_idx = 0;
struct rate_info txrate;
lockdep_assert_held(&ar->data_lock);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next v6 19/23] zinc: Curve25519 ARM implementation
From: Jason A. Donenfeld @ 2018-10-05 18:40 UTC (permalink / raw)
To: ard.biesheuvel, linux-kernel, netdev, linux-crypto, davem, gregkh,
sneves, luto, jeanphilippe.aumasson, linux, linux-arm-kernel,
peter, djb
In-Reply-To: <20181005150538.17006.qmail@cr.yp.to>
[-- Attachment #1: Type: text/plain, Size: 934 bytes --]
Hey Dan,
On Fri, Oct 05, 2018 at 03:05:38PM -0000, D. J. Bernstein wrote:
> Of course, there are other ARM microarchitectures, and there are many
> cases where different microarchitectures prefer different optimizations.
> The kernel already has boot-time benchmarks for different optimizations
> for raid6, and should do the same for crypto code, so that implementors
> can focus on each microarchitecture separately rather than living in the
> barbaric world of having to choose which CPUs to favor.
I've been playing a bit with some code to do this sort of thing,
choosing a set of implementations to enable or disable by trying all the
combinations, and then calculating a quick median. I don't know if I'll
submit that for the initial merge of this patchset -- and in fact all
the current implementations I'm proposing are pretty much okay on
microarchitectures -- but down the line this could be useful as a
mechanism.
Jason
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: question regarding Linux kernel handling of packets received that has src address of interface address
From: Mikael Abrahamsson @ 2018-10-05 11:41 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev
In-Reply-To: <20181005102701.GA20105@splinter>
On Fri, 5 Oct 2018, Ido Schimmel wrote:
> Did you set 'accept_local' [1] ?
I did not. Thanks for the pointer, looks like exactly what I was looking
for!
--
Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
* Re: [PATCH v2 0/5] Introducing ixgbe AF_XDP ZC support
From: Björn Töpel @ 2018-10-05 11:30 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Björn Töpel
Cc: jeffrey.t.kirsher, intel-wired-lan, magnus.karlsson,
magnus.karlsson, ast, daniel, netdev, u9012063, tuc,
jakub.kicinski
In-Reply-To: <5a801d72-6dcd-a8e4-f1a6-c8f3cc1e560e@intel.com>
On 2018-10-05 06:59, Björn Töpel wrote:
> On 2018-10-04 23:18, Jesper Dangaard Brouer wrote:
>> I see similar performance numbers, but my system can crash with 'txonly'.
>
> Thanks for finding this, Jesper!
>
> Can you give me your "lspci -vvv" dump of your NIC, so I know what ixgbe
> flavor you've got?
>
> I'll dig into it right away.
>
Jesper, there's (hopefully) a fix for the crash here:
https://patchwork.ozlabs.org/patch/979442/
Thanks for spending time on the ixgbe ZC patches!
>
> Björn
^ permalink raw reply
* [PATCH bpf-next] xsk: proper AF_XDP socket teardown ordering
From: Björn Töpel @ 2018-10-05 11:25 UTC (permalink / raw)
To: ast, daniel, netdev, brouer
Cc: Björn Töpel, magnus.karlsson, magnus.karlsson
From: Björn Töpel <bjorn.topel@intel.com>
The AF_XDP socket struct can exist in three different, implicit
states: setup, bound and released. Setup is prior the socket has been
bound to a device. Bound is when the socket is active for receive and
send. Released is when the process/userspace side of the socket is
released, but the sock object is still lingering, e.g. when there is a
reference to the socket in an XSKMAP after process termination.
The Rx fast-path code uses the "dev" member of struct xdp_sock to
check whether a socket is bound or relased, and the Tx code uses the
struct xdp_umem "xsk_list" member in conjunction with "dev" to
determine the state of a socket.
However, the transition from bound to released did not tear the socket
down in correct order.
On the Rx side "dev" was cleared after synchronize_net() making the
synchronization useless. On the Tx side, the internal queues were
destroyed prior removing them from the "xsk_list".
This commit corrects the cleanup order, and by doing so
xdp_del_sk_umem() can be simplified and one synchronize_net() can be
removed.
Fixes: 965a99098443 ("xsk: add support for bind for Rx")
Fixes: ac98d8aab61b ("xsk: wire upp Tx zero-copy functions")
Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
net/xdp/xdp_umem.c | 11 +++--------
net/xdp/xsk.c | 13 ++++++++-----
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index c6007c58231c..a264cf2accd0 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -32,14 +32,9 @@ void xdp_del_sk_umem(struct xdp_umem *umem, struct xdp_sock *xs)
{
unsigned long flags;
- if (xs->dev) {
- spin_lock_irqsave(&umem->xsk_list_lock, flags);
- list_del_rcu(&xs->list);
- spin_unlock_irqrestore(&umem->xsk_list_lock, flags);
-
- if (umem->zc)
- synchronize_net();
- }
+ spin_lock_irqsave(&umem->xsk_list_lock, flags);
+ list_del_rcu(&xs->list);
+ spin_unlock_irqrestore(&umem->xsk_list_lock, flags);
}
/* The umem is stored both in the _rx struct and the _tx struct as we do
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index caeddad15b7c..0577cd49aa72 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -355,12 +355,18 @@ static int xsk_release(struct socket *sock)
local_bh_enable();
if (xs->dev) {
+ struct net_device *dev = xs->dev;
+
/* Wait for driver to stop using the xdp socket. */
- synchronize_net();
- dev_put(xs->dev);
+ xdp_del_sk_umem(xs->umem, xs);
xs->dev = NULL;
+ synchronize_net();
+ dev_put(dev);
}
+ xskq_destroy(xs->rx);
+ xskq_destroy(xs->tx);
+
sock_orphan(sk);
sock->sk = NULL;
@@ -714,9 +720,6 @@ static void xsk_destruct(struct sock *sk)
if (!sock_flag(sk, SOCK_DEAD))
return;
- xskq_destroy(xs->rx);
- xskq_destroy(xs->tx);
- xdp_del_sk_umem(xs->umem, xs);
xdp_put_umem(xs->umem);
sk_refcnt_debug_dec(sk);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next v6 00/23] WireGuard: Secure Network Tunnel
From: David Miller @ 2018-10-05 17:50 UTC (permalink / raw)
To: richard.weinberger
Cc: Jason, ebiggers, ard.biesheuvel, herbert, linux-kernel, netdev,
linux-crypto, gregkh
In-Reply-To: <CAFLxGvw154vyOziRKuFf5-XTa1NcQm5ZVmgUy3=F79k=Rw2dvA@mail.gmail.com>
From: Richard Weinberger <richard.weinberger@gmail.com>
Date: Fri, 5 Oct 2018 15:37:57 +0200
> And I strongly vote that Herbert Xu shall remain the maintainer of
> the whole crypto system (including zinc!) in the kernel.
I 100% agree with this.
^ permalink raw reply
* Re: [PATCH] bpf: 32-bit RSH verification must truncate input before the ALU op
From: Daniel Borkmann @ 2018-10-05 17:47 UTC (permalink / raw)
To: Jann Horn, Alexei Starovoitov, netdev; +Cc: David S. Miller, linux-kernel
In-Reply-To: <20181005161759.177992-1-jannh@google.com>
On 10/05/2018 06:17 PM, Jann Horn wrote:
> When I wrote commit 468f6eafa6c4 ("bpf: fix 32-bit ALU op verification"), I
> assumed that, in order to emulate 64-bit arithmetic with 32-bit logic, it
> is sufficient to just truncate the output to 32 bits; and so I just moved
> the register size coercion that used to be at the start of the function to
> the end of the function.
>
> That assumption is true for almost every op, but not for 32-bit right
> shifts, because those can propagate information towards the least
> significant bit. Fix it by always truncating inputs for 32-bit ops to 32
> bits.
>
> Also get rid of the coerce_reg_to_size() after the ALU op, since that has
> no effect.
>
> Fixes: 468f6eafa6c4 ("bpf: fix 32-bit ALU op verification")
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Jann Horn <jannh@google.com>
Applied to bpf, thanks Jann!
^ permalink raw reply
* [PATCH net-next] net/ncsi: Add NCSI OEM command support
From: Vijay Khemka @ 2018-10-05 17:46 UTC (permalink / raw)
To: Samuel Mendoza-Jonas, David S. Miller, netdev, linux-kernel
Cc: vijaykhemka, openbmc @ lists . ozlabs . org,
Justin . Lee1 @ Dell . com, joel @ jms . id . au,
linux-aspeed @ lists . ozlabs . org, Sai Dasari,
christian @ cmd . nu
This patch adds OEM commands and response handling. It also defines OEM
command and response structure as per NCSI specification along with its
handlers.
ncsi_cmd_handler_oem: This is a generic command request handler for OEM
commands
ncsi_rsp_handler_oem: This is a generic response handler for OEM commands
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
Reviewed-by: Justin Lee <justin.lee1@dell.com>
Reviewed-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
net/ncsi/internal.h | 5 +++++
net/ncsi/ncsi-cmd.c | 30 +++++++++++++++++++++++++++---
net/ncsi/ncsi-pkt.h | 14 ++++++++++++++
net/ncsi/ncsi-rsp.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 88 insertions(+), 4 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 8055e3965cef..3d0a33b874f5 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -68,6 +68,10 @@ enum {
NCSI_MODE_MAX
};
+/* OEM Vendor Manufacture ID */
+#define NCSI_OEM_MFR_MLX_ID 0x8119
+#define NCSI_OEM_MFR_BCM_ID 0x113d
+
struct ncsi_channel_version {
u32 version; /* Supported BCD encoded NCSI version */
u32 alpha2; /* Supported BCD encoded NCSI version */
@@ -305,6 +309,7 @@ struct ncsi_cmd_arg {
unsigned short words[8];
unsigned int dwords[4];
};
+ unsigned char *data; /* NCSI OEM data */
};
extern struct list_head ncsi_dev_list;
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 7567ca63aae2..82b7d9201db8 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
return 0;
}
+static int ncsi_cmd_handler_oem(struct sk_buff *skb,
+ struct ncsi_cmd_arg *nca)
+{
+ struct ncsi_cmd_oem_pkt *cmd;
+ unsigned int len;
+
+ len = sizeof(struct ncsi_cmd_pkt_hdr) + 4;
+ if (nca->payload < 26)
+ len += 26;
+ else
+ len += nca->payload;
+
+ cmd = skb_put_zero(skb, len);
+ memcpy(&cmd->mfr_id, nca->data, nca->payload);
+ ncsi_cmd_build_header(&cmd->cmd.common, nca);
+
+ return 0;
+}
+
static struct ncsi_cmd_handler {
unsigned char type;
int payload;
@@ -244,7 +263,7 @@ static struct ncsi_cmd_handler {
{ NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default },
{ NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default },
{ NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_OEM, 0, NULL },
+ { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem },
{ NCSI_PKT_CMD_PLDM, 0, NULL },
{ NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
};
@@ -316,8 +335,13 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
return -ENOENT;
}
- /* Get packet payload length and allocate the request */
- nca->payload = nch->payload;
+ /* Get packet payload length and allocate the request
+ * It is expected that if length set as negative in
+ * handler structure means caller is initializing it
+ * and setting length in nca before calling xmit function
+ */
+ if (nch->payload >= 0)
+ nca->payload = nch->payload;
nr = ncsi_alloc_command(nca);
if (!nr)
return -ENOMEM;
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 91b4b66438df..0f2087c8d42a 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -151,6 +151,20 @@ struct ncsi_cmd_snfc_pkt {
unsigned char pad[22];
};
+/* OEM Request Command as per NCSI Specification */
+struct ncsi_cmd_oem_pkt {
+ struct ncsi_cmd_pkt_hdr cmd; /* Command header */
+ __be32 mfr_id; /* Manufacture ID */
+ unsigned char data[]; /* OEM Payload Data */
+};
+
+/* OEM Response Packet as per NCSI Specification */
+struct ncsi_rsp_oem_pkt {
+ struct ncsi_rsp_pkt_hdr rsp; /* Command header */
+ __be32 mfr_id; /* Manufacture ID */
+ unsigned char data[]; /* Payload data */
+};
+
/* Get Link Status */
struct ncsi_rsp_gls_pkt {
struct ncsi_rsp_pkt_hdr rsp; /* Response header */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 930c1d3796f0..d66b34749027 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -596,6 +596,47 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
return 0;
}
+static struct ncsi_rsp_oem_handler {
+ unsigned int mfr_id;
+ int (*handler)(struct ncsi_request *nr);
+} ncsi_rsp_oem_handlers[] = {
+ { NCSI_OEM_MFR_MLX_ID, NULL },
+ { NCSI_OEM_MFR_BCM_ID, NULL }
+};
+
+/* Response handler for OEM command */
+static int ncsi_rsp_handler_oem(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_oem_pkt *rsp;
+ struct ncsi_rsp_oem_handler *nrh = NULL;
+ unsigned int mfr_id, i;
+
+ /* Get the response header */
+ rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+ mfr_id = ntohl(rsp->mfr_id);
+
+ /* Check for manufacturer id and Find the handler */
+ for (i = 0; i < ARRAY_SIZE(ncsi_rsp_oem_handlers); i++) {
+ if (ncsi_rsp_oem_handlers[i].mfr_id == mfr_id) {
+ if (ncsi_rsp_oem_handlers[i].handler)
+ nrh = &ncsi_rsp_oem_handlers[i];
+ else
+ nrh = NULL;
+
+ break;
+ }
+ }
+
+ if (!nrh) {
+ netdev_err(nr->ndp->ndev.dev, "Received unrecognized OEM packet with MFR-ID (0x%x)\n",
+ mfr_id);
+ return -ENOENT;
+ }
+
+ /* Process the packet */
+ return nrh->handler(nr);
+}
+
static int ncsi_rsp_handler_gvi(struct ncsi_request *nr)
{
struct ncsi_rsp_gvi_pkt *rsp;
@@ -932,7 +973,7 @@ static struct ncsi_rsp_handler {
{ NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns },
{ NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts },
{ NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps },
- { NCSI_PKT_RSP_OEM, 0, NULL },
+ { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem },
{ NCSI_PKT_RSP_PLDM, 0, NULL },
{ NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid }
};
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox