* [net-next v3 2/5] net/tls: Use socket data_ready callback on record availability
From: Vakul Garg @ 2018-07-19 11:16 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719111643.12787-1-vakul.garg@nxp.com>
On receipt of a complete tls record, use socket's saved data_ready
callback instead of state_change callback.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
net/tls/tls_sw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index e94cb54a6994..186152dced25 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1019,7 +1019,7 @@ static void tls_queue(struct strparser *strp, struct sk_buff *skb)
ctx->recv_pkt = skb;
strp_pause(strp);
- strp->sk->sk_state_change(strp->sk);
+ ctx->saved_data_ready(strp->sk);
}
static void tls_data_ready(struct sock *sk)
--
2.13.6
^ permalink raw reply related
* [net-next v3 3/5] net/tls: Remove redundant variable assignments and wakeup
From: Vakul Garg @ 2018-07-19 11:16 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719111643.12787-1-vakul.garg@nxp.com>
In function decrypt_skb_update(), the assignment to tls receive context
variable 'decrypted' is redundant as the same is being done in function
tls_sw_recvmsg() after calling decrypt_skb_update(). Also calling callback
function to wakeup processes sleeping on socket data availability is
useless as decrypt_skb_update() is invoked from user processes only. This
patch cleans these up.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
v2 -> v3
Removed compilation warning.
net/tls/tls_sw.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 186152dced25..5dcfbaf33680 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -659,7 +659,6 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
struct scatterlist *sgout)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
- struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
struct strp_msg *rxm = strp_msg(skb);
int err = 0;
@@ -675,8 +674,6 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
rxm->offset += tls_ctx->rx.prepend_size;
rxm->full_len -= tls_ctx->rx.overhead_size;
tls_advance_record_sn(sk, &tls_ctx->rx);
- ctx->decrypted = true;
- ctx->saved_data_ready(sk);
return err;
}
--
2.13.6
^ permalink raw reply related
* [net-next v3 4/5] net/tls: Remove redundant array allocation.
From: Vakul Garg @ 2018-07-19 11:16 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719111643.12787-1-vakul.garg@nxp.com>
In function decrypt_skb(), array allocation in case when sgout is NULL
is unnecessary. Instead, local variable sgin_arr[] can be used.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
net/tls/tls_sw.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 5dcfbaf33680..ae0b40d6671b 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -699,7 +699,6 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
if (!sgout) {
nsg = skb_cow_data(skb, 0, &unused) + 1;
- sgin = kmalloc_array(nsg, sizeof(*sgin), sk->sk_allocation);
sgout = sgin;
}
@@ -720,9 +719,6 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
rxm->full_len - tls_ctx->rx.overhead_size,
skb, sk->sk_allocation);
- if (sgin != &sgin_arr[0])
- kfree(sgin);
-
return ret;
}
--
2.13.6
^ permalink raw reply related
* [net-next v3 5/5] net/tls: Rework error checking after decrypt_skb_update()
From: Vakul Garg @ 2018-07-19 11:16 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719111643.12787-1-vakul.garg@nxp.com>
Error checking code after invoking decrypt_skb_update() for zero-copy
and non-zero-copy cases in tls_sw_recvmsg has been made common.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
net/tls/tls_sw.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index ae0b40d6671b..da3b884bea93 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -824,18 +824,16 @@ int tls_sw_recvmsg(struct sock *sk,
err = decrypt_skb_update(sk, skb, sgin);
for (; pages > 0; pages--)
put_page(sg_page(&sgin[pages]));
- if (err < 0) {
- tls_err_abort(sk, EBADMSG);
- goto recv_end;
- }
} else {
fallback_to_reg_recv:
err = decrypt_skb_update(sk, skb, NULL);
- if (err < 0) {
- tls_err_abort(sk, EBADMSG);
- goto recv_end;
- }
}
+
+ if (err < 0) {
+ tls_err_abort(sk, EBADMSG);
+ goto recv_end;
+ }
+
ctx->decrypted = true;
}
--
2.13.6
^ permalink raw reply related
* Re: [PATCH net 1/2] openvswitch: check for null return for nla_nest_start
From: Pravin Shelar @ 2018-07-19 6:49 UTC (permalink / raw)
To: Stephen Hemminger
Cc: ovs dev, Linux Kernel Network Developers, Stephen Hemminger,
David S. Miller
In-Reply-To: <20180718161216.27820-2-sthemmin-0li6OtcxBFHby3iVrkZq2A@public.gmane.org>
On Wed, Jul 18, 2018 at 9:12 AM, Stephen Hemminger
<stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org> wrote:
> The call to nla_nest_start in conntrack can lead to a NULL
> return so it's possible for attr to become NULL and we can potentially
> get a NULL pointer dereference on attr. Fix this by checking for
> a NULL return.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=200533
> Fixes: 11efd5cb04a1 ("openvswitch: Support conntrack zone limit")
> Signed-off-by: Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>
> ---
> net/openvswitch/conntrack.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 284aca2a252d..2e316f641df8 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -2132,6 +2132,8 @@ static int ovs_ct_limit_cmd_get(struct sk_buff *skb, struct genl_info *info)
> return PTR_ERR(reply);
>
> nla_reply = nla_nest_start(reply, OVS_CT_LIMIT_ATTR_ZONE_LIMIT);
> + if (!nla_reply)
> + return PRT_ERR(-EMSGSIZE);
>
> if (a[OVS_CT_LIMIT_ATTR_ZONE_LIMIT]) {
> err = ovs_ct_limit_get_zone_limit(
> --
Acked-by: Pravin B Shelar <pshelar-LZ6Gd1LRuIk@public.gmane.org>
Thanks.
^ permalink raw reply
* Re: [PATCH net 2/2] openvswitch: check for null return for nla_nest_start in datapath
From: Pravin Shelar @ 2018-07-19 6:49 UTC (permalink / raw)
To: Stephen Hemminger
Cc: ovs dev, Linux Kernel Network Developers, Stephen Hemminger,
David S. Miller
In-Reply-To: <20180718161216.27820-3-sthemmin-0li6OtcxBFHby3iVrkZq2A@public.gmane.org>
On Wed, Jul 18, 2018 at 9:12 AM, Stephen Hemminger
<stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org> wrote:
> The call to nla_nest_start when forming packet messages can lead to a NULL
> return so it's possible for attr to become NULL and we can potentially
> get a NULL pointer dereference on attr. Fix this by checking for
> a NULL return.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=200537
> Fixes: 8f0aad6f35f7 ("openvswitch: Extend packet attribute for egress tunnel info")
> Signed-off-by: Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>
> ---
> net/openvswitch/datapath.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 0f5ce77460d4..93c3eb635827 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -460,6 +460,10 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
>
> if (upcall_info->egress_tun_info) {
> nla = nla_nest_start(user_skb, OVS_PACKET_ATTR_EGRESS_TUN_KEY);
> + if (!nla) {
> + err = -EMSGSIZE;
> + goto out;
> + }
> err = ovs_nla_put_tunnel_info(user_skb,
> upcall_info->egress_tun_info);
> BUG_ON(err);
> @@ -468,6 +472,10 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
>
> if (upcall_info->actions_len) {
> nla = nla_nest_start(user_skb, OVS_PACKET_ATTR_ACTIONS);
> + if (!nla) {
> + err = -EMSGSIZE;
> + goto out;
> + }
> err = ovs_nla_put_actions(upcall_info->actions,
> upcall_info->actions_len,
> user_skb);
Acked-by: Pravin B Shelar <pshelar-LZ6Gd1LRuIk@public.gmane.org>
Thanks.
^ permalink raw reply
* [PATCH net] net: phy: consider PHY_IGNORE_INTERRUPT in phy_start_aneg_priv
From: Heiner Kallweit @ 2018-07-19 6:15 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
The situation described in the comment can occur also with
PHY_IGNORE_INTERRUPT, therefore change the condition to include it.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index d2baedc4..914fe8e6 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -519,7 +519,7 @@ static int phy_start_aneg_priv(struct phy_device *phydev, bool sync)
* negotiation may already be done and aneg interrupt may not be
* generated.
*/
- if (phy_interrupt_is_valid(phydev) && (phydev->state == PHY_AN)) {
+ if (phydev->irq != PHY_POLL && phydev->state == PHY_AN) {
err = phy_aneg_done(phydev);
if (err > 0) {
trigger = true;
--
2.18.0
^ permalink raw reply related
* Re: [net 4/8] net/mlx5e: Don't allow aRFS for encapsulated packets
From: Or Gerlitz @ 2018-07-19 6:23 UTC (permalink / raw)
To: Saeed Mahameed, Eran Ben Elisha
Cc: David S. Miller, Linux Netdev List, Alexander Duyck, Tom Herbert
In-Reply-To: <20180719012612.25907-5-saeedm@mellanox.com>
On Thu, Jul 19, 2018 at 4:26 AM, Saeed Mahameed <saeedm@mellanox.com> wrote:
> From: Eran Ben Elisha <eranbe@mellanox.com>
>
> Driver is yet to support aRFS for encapsulated packets, return early
> error in such case.
Eran,
Isn't that something which is done wrong by the arfs stack code?
If the kernel has an SKB which has encap set and an arfs steering
rule is programed into the driver, the API should include a driver neutral
description for the encap header for the HW to match, so maybe we can just do
diff --git a/net/core/dev.c b/net/core/dev.c
index 0df1771..b93008f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3934,6 +3934,10 @@ set_rps_cpu(struct net_device *dev, struct sk_buff *skb,
flow_table = rcu_dereference(rxqueue->rps_flow_table);
if (!flow_table)
goto out;
+
+ if (skb->encapsulation)
+ return -EPROTONOSUPPORT;
+
flow_id = skb_get_hash(skb) & flow_table->mask;
rc = dev->netdev_ops->ndo_rx_flow_steer(dev, skb,
rxq_index, flow_id);
^ permalink raw reply related
* [PATCH bpf-next] bpfilter: Fix mismatch in function argument types
From: YueHaibing @ 2018-07-19 7:25 UTC (permalink / raw)
To: ast, daniel; +Cc: linux-kernel, netdev, YueHaibing
Fix following warning:
net/ipv4/bpfilter/sockopt.c:28:5: error: symbol 'bpfilter_ip_set_sockopt' redeclared with different type
net/ipv4/bpfilter/sockopt.c:34:5: error: symbol 'bpfilter_ip_get_sockopt' redeclared with different type
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
include/linux/bpfilter.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index 687b176..43acfa8 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -5,9 +5,9 @@
#include <uapi/linux/bpfilter.h>
struct sock;
-int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char *optval,
+int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char __user *optval,
unsigned int optlen);
-int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char *optval,
+int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
int *optlen);
extern int (*bpfilter_process_sockopt)(struct sock *sk, int optname,
char __user *optval,
--
2.7.0
^ permalink raw reply related
* Re: [net 4/8] net/mlx5e: Don't allow aRFS for encapsulated packets
From: Eran Ben Elisha @ 2018-07-19 6:55 UTC (permalink / raw)
To: Or Gerlitz
Cc: Saeed Mahameed, Eran Ben Elisha, David S. Miller,
Linux Netdev List, Alexander Duyck, Tom Herbert
In-Reply-To: <CAJ3xEMimJ-KGmBXCWzEaOUULSTTq+wGV49oiXpw1f+PUaTpPNQ@mail.gmail.com>
On Thu, Jul 19, 2018 at 9:23 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Thu, Jul 19, 2018 at 4:26 AM, Saeed Mahameed <saeedm@mellanox.com> wrote:
>> From: Eran Ben Elisha <eranbe@mellanox.com>
>>
>> Driver is yet to support aRFS for encapsulated packets, return early
>> error in such case.
>
>
> Eran,
>
> Isn't that something which is done wrong by the arfs stack code?
>
> If the kernel has an SKB which has encap set and an arfs steering
> rule is programed into the driver, the API should include a driver neutral
> description for the encap header for the HW to match, so maybe we can just do
>
Hi Or,
This could break existing drivers support for tunneled aRFS, and hurts
their RX performance dramatically..
IMHO, it is expected from the driver to figure out that the skb holds
encap packet and act accordingly.
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 0df1771..b93008f 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3934,6 +3934,10 @@ set_rps_cpu(struct net_device *dev, struct sk_buff *skb,
> flow_table = rcu_dereference(rxqueue->rps_flow_table);
> if (!flow_table)
> goto out;
> +
> + if (skb->encapsulation)
> + return -EPROTONOSUPPORT;
> +
> flow_id = skb_get_hash(skb) & flow_table->mask;
> rc = dev->netdev_ops->ndo_rx_flow_steer(dev, skb,
> rxq_index, flow_id);
^ permalink raw reply
* Re: [RFC PATCH 3/3] net: macb: add support for padding and fcs computation
From: Claudiu Beznea @ 2018-07-19 7:03 UTC (permalink / raw)
To: David Miller; +Cc: nicolas.ferre, netdev, jennifer.dahm, nathan.sullivan
In-Reply-To: <20180719.025423.1409264262161292298.davem@davemloft.net>
On 18.07.2018 20:54, David Miller wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> Date: Wed, 18 Jul 2018 15:58:09 +0300
>
>>
>> +static int macb_pad_and_fcs(struct sk_buff **skb, struct net_device *ndev)
>> +{
>> + struct sk_buff *nskb;
>> + int padlen = ETH_ZLEN - (*skb)->len;
>> + int headroom = skb_headroom(*skb);
>> + int tailroom = skb_tailroom(*skb);
>> + bool cloned = skb_cloned(*skb) || skb_header_cloned(*skb);
>> + u32 fcs;
>
> Please keep local variable ordered from longest to shortest line
> (ie. reverse christmas tree format).
OK! Thank you!
>
> Thank you.
>
^ permalink raw reply
* Re: [PATCH bpf-next] bpfilter: Fix mismatch in function argument types
From: YueHaibing @ 2018-07-19 7:55 UTC (permalink / raw)
To: ast, daniel; +Cc: linux-kernel, netdev
In-Reply-To: <20180719072556.20076-1-yuehaibing@huawei.com>
send wrong patch ,pls ignore it.
On 2018/7/19 15:25, YueHaibing wrote:
> Fix following warning:
> net/ipv4/bpfilter/sockopt.c:28:5: error: symbol 'bpfilter_ip_set_sockopt' redeclared with different type
> net/ipv4/bpfilter/sockopt.c:34:5: error: symbol 'bpfilter_ip_get_sockopt' redeclared with different type
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> include/linux/bpfilter.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
> index 687b176..43acfa8 100644
> --- a/include/linux/bpfilter.h
> +++ b/include/linux/bpfilter.h
> @@ -5,9 +5,9 @@
> #include <uapi/linux/bpfilter.h>
>
> struct sock;
> -int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char *optval,
> +int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char __user *optval,
> unsigned int optlen);
> -int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char *optval,
> +int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
> int *optlen);
> extern int (*bpfilter_process_sockopt)(struct sock *sk, int optname,
> char __user *optval,
>
^ permalink raw reply
* [PATCH v2 bpf-next] bpfilter: Fix mismatch in function argument types
From: YueHaibing @ 2018-07-19 7:56 UTC (permalink / raw)
To: ast, daniel; +Cc: linux-kernel, netdev, YueHaibing
Fix following warning:
net/ipv4/bpfilter/sockopt.c:28:5: error: symbol 'bpfilter_ip_set_sockopt' redeclared with different type
net/ipv4/bpfilter/sockopt.c:34:5: error: symbol 'bpfilter_ip_get_sockopt' redeclared with different type
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
include/linux/bpfilter.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index 687b176..f02cee0 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -5,10 +5,10 @@
#include <uapi/linux/bpfilter.h>
struct sock;
-int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char *optval,
+int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char __user *optval,
unsigned int optlen);
-int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char *optval,
- int *optlen);
+int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
+ int __user *optlen);
extern int (*bpfilter_process_sockopt)(struct sock *sk, int optname,
char __user *optval,
unsigned int optlen, bool is_set);
--
2.7.0
^ permalink raw reply related
* Re: [PATCH 5/5] net: add MTD support to eth_platform_get_mac_address()
From: Bartosz Golaszewski @ 2018-07-19 8:14 UTC (permalink / raw)
To: Andrew Lunn
Cc: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Linux ARM, Linux Kernel Mailing List,
linux-omap, netdev, Bartosz Golaszewski
In-Reply-To: <20180718170352.GF12477@lunn.ch>
2018-07-18 19:03 GMT+02:00 Andrew Lunn <andrew@lunn.ch>:
>> >> +#ifdef CONFIG_MTD
>> >> + /* NOTE: this should go away as soon as MTD gets nvmem support. */
>> >> + if (!addr) {
>> >> + struct mtd_info *mtd;
>> >> + int rv;
>> >> +
>> >> + mtd = get_mtd_device_nm("MAC-Address");
>> >
>> > In order for this to go away, you need to keep backwards
>> > compatibility. When using nvmem, you look for a cell called
>> > "mac-address". Here you are looking for "MAC-Address". That is going
>> > to make backwards compatibility harder. How do you plan to do it?
>> >
>> > Andrew
>>
>> I'm trying to adjust to already existing users. The only user of
>> get_mtd_device_nm() who calls it to read the MAC address registers a
>> partition called "MAC-Address". We can't change it since it's visible
>> from user space. In the future we'd just have to have a list of
>> supported string that we'd use to do the nvmem lookup.
>
> Why not have the nvmem cell called "MAC-Address"? When you add nvmem
> support to MTD, i assume you are going to map each MTD partition to an
> nvmem cell?
Because all existing users of nvmem use "mac-address" as the name of
this cell already. I guess we will need to live with both in this
particular function.
Bart
^ permalink raw reply
* [PATCH v2 1/3] net: fortify eth_platform_get_mac_address()
From: Bartosz Golaszewski @ 2018-07-19 8:20 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180719082028.26116-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We'll soon have more sources from which to read the MAC address in this
routine. Make sure the address is correct before returning it.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
net/ethernet/eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index ee28440f57c5..39af03894598 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -541,7 +541,7 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
if (!addr)
addr = arch_get_platform_mac_address();
- if (!addr)
+ if (!addr || !is_valid_ether_addr(addr))
return -ENODEV;
ether_addr_copy(mac_addr, addr);
--
2.17.1
^ permalink raw reply related
* [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
From: Bartosz Golaszewski @ 2018-07-19 8:20 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180719082028.26116-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Many non-DT platforms read the MAC address from EEPROM. Usually it's
either done with callbacks defined in board files or from SoC-specific
ethernet drivers.
In order to generalize this, try to read the MAC from nvmem in
eth_platform_get_mac_address() using a standard lookup name:
"mac-address".
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
net/ethernet/eth.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 39af03894598..af3b4b1b77eb 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -54,6 +54,7 @@
#include <linux/if_ether.h>
#include <linux/of_net.h>
#include <linux/pci.h>
+#include <linux/nvmem-consumer.h>
#include <net/dst.h>
#include <net/arp.h>
#include <net/sock.h>
@@ -527,8 +528,11 @@ unsigned char * __weak arch_get_platform_mac_address(void)
int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
{
+ unsigned char addrbuf[ETH_ALEN];
const unsigned char *addr;
+ struct nvmem_cell *nvmem;
struct device_node *dp;
+ size_t alen;
if (dev_is_pci(dev))
dp = pci_device_to_OF_node(to_pci_dev(dev));
@@ -541,6 +545,29 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
if (!addr)
addr = arch_get_platform_mac_address();
+ if (!addr) {
+ nvmem = nvmem_cell_get(dev, "mac-address");
+ if (IS_ERR(nvmem) && PTR_ERR(nvmem) == -EPROBE_DEFER)
+ /* We may have a lookup registered for MAC address but
+ * the corresponding nvmem provider hasn't been
+ * registered yet.
+ */
+ return -EPROBE_DEFER;
+
+ if (!IS_ERR(nvmem)) {
+ addr = nvmem_cell_read(nvmem, &alen);
+ if (!IS_ERR(addr)) {
+ if (alen == ETH_ALEN)
+ ether_addr_copy(addrbuf, addr);
+
+ kfree(addr);
+ addr = alen == ETH_ALEN ? addrbuf : NULL;
+ }
+
+ nvmem_cell_put(nvmem);
+ }
+ }
+
if (!addr || !is_valid_ether_addr(addr))
return -ENODEV;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 3/3] net: add MTD support to eth_platform_get_mac_address()
From: Bartosz Golaszewski @ 2018-07-19 8:20 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180719082028.26116-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
MTD doesn't support nvmem yet. Some platforms use MTD to read the MAC
address from SPI flash. If we want this function to generalize reading
the MAC address, we need to separately try to use MTD.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
net/ethernet/eth.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index af3b4b1b77eb..addbb3375e3b 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -55,6 +55,7 @@
#include <linux/of_net.h>
#include <linux/pci.h>
#include <linux/nvmem-consumer.h>
+#include <linux/mtd/mtd.h>
#include <net/dst.h>
#include <net/arp.h>
#include <net/sock.h>
@@ -568,6 +569,23 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
}
}
+#ifdef CONFIG_MTD
+ /* NOTE: this should go away as soon as MTD gets nvmem support. */
+ if (!addr) {
+ struct mtd_info *mtd;
+ int rv;
+
+ mtd = get_mtd_device_nm("MAC-Address");
+ if (!IS_ERR(mtd)) {
+ rv = mtd_read(mtd, 0, ETH_ALEN, &alen, addrbuf);
+ if (rv == 0)
+ addr = addrbuf;
+
+ put_mtd_device(mtd);
+ }
+ }
+#endif /* CONFIG_MTD */
+
if (!addr || !is_valid_ether_addr(addr))
return -ENODEV;
--
2.17.1
^ permalink raw reply related
* Re: [RFC ipsec-next] xfrm: Remove xfrmi interface ID from flowi
From: Steffen Klassert @ 2018-07-19 7:45 UTC (permalink / raw)
To: Benedict Wong; +Cc: netdev, nharold, lorenzo
In-Reply-To: <20180717214004.102501-1-benedictwong@google.com>
On Tue, Jul 17, 2018 at 02:40:04PM -0700, Benedict Wong wrote:
> @@ -2301,6 +2322,13 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
> int reverse;
> struct flowi fl;
> int xerr_idx = -1;
> + const struct xfrm_if_cb *ifcb;
> + struct xfrm_if *xi;
> + u32 if_id = 0;
> +
> + rcu_read_lock();
> + ifcb = xfrm_if_get_cb();
> + rcu_read_unlock();
>
> reverse = dir & ~XFRM_POLICY_MASK;
> dir &= XFRM_POLICY_MASK;
> @@ -2325,10 +2353,16 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
> }
> }
>
> + if (ifcb) {
> + xi = ifcb->decode_session(skb);
> + if (xi)
> + if_id = xi->p.if_id;
> + }
The usage of the ifcb pointer should go into the
rcu_read_lock section above.
Looks good otherwise, nice improvement.
Please respin and do an official submission of this
patch, I'd like to merge it before I send the pull
request for the ipsec-next tree.
^ permalink raw reply
* Re: KASAN: stack-out-of-bounds Read in bpf_tcp_close
From: Dmitry Vyukov @ 2018-07-19 7:46 UTC (permalink / raw)
To: syzbot; +Cc: Alexei Starovoitov, Daniel Borkmann, LKML, netdev, syzkaller-bugs
In-Reply-To: <0000000000002f5daa05714d739b@google.com>
#syz fix: bpf: sockhash, disallow bpf_tcp_close and update in parallel
On Thu, Jul 19, 2018 at 12:19 AM, syzbot
<syzbot+65a97319fd875ea52b73@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 8ae71e76cf1f Merge branch 'bpf-offload-sharing'
> git tree: bpf-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=1379a978400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=89129667b46496c3
> dashboard link: https://syzkaller.appspot.com/bug?extid=65a97319fd875ea52b73
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=114f4b2c400000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1774932c400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+65a97319fd875ea52b73@syzkaller.appspotmail.com
>
> random: sshd: uninitialized urandom read (32 bytes read)
> random: sshd: uninitialized urandom read (32 bytes read)
> IPVS: ftp: loaded support on port[0] = 21
> ==================================================================
> swap_info_get: Bad swap file entry 8007fffc400d72b
> BUG: KASAN: stack-out-of-bounds in __read_once_size
> include/linux/compiler.h:188 [inline]
> BUG: KASAN: stack-out-of-bounds in smap_psock_sk kernel/bpf/sockmap.c:149
> [inline]
> BUG: KASAN: stack-out-of-bounds in bpf_tcp_close+0xf10/0x1050
> kernel/bpf/sockmap.c:316
> Read of size 8 at addr ffff8801adcc4428 by task syz-executor115/24313
> BUG: Bad page map in process syz-executor115 pte:1ffff10035cac810
> pmd:1ae564067
>
> CPU: 0 PID: 24313 Comm: syz-executor115 Not tainted 4.18.0-rc3+ #58
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>
> addr:(____ptrval____) vm_flags:00000875 anon_vma: (null)
> mapping:(____ptrval____) index:0
> Allocated by task 2294230744:
> usercopy: Kernel memory overwrite attempt detected to SLAB object
> 'task_struct(17:syz0)' (offset 6088, size 2)!
> file:syz-executor115250413 fault:ext4_filemap_fault mmap:ext4_file_mmap
> readpage:ext4_readpage
> ------------[ cut here ]------------
> Bad or missing usercopy whitelist? Kernel memory overwrite attempt detected
> to SLAB object 'task_struct(17:syz0)' (offset 4936, size 2)!
> WARNING: CPU: 0 PID: 24313 at mm/usercopy.c:81 usercopy_warn+0xf5/0x120
> mm/usercopy.c:76
> CPU: 1 PID: 4479 Comm: syz-executor115 Not tainted 4.18.0-rc3+ #58
> Kernel panic - not syncing: panic_on_warn set ...
>
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
> print_bad_pte.cold.116+0x1cd/0x22b mm/memory.c:774
> zap_pte_range mm/memory.c:1380 [inline]
> zap_pmd_range mm/memory.c:1437 [inline]
> zap_pud_range mm/memory.c:1466 [inline]
> zap_p4d_range mm/memory.c:1487 [inline]
> unmap_page_range+0x1cb9/0x2220 mm/memory.c:1508
> unmap_single_vma+0x1a0/0x310 mm/memory.c:1553
> unmap_vmas+0x120/0x1f0 mm/memory.c:1583
> exit_mmap+0x2c2/0x5b0 mm/mmap.c:3105
> __mmput kernel/fork.c:970 [inline]
> mmput+0x265/0x620 kernel/fork.c:991
> exit_mm kernel/exit.c:544 [inline]
> do_exit+0xea9/0x2750 kernel/exit.c:852
> do_group_exit+0x177/0x440 kernel/exit.c:968
> get_signal+0x88e/0x1970 kernel/signal.c:2468
> do_signal+0x9c/0x21c0 arch/x86/kernel/signal.c:816
> exit_to_usermode_loop+0x2e0/0x370 arch/x86/entry/common.c:162
> prepare_exit_to_usermode+0x342/0x3b0 arch/x86/entry/common.c:197
> retint_user+0x8/0x18
> RIP: 0033:0x4731e0
> Code: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 <00> 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> RSP: 002b:00007ffd426a9928 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: 0000000000002e7e RCX: 00000000004731e0
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00007ffd426a9930
> RBP: 0000000000002e7e R08: 0000000000000001 R09: 0000000000e2a880
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000f7e
> R13: 000000000003ab5f R14: 0000000000000000 R15: 0000000000000000
> CPU: 0 PID: 24313 Comm: syz-executor115 Not tainted 4.18.0-rc3+ #58
> swap_info_get: Bad swap file entry 403fffe200725fc
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Kernel Offset: disabled
> Rebooting in 86400 seconds..
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/syzkaller-bugs/0000000000002f5daa05714d739b%40google.com.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [net 4/8] net/mlx5e: Don't allow aRFS for encapsulated packets
From: Or Gerlitz @ 2018-07-19 7:50 UTC (permalink / raw)
To: Eran Ben Elisha
Cc: Saeed Mahameed, Eran Ben Elisha, David S. Miller,
Linux Netdev List, Alexander Duyck, Tom Herbert
In-Reply-To: <CAKHjkj=z3BzcyJ84WD7=oOcRdVZwny_+rznPU6gFPXR+7igTcQ@mail.gmail.com>
On Thu, Jul 19, 2018 at 9:55 AM, Eran Ben Elisha
<eranlinuxmellanox@gmail.com> wrote:
> On Thu, Jul 19, 2018 at 9:23 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> On Thu, Jul 19, 2018 at 4:26 AM, Saeed Mahameed <saeedm@mellanox.com> wrote:
>>> From: Eran Ben Elisha <eranbe@mellanox.com>
>>>
>>> Driver is yet to support aRFS for encapsulated packets, return early
>>> error in such case.
>>
>>
>> Eran,
>>
>> Isn't that something which is done wrong by the arfs stack code?
>>
>> If the kernel has an SKB which has encap set and an arfs steering
>> rule is programed into the driver, the API should include a driver neutral
>> description for the encap header for the HW to match, so maybe we can just do
>>
>
> Hi Or,
> This could break existing drivers support for tunneled aRFS, and hurts
> their RX performance dramatically..
> IMHO, it is expected from the driver to figure out that the skb holds
> encap packet and act accordingly.
I don't think this one bit indication on the skb is enough for
any HW driver (e.g mlx4, mlx5 and others) to properly set
the steering rules.
The problem you indicate typically doesn't come into play in the presence
of VMs, since the host TCP stack isn't active on such traffic.
This is maybe why it wasn't pointed earlier.
I believe that more drivers are broken (mlx4?)
Looking now on bnxt, I see they dissect the skb and then check the
FLOW_DIS_ENCAPSULATION flag but not always err.
If we want to make sure we don't break anyone else, we can indeed have
the check done in our drivers.
It seems that the check done by bnxt is more general, thoughts?
Or.
^ permalink raw reply
* Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
From: Dan Carpenter @ 2018-07-19 8:48 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Grygorii Strashko, David Lechner, Ivan Khoronzhuk,
Kevin Hilman, Greg Kroah-Hartman, Sekhar Nori, Russell King,
linux-kernel, Andrew Lunn, Bartosz Golaszewski, Lukas Wunner,
Srinivas Kandagatla, netdev, Florian Fainelli, linux-omap,
David S . Miller, linux-arm-kernel
In-Reply-To: <20180719082028.26116-3-brgl@bgdev.pl>
On Thu, Jul 19, 2018 at 10:20:27AM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Many non-DT platforms read the MAC address from EEPROM. Usually it's
> either done with callbacks defined in board files or from SoC-specific
> ethernet drivers.
>
> In order to generalize this, try to read the MAC from nvmem in
> eth_platform_get_mac_address() using a standard lookup name:
> "mac-address".
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> net/ethernet/eth.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
> index 39af03894598..af3b4b1b77eb 100644
> --- a/net/ethernet/eth.c
> +++ b/net/ethernet/eth.c
> @@ -54,6 +54,7 @@
> #include <linux/if_ether.h>
> #include <linux/of_net.h>
> #include <linux/pci.h>
> +#include <linux/nvmem-consumer.h>
> #include <net/dst.h>
> #include <net/arp.h>
> #include <net/sock.h>
> @@ -527,8 +528,11 @@ unsigned char * __weak arch_get_platform_mac_address(void)
>
> int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
> {
> + unsigned char addrbuf[ETH_ALEN];
> const unsigned char *addr;
> + struct nvmem_cell *nvmem;
> struct device_node *dp;
> + size_t alen;
>
> if (dev_is_pci(dev))
> dp = pci_device_to_OF_node(to_pci_dev(dev));
> @@ -541,6 +545,29 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
> if (!addr)
> addr = arch_get_platform_mac_address();
>
> + if (!addr) {
> + nvmem = nvmem_cell_get(dev, "mac-address");
> + if (IS_ERR(nvmem) && PTR_ERR(nvmem) == -EPROBE_DEFER)
> + /* We may have a lookup registered for MAC address but
> + * the corresponding nvmem provider hasn't been
> + * registered yet.
> + */
> + return -EPROBE_DEFER;
> +
> + if (!IS_ERR(nvmem)) {
> + addr = nvmem_cell_read(nvmem, &alen);
> + if (!IS_ERR(addr)) {
^^^^
Never do success handling. Always error handling. Otherwise the code
is indent a lot and the error handling is far from the call.
> + if (alen == ETH_ALEN)
> + ether_addr_copy(addrbuf, addr);
> +
> + kfree(addr);
> + addr = alen == ETH_ALEN ? addrbuf : NULL;
> + }
> +
> + nvmem_cell_put(nvmem);
> + }
> + }
> +
> if (!addr || !is_valid_ether_addr(addr))
^^^^
Instead of handling the error we dereference the error pointer here.
*frowny face*
> return -ENODEV;
>
> --
Maybe this?
if (!addr) {
nvmem = nvmem_cell_get(dev, "mac-address");
if (PTR_ERR(nvmem) == -EPROBE_DEFER)
return -EPROBE_DEFER;
if (IS_ERR(nvmem))
return -ENODEV;
addr = nvmem_cell_read(nvmem, &alen);
if (IS_ERR(addr))
return PTR_ERR(addr);
if (alen != ETH_ALEN) {
kfree(addr);
return -ENODEV;
}
ether_addr_copy(addrbuf, addr);
kfree(addr);
addr = addrbuf;
}
if (!is_valid_ether_addr(addr))
return -ENODEV;
ether_addr_copy(mac_addr, addr);
return 0;
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v4 01/18] nvmem: add support for cell lookups
From: Srinivas Kandagatla @ 2018-07-16 12:19 UTC (permalink / raw)
To: Bartosz Golaszewski, Sekhar Nori, Kevin Hilman, Russell King,
Grygorii Strashko, David S . Miller, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180629094039.7543-2-brgl@bgdev.pl>
On 29/06/18 10:40, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> We can currently only register nvmem cells from device tree or by
> manually calling nvmem_add_cells(). The latter options however forces
> users to make sure that the nvmem provider with which the cells are
> associated is registered before the call.
>
> This patch proposes a new solution inspired by other frameworks that
> offer resource lookups (GPIO, PWM etc.). It adds functions that allow
> machine code to register nvmem lookup which are later lazily used to
> add corresponding nvmem cells and remove them if no longer needed.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Acked-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
^ permalink raw reply
* Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
From: Bartosz Golaszewski @ 2018-07-19 8:57 UTC (permalink / raw)
To: Dan Carpenter
Cc: Bartosz Golaszewski, Sekhar Nori, Kevin Hilman, Russell King,
Grygorii Strashko, David S . Miller, Srinivas Kandagatla,
Lukas Wunner, Rob Herring, Florian Fainelli, Ivan Khoronzhuk,
David Lechner, Greg Kroah-Hartman, Andrew Lunn, arm-soc, LKML,
Linux-OMAP, netdev
In-Reply-To: <20180719084503.tfv6jllsukk2zv3f@mwanda>
2018-07-19 10:48 GMT+02:00 Dan Carpenter <dan.carpenter@oracle.com>:
> On Thu, Jul 19, 2018 at 10:20:27AM +0200, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> Many non-DT platforms read the MAC address from EEPROM. Usually it's
>> either done with callbacks defined in board files or from SoC-specific
>> ethernet drivers.
>>
>> In order to generalize this, try to read the MAC from nvmem in
>> eth_platform_get_mac_address() using a standard lookup name:
>> "mac-address".
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>> net/ethernet/eth.c | 27 +++++++++++++++++++++++++++
>> 1 file changed, 27 insertions(+)
>>
>> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
>> index 39af03894598..af3b4b1b77eb 100644
>> --- a/net/ethernet/eth.c
>> +++ b/net/ethernet/eth.c
>> @@ -54,6 +54,7 @@
>> #include <linux/if_ether.h>
>> #include <linux/of_net.h>
>> #include <linux/pci.h>
>> +#include <linux/nvmem-consumer.h>
>> #include <net/dst.h>
>> #include <net/arp.h>
>> #include <net/sock.h>
>> @@ -527,8 +528,11 @@ unsigned char * __weak arch_get_platform_mac_address(void)
>>
>> int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
>> {
>> + unsigned char addrbuf[ETH_ALEN];
>> const unsigned char *addr;
>> + struct nvmem_cell *nvmem;
>> struct device_node *dp;
>> + size_t alen;
>>
>> if (dev_is_pci(dev))
>> dp = pci_device_to_OF_node(to_pci_dev(dev));
>> @@ -541,6 +545,29 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
>> if (!addr)
>> addr = arch_get_platform_mac_address();
>>
>> + if (!addr) {
>> + nvmem = nvmem_cell_get(dev, "mac-address");
>> + if (IS_ERR(nvmem) && PTR_ERR(nvmem) == -EPROBE_DEFER)
>> + /* We may have a lookup registered for MAC address but
>> + * the corresponding nvmem provider hasn't been
>> + * registered yet.
>> + */
>> + return -EPROBE_DEFER;
>> +
>> + if (!IS_ERR(nvmem)) {
>> + addr = nvmem_cell_read(nvmem, &alen);
>> + if (!IS_ERR(addr)) {
> ^^^^
> Never do success handling. Always error handling. Otherwise the code
> is indent a lot and the error handling is far from the call.
>
>> + if (alen == ETH_ALEN)
>> + ether_addr_copy(addrbuf, addr);
>> +
>> + kfree(addr);
>> + addr = alen == ETH_ALEN ? addrbuf : NULL;
>> + }
>> +
>> + nvmem_cell_put(nvmem);
>> + }
>> + }
>> +
>> if (!addr || !is_valid_ether_addr(addr))
> ^^^^
> Instead of handling the error we dereference the error pointer here.
>
True - we should add a check for IS_ERR(addr) here.
> *frowny face*
>
>> return -ENODEV;
>>
>> --
>
> Maybe this?
>
> if (!addr) {
> nvmem = nvmem_cell_get(dev, "mac-address");
> if (PTR_ERR(nvmem) == -EPROBE_DEFER)
> return -EPROBE_DEFER;
> if (IS_ERR(nvmem))
> return -ENODEV;
> addr = nvmem_cell_read(nvmem, &alen);
> if (IS_ERR(addr))
> return PTR_ERR(addr);
> if (alen != ETH_ALEN) {
> kfree(addr);
> return -ENODEV;
> }
> ether_addr_copy(addrbuf, addr);
> kfree(addr);
> addr = addrbuf;
> }
> if (!is_valid_ether_addr(addr))
> return -ENODEV;
> ether_addr_copy(mac_addr, addr);
> return 0;
>
I would normally go this way but here we don't want to bail out when
we encounter an error but rather continue on to the next possible
source of a MAC address. We'll get -ENODEV from nvmem_cell_get() if
the lookup fails for "mac-address" but instead of returning an error
code we should then check if we can read the MAC from MTD.
Bart
^ permalink raw reply
* [PATCH v2 0/3] net: extend eth_platform_get_mac_address()
From: Bartosz Golaszewski @ 2018-07-19 8:20 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
This is a follow-up to a series I posted a while ago the goal of which
was to replace the at24 platform data with device properties. To do so
we need to somehow remove reading the MAC address from relevant board
files.
In my patches I used nvmem and MTD to read the MAC address from within
the davinci emac driver. It was suggested that we generalize it further
but since MTD doesn't support nvmem yet, the best we can do is to move
this code over to net core code.
The following patches modify the eth_platform_get_mac_address()
function which seems to be the best candidate for this code.
The first patch calls is_valid_ether_addr() on the read address so
that we're sure it's correct.
The last two patches add nvmem and MTD support to the function. In
order to stay compatible with existing users, nvmem and MTD will be
tried last - after device tree and arch-specific callback.
If this series gets accepted I will modify my previous patches to
use it instead of handcoding the same operations in davinci_emac.
v1 -> v2:
- dropped patches 1 & 2
- improved the MAC address verification and fixed a potential buffer
overflow in patch 2/3
Bartosz Golaszewski (3):
net: fortify eth_platform_get_mac_address()
net: add support for nvmem to eth_platform_get_mac_address()
net: add MTD support to eth_platform_get_mac_address()
net/ethernet/eth.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 46 insertions(+), 1 deletion(-)
--
2.17.1
^ permalink raw reply
* Re: [net-next 10/16] net/mlx5: Support PCIe buffer congestion handling via Devlink
From: Jiri Pirko @ 2018-07-19 8:24 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: David S. Miller, netdev, Eran Ben Elisha
In-Reply-To: <20180719010107.22363-11-saeedm@mellanox.com>
Thu, Jul 19, 2018 at 03:01:01AM CEST, saeedm@mellanox.com wrote:
>From: Eran Ben Elisha <eranbe@mellanox.com>
>
>Add support for two driver parameters via devlink params interface:
>- Congestion action
> HW mechanism in the PCIe buffer which monitors the amount of
> consumed PCIe buffer per host. This mechanism supports the
> following actions in case of threshold overflow:
> - Disabled - NOP (Default)
> - Drop
> - Mark - Mark CE bit in the CQE of received packet
>- Congestion mode
> - Aggressive - Aggressive static trigger threshold (Default)
> - Dynamic - Dynamically change the trigger threshold
>
>Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
>Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>---
> .../net/ethernet/mellanox/mlx5/core/devlink.c | 105 +++++++++++++++++-
> 1 file changed, 104 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>index 9800c98b01d3..1f04decef043 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>@@ -153,12 +153,115 @@ static int mlx5_devlink_query_tx_overflow_sense(struct mlx5_core_dev *mdev,
> return 0;
> }
>
>+static int mlx5_devlink_set_congestion_action(struct devlink *devlink, u32 id,
>+ struct devlink_param_gset_ctx *ctx)
>+{
>+ struct mlx5_core_dev *dev = devlink_priv(devlink);
>+ u8 max = MLX5_DEVLINK_CONGESTION_ACTION_MAX;
>+ u8 sense;
>+ int err;
>+
>+ if (!MLX5_CAP_MCAM_FEATURE(dev, mark_tx_action_cqe) &&
>+ !MLX5_CAP_MCAM_FEATURE(dev, mark_tx_action_cnp))
>+ max = MLX5_DEVLINK_CONGESTION_ACTION_MARK - 1;
>+
>+ if (ctx->val.vu8 > max)
This should not be num. It should be a string. Same for "mode".
>+ return -ERANGE;
>+
>+ err = mlx5_devlink_query_tx_overflow_sense(dev, &sense);
>+ if (err)
>+ return err;
>+
>+ if (ctx->val.vu8 == MLX5_DEVLINK_CONGESTION_ACTION_DISABLED &&
>+ sense != MLX5_DEVLINK_CONGESTION_MODE_AGGRESSIVE)
>+ return -EINVAL;
>+
>+ return mlx5_devlink_set_tx_lossy_overflow(dev, ctx->val.vu8);
>+}
>+
>+static int mlx5_devlink_get_congestion_action(struct devlink *devlink, u32 id,
>+ struct devlink_param_gset_ctx *ctx)
>+{
>+ struct mlx5_core_dev *dev = devlink_priv(devlink);
>+
>+ return mlx5_devlink_query_tx_lossy_overflow(dev, &ctx->val.vu8);
>+}
>+
>+static int mlx5_devlink_set_congestion_mode(struct devlink *devlink, u32 id,
>+ struct devlink_param_gset_ctx *ctx)
>+{
>+ struct mlx5_core_dev *dev = devlink_priv(devlink);
>+ u8 tx_lossy_overflow;
>+ int err;
>+
>+ if (ctx->val.vu8 > MLX5_DEVLINK_CONGESTION_MODE_MAX)
>+ return -ERANGE;
>+
>+ err = mlx5_devlink_query_tx_lossy_overflow(dev, &tx_lossy_overflow);
>+ if (err)
>+ return err;
>+
>+ if (ctx->val.vu8 != MLX5_DEVLINK_CONGESTION_MODE_AGGRESSIVE &&
>+ tx_lossy_overflow == MLX5_DEVLINK_CONGESTION_ACTION_DISABLED)
>+ return -EINVAL;
>+
>+ return mlx5_devlink_set_tx_overflow_sense(dev, ctx->val.vu8);
>+}
>+
>+static int mlx5_devlink_get_congestion_mode(struct devlink *devlink, u32 id,
>+ struct devlink_param_gset_ctx *ctx)
>+{
>+ struct mlx5_core_dev *dev = devlink_priv(devlink);
>+
>+ return mlx5_devlink_query_tx_overflow_sense(dev, &ctx->val.vu8);
>+}
>+
>+enum mlx5_devlink_param_id {
>+ MLX5_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
>+ MLX5_DEVLINK_PARAM_ID_CONGESTION_ACTION,
>+ MLX5_DEVLINK_PARAM_ID_CONGESTION_MODE,
>+};
>+
>+static const struct devlink_param mlx5_devlink_params[] = {
>+ DEVLINK_PARAM_DRIVER(MLX5_DEVLINK_PARAM_ID_CONGESTION_ACTION,
>+ "congestion_action",
>+ DEVLINK_PARAM_TYPE_U8,
>+ BIT(DEVLINK_PARAM_CMODE_RUNTIME),
>+ mlx5_devlink_get_congestion_action,
>+ mlx5_devlink_set_congestion_action, NULL),
>+ DEVLINK_PARAM_DRIVER(MLX5_DEVLINK_PARAM_ID_CONGESTION_MODE,
>+ "congestion_mode",
>+ DEVLINK_PARAM_TYPE_U8,
>+ BIT(DEVLINK_PARAM_CMODE_RUNTIME),
>+ mlx5_devlink_get_congestion_mode,
>+ mlx5_devlink_set_congestion_mode, NULL),
>+};
>+
> int mlx5_devlink_register(struct devlink *devlink, struct device *dev)
> {
>- return devlink_register(devlink, dev);
>+ int err;
>+
>+ err = devlink_register(devlink, dev);
>+ if (err)
>+ return err;
>+
>+ err = devlink_params_register(devlink, mlx5_devlink_params,
>+ ARRAY_SIZE(mlx5_devlink_params));
>+ if (err) {
>+ dev_err(dev, "devlink_params_register failed, err = %d\n", err);
>+ goto unregister;
>+ }
>+
>+ return 0;
>+
>+unregister:
>+ devlink_unregister(devlink);
>+ return err;
> }
>
> void mlx5_devlink_unregister(struct devlink *devlink)
> {
>+ devlink_params_unregister(devlink, mlx5_devlink_params,
>+ ARRAY_SIZE(mlx5_devlink_params));
> devlink_unregister(devlink);
> }
>--
>2.17.0
>
^ 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