* RE: [PATCH net] staging: fsl-dpaa2: ethsw: Add missing netdevice check
From: Ioana Ciornei @ 2019-02-23 8:45 UTC (permalink / raw)
To: Florian Fainelli, netdev@vger.kernel.org
Cc: Ioana Ciocoi Radulescu, Greg Kroah-Hartman,
open list:DPAA2 ETHERNET SWITCH DRIVER,
open list:STAGING SUBSYSTEM
In-Reply-To: <20190222220214.25401-1-f.fainelli@gmail.com>
> Subject: [PATCH net] staging: fsl-dpaa2: ethsw: Add missing netdevice check
>
> port_switchdev_event() does not check that the target network device is
> actually backed by the ethsw driver, this could be problematic in a stacked
> environment case.
>
Just FYI, I sent a patch set containing a similar patch verifying if the netdev is backed by the ethsw:
https://lkml.org/lkml/2019/2/6/216
I sent the entire patch set against the staging tree.
Ioana C
> Fixes: 44baaa43d7cc ("staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet
> Switch driver")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-
> dpaa2/ethsw/ethsw.c
> index daabaceeea52..2edd82f5229a 100644
> --- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> +++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> @@ -1047,6 +1047,9 @@ static int port_switchdev_event(struct notifier_block
> *unused,
> struct ethsw_switchdev_event_work *switchdev_work;
> struct switchdev_notifier_fdb_info *fdb_info = ptr;
>
> + if (!ethsw_port_dev_check(dev))
> + return NOTIFY_DONE;
> +
> switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC);
> if (!switchdev_work)
> return NOTIFY_BAD;
> --
> 2.17.1
^ permalink raw reply
* [PATCHv3 net-next] tls: Return type of non-data records retrieved using MSG_PEEK in recvmsg
From: Vakul Garg @ 2019-02-23 8:42 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: borisp@mellanox.com, aviadye@mellanox.com, davejwatson@fb.com,
davem@davemloft.net, doronrk@fb.com, Vakul Garg
The patch enables returning 'type' in msghdr for records that are
retrieved with MSG_PEEK in recvmsg. Further it prevents records peeked
from socket from getting clubbed with any other record of different
type when records are subsequently dequeued from strparser.
For each record, we now retain its type in sk_buff's control buffer
cb[]. Inside control buffer, record's full length and offset are already
stored by strparser in 'struct strp_msg'. We store record type after
'struct strp_msg' inside 'struct tls_msg'. For tls1.2, the type is
stored just after record dequeue. For tls1.3, the type is stored after
record has been decrypted.
Inside process_rx_list(), before processing a non-data record, we check
that we must be able to return back the record type to the user
application. If not, the decrypted records in tls context's rx_list is
left there without consuming any data.
Fixes: 692d7b5d1f912 ("tls: Fix recvmsg() to be able to peek across
multiple records")
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
Changes in v2:
- Modified 'Fixed:' line to use full commit header line.
Changes in v3:
- Added a missing ' " ' in Fixed: line.
include/net/tls.h | 10 +++++++
net/tls/tls_sw.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++--------
2 files changed, 77 insertions(+), 11 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index a8b37226a287..9f4117ae2297 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -129,6 +129,11 @@ struct tls_rec {
u8 aead_req_ctx[];
};
+struct tls_msg {
+ struct strp_msg rxm;
+ u8 control;
+};
+
struct tx_work {
struct delayed_work work;
struct sock *sk;
@@ -333,6 +338,11 @@ int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
int tls_push_pending_closed_record(struct sock *sk, struct tls_context *ctx,
int flags, long *timeo);
+static inline struct tls_msg *tls_msg(struct sk_buff *skb)
+{
+ return (struct tls_msg *)strp_msg(skb);
+}
+
static inline bool tls_is_pending_closed_record(struct tls_context *ctx)
{
return test_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 71be8acfbc9b..1cc830582fa8 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1530,22 +1530,38 @@ static bool tls_sw_advance_skb(struct sock *sk, struct sk_buff *skb,
}
/* This function traverses the rx_list in tls receive context to copies the
- * decrypted data records into the buffer provided by caller zero copy is not
+ * decrypted records into the buffer provided by caller zero copy is not
* true. Further, the records are removed from the rx_list if it is not a peek
* case and the record has been consumed completely.
*/
static int process_rx_list(struct tls_sw_context_rx *ctx,
struct msghdr *msg,
+ u8 *control,
+ bool *cmsg,
size_t skip,
size_t len,
bool zc,
bool is_peek)
{
struct sk_buff *skb = skb_peek(&ctx->rx_list);
+ u8 ctrl = *control;
+ u8 msgc = *cmsg;
+ struct tls_msg *tlm;
ssize_t copied = 0;
+ /* Set the record type in 'control' if caller didn't pass it */
+ if (!ctrl && skb) {
+ tlm = tls_msg(skb);
+ ctrl = tlm->control;
+ }
+
while (skip && skb) {
struct strp_msg *rxm = strp_msg(skb);
+ tlm = tls_msg(skb);
+
+ /* Cannot process a record of different type */
+ if (ctrl != tlm->control)
+ return 0;
if (skip < rxm->full_len)
break;
@@ -1559,6 +1575,27 @@ static int process_rx_list(struct tls_sw_context_rx *ctx,
struct strp_msg *rxm = strp_msg(skb);
int chunk = min_t(unsigned int, rxm->full_len - skip, len);
+ tlm = tls_msg(skb);
+
+ /* Cannot process a record of different type */
+ if (ctrl != tlm->control)
+ return 0;
+
+ /* Set record type if not already done. For a non-data record,
+ * do not proceed if record type could not be copied.
+ */
+ if (!msgc) {
+ int cerr = put_cmsg(msg, SOL_TLS, TLS_GET_RECORD_TYPE,
+ sizeof(ctrl), &ctrl);
+ msgc = true;
+ if (ctrl != TLS_RECORD_TYPE_DATA) {
+ if (cerr || msg->msg_flags & MSG_CTRUNC)
+ return -EIO;
+
+ *cmsg = msgc;
+ }
+ }
+
if (!zc || (rxm->full_len - skip) > len) {
int err = skb_copy_datagram_msg(skb, rxm->offset + skip,
msg, chunk);
@@ -1597,6 +1634,7 @@ static int process_rx_list(struct tls_sw_context_rx *ctx,
skb = next_skb;
}
+ *control = ctrl;
return copied;
}
@@ -1614,6 +1652,7 @@ int tls_sw_recvmsg(struct sock *sk,
unsigned char control = 0;
ssize_t decrypted = 0;
struct strp_msg *rxm;
+ struct tls_msg *tlm;
struct sk_buff *skb;
ssize_t copied = 0;
bool cmsg = false;
@@ -1632,7 +1671,8 @@ int tls_sw_recvmsg(struct sock *sk,
lock_sock(sk);
/* Process pending decrypted records. It must be non-zero-copy */
- err = process_rx_list(ctx, msg, 0, len, false, is_peek);
+ err = process_rx_list(ctx, msg, &control, &cmsg, 0, len, false,
+ is_peek);
if (err < 0) {
tls_err_abort(sk, err);
goto end;
@@ -1668,6 +1708,12 @@ int tls_sw_recvmsg(struct sock *sk,
}
}
goto recv_end;
+ } else {
+ tlm = tls_msg(skb);
+ if (prot->version == TLS_1_3_VERSION)
+ tlm->control = 0;
+ else
+ tlm->control = ctx->control;
}
rxm = strp_msg(skb);
@@ -1694,22 +1740,34 @@ int tls_sw_recvmsg(struct sock *sk,
if (err == -EINPROGRESS)
num_async++;
+ else if (prot->version == TLS_1_3_VERSION)
+ tlm->control = ctx->control;
+
+ /* If the type of records being processed is not known yet,
+ * set it to record type just dequeued. If it is already known,
+ * but does not match the record type just dequeued, go to end.
+ * We always get record type here since for tls1.2, record type
+ * is known just after record is dequeued from stream parser.
+ * For tls1.3, we disable async.
+ */
+
+ if (!control)
+ control = tlm->control;
+ else if (control != tlm->control)
+ goto recv_end;
if (!cmsg) {
int cerr;
cerr = put_cmsg(msg, SOL_TLS, TLS_GET_RECORD_TYPE,
- sizeof(ctx->control), &ctx->control);
+ sizeof(control), &control);
cmsg = true;
- control = ctx->control;
- if (ctx->control != TLS_RECORD_TYPE_DATA) {
+ if (control != TLS_RECORD_TYPE_DATA) {
if (cerr || msg->msg_flags & MSG_CTRUNC) {
err = -EIO;
goto recv_end;
}
}
- } else if (control != ctx->control) {
- goto recv_end;
}
if (async)
@@ -1784,18 +1842,16 @@ int tls_sw_recvmsg(struct sock *sk,
/* Drain records from the rx_list & copy if required */
if (is_peek || is_kvec)
- err = process_rx_list(ctx, msg, copied,
+ err = process_rx_list(ctx, msg, &control, &cmsg, copied,
decrypted, false, is_peek);
else
- err = process_rx_list(ctx, msg, 0,
+ err = process_rx_list(ctx, msg, &control, &cmsg, 0,
decrypted, true, is_peek);
if (err < 0) {
tls_err_abort(sk, err);
copied = 0;
goto end;
}
-
- WARN_ON(decrypted != err);
}
copied += decrypted;
--
2.13.6
^ permalink raw reply related
* [PATCHv2 net-next] tls: Return type of non-data records retrieved using MSG_PEEK in recvmsg
From: Vakul Garg @ 2019-02-23 8:39 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: borisp@mellanox.com, aviadye@mellanox.com, davejwatson@fb.com,
davem@davemloft.net, doronrk@fb.com, Vakul Garg
The patch enables returning 'type' in msghdr for records that are
retrieved with MSG_PEEK in recvmsg. Further it prevents records peeked
from socket from getting clubbed with any other record of different
type when records are subsequently dequeued from strparser.
For each record, we now retain its type in sk_buff's control buffer
cb[]. Inside control buffer, record's full length and offset are already
stored by strparser in 'struct strp_msg'. We store record type after
'struct strp_msg' inside 'struct tls_msg'. For tls1.2, the type is
stored just after record dequeue. For tls1.3, the type is stored after
record has been decrypted.
Inside process_rx_list(), before processing a non-data record, we check
that we must be able to return back the record type to the user
application. If not, the decrypted records in tls context's rx_list is
left there without consuming any data.
Fixes: 692d7b5d1f912 ("tls: Fix recvmsg() to be able to peek across
multiple records)
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
Changes in v2:
- Modified 'Fixed:' line to use full commit header line.
include/net/tls.h | 10 +++++++
net/tls/tls_sw.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++--------
2 files changed, 77 insertions(+), 11 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index a8b37226a287..9f4117ae2297 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -129,6 +129,11 @@ struct tls_rec {
u8 aead_req_ctx[];
};
+struct tls_msg {
+ struct strp_msg rxm;
+ u8 control;
+};
+
struct tx_work {
struct delayed_work work;
struct sock *sk;
@@ -333,6 +338,11 @@ int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
int tls_push_pending_closed_record(struct sock *sk, struct tls_context *ctx,
int flags, long *timeo);
+static inline struct tls_msg *tls_msg(struct sk_buff *skb)
+{
+ return (struct tls_msg *)strp_msg(skb);
+}
+
static inline bool tls_is_pending_closed_record(struct tls_context *ctx)
{
return test_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 71be8acfbc9b..1cc830582fa8 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1530,22 +1530,38 @@ static bool tls_sw_advance_skb(struct sock *sk, struct sk_buff *skb,
}
/* This function traverses the rx_list in tls receive context to copies the
- * decrypted data records into the buffer provided by caller zero copy is not
+ * decrypted records into the buffer provided by caller zero copy is not
* true. Further, the records are removed from the rx_list if it is not a peek
* case and the record has been consumed completely.
*/
static int process_rx_list(struct tls_sw_context_rx *ctx,
struct msghdr *msg,
+ u8 *control,
+ bool *cmsg,
size_t skip,
size_t len,
bool zc,
bool is_peek)
{
struct sk_buff *skb = skb_peek(&ctx->rx_list);
+ u8 ctrl = *control;
+ u8 msgc = *cmsg;
+ struct tls_msg *tlm;
ssize_t copied = 0;
+ /* Set the record type in 'control' if caller didn't pass it */
+ if (!ctrl && skb) {
+ tlm = tls_msg(skb);
+ ctrl = tlm->control;
+ }
+
while (skip && skb) {
struct strp_msg *rxm = strp_msg(skb);
+ tlm = tls_msg(skb);
+
+ /* Cannot process a record of different type */
+ if (ctrl != tlm->control)
+ return 0;
if (skip < rxm->full_len)
break;
@@ -1559,6 +1575,27 @@ static int process_rx_list(struct tls_sw_context_rx *ctx,
struct strp_msg *rxm = strp_msg(skb);
int chunk = min_t(unsigned int, rxm->full_len - skip, len);
+ tlm = tls_msg(skb);
+
+ /* Cannot process a record of different type */
+ if (ctrl != tlm->control)
+ return 0;
+
+ /* Set record type if not already done. For a non-data record,
+ * do not proceed if record type could not be copied.
+ */
+ if (!msgc) {
+ int cerr = put_cmsg(msg, SOL_TLS, TLS_GET_RECORD_TYPE,
+ sizeof(ctrl), &ctrl);
+ msgc = true;
+ if (ctrl != TLS_RECORD_TYPE_DATA) {
+ if (cerr || msg->msg_flags & MSG_CTRUNC)
+ return -EIO;
+
+ *cmsg = msgc;
+ }
+ }
+
if (!zc || (rxm->full_len - skip) > len) {
int err = skb_copy_datagram_msg(skb, rxm->offset + skip,
msg, chunk);
@@ -1597,6 +1634,7 @@ static int process_rx_list(struct tls_sw_context_rx *ctx,
skb = next_skb;
}
+ *control = ctrl;
return copied;
}
@@ -1614,6 +1652,7 @@ int tls_sw_recvmsg(struct sock *sk,
unsigned char control = 0;
ssize_t decrypted = 0;
struct strp_msg *rxm;
+ struct tls_msg *tlm;
struct sk_buff *skb;
ssize_t copied = 0;
bool cmsg = false;
@@ -1632,7 +1671,8 @@ int tls_sw_recvmsg(struct sock *sk,
lock_sock(sk);
/* Process pending decrypted records. It must be non-zero-copy */
- err = process_rx_list(ctx, msg, 0, len, false, is_peek);
+ err = process_rx_list(ctx, msg, &control, &cmsg, 0, len, false,
+ is_peek);
if (err < 0) {
tls_err_abort(sk, err);
goto end;
@@ -1668,6 +1708,12 @@ int tls_sw_recvmsg(struct sock *sk,
}
}
goto recv_end;
+ } else {
+ tlm = tls_msg(skb);
+ if (prot->version == TLS_1_3_VERSION)
+ tlm->control = 0;
+ else
+ tlm->control = ctx->control;
}
rxm = strp_msg(skb);
@@ -1694,22 +1740,34 @@ int tls_sw_recvmsg(struct sock *sk,
if (err == -EINPROGRESS)
num_async++;
+ else if (prot->version == TLS_1_3_VERSION)
+ tlm->control = ctx->control;
+
+ /* If the type of records being processed is not known yet,
+ * set it to record type just dequeued. If it is already known,
+ * but does not match the record type just dequeued, go to end.
+ * We always get record type here since for tls1.2, record type
+ * is known just after record is dequeued from stream parser.
+ * For tls1.3, we disable async.
+ */
+
+ if (!control)
+ control = tlm->control;
+ else if (control != tlm->control)
+ goto recv_end;
if (!cmsg) {
int cerr;
cerr = put_cmsg(msg, SOL_TLS, TLS_GET_RECORD_TYPE,
- sizeof(ctx->control), &ctx->control);
+ sizeof(control), &control);
cmsg = true;
- control = ctx->control;
- if (ctx->control != TLS_RECORD_TYPE_DATA) {
+ if (control != TLS_RECORD_TYPE_DATA) {
if (cerr || msg->msg_flags & MSG_CTRUNC) {
err = -EIO;
goto recv_end;
}
}
- } else if (control != ctx->control) {
- goto recv_end;
}
if (async)
@@ -1784,18 +1842,16 @@ int tls_sw_recvmsg(struct sock *sk,
/* Drain records from the rx_list & copy if required */
if (is_peek || is_kvec)
- err = process_rx_list(ctx, msg, copied,
+ err = process_rx_list(ctx, msg, &control, &cmsg, copied,
decrypted, false, is_peek);
else
- err = process_rx_list(ctx, msg, 0,
+ err = process_rx_list(ctx, msg, &control, &cmsg, 0,
decrypted, true, is_peek);
if (err < 0) {
tls_err_abort(sk, err);
copied = 0;
goto end;
}
-
- WARN_ON(decrypted != err);
}
copied += decrypted;
--
2.13.6
^ permalink raw reply related
* Re: [PATCH net-next 2/5] net/mlx5e: Make the log friendly when decapsulation offload not supported
From: Tonghao Zhang @ 2019-02-23 7:58 UTC (permalink / raw)
To: Or Gerlitz; +Cc: Saeed Mahameed, Linux Netdev List
In-Reply-To: <CAJ3xEMijFiNPZZjtKVvRVbUUFXqaZPP=K=cbQvdKcRNowGODwA@mail.gmail.com>
On Fri, Feb 22, 2019 at 5:07 PM Or Gerlitz <gerlitz.or@gmail.com> wrote:
>
> On Fri, Feb 22, 2019 at 9:49 AM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
> >
> > On Fri, Feb 22, 2019 at 12:32 AM Or Gerlitz <gerlitz.or@gmail.com> wrote:
> > >
> > > On Thu, Feb 21, 2019 at 3:42 PM <xiangxia.m.yue@gmail.com> wrote:
> > > >
> > > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > > >
> > > > If we try to offload decapsulation actions to VFs hw, we get the log [1].
> > >
> > > but the switching was on the tunnel type (if (tunnel_type == [...]) -
> > Yes, but we try to offload tc flow to VF device. For example
> > the p2p1_0 is VF, but not rep
>
> so this should go to the nic and not esw tc offload code path in en_tc.c
nic and esw will call parse_cls_flower() to parse the flower flows,
and more information is shown as below.
> and the nic path (look for parse_tc_nic_actions or a like) doesn't have
> a case for the tunnel key action - you should not have got to this code
> at all, please look deeper to realize what is going on there, maybe p2p1_0
> is a rep? what does ip -d link show gives on it?
# ip -d link
14: eth0_p1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
UP mode DEFAULT group default qlen 1000
link/ether 98:03:9b:06:d9:09 brd ff:ff:ff:ff:ff:ff promiscuity 0
minmtu 68 maxmtu 9978 addrgenmode none numtxqueues 128 numrxqueues 16
gso_max_size 65536 gso_max_segs 65535 portname p1 switchid
98039b06d909
vf 0 MAC 00:11:22:33:44:00, spoof checking off, link-state auto,
trust off, query_rss off
vf 1 MAC 00:11:22:33:44:01, spoof checking off, link-state auto,
trust off, query_rss off
15: eth0_pf1vf0: <BROADCAST,MULTICAST,PROMISC,UP,LOWER_UP> mtu 1500
qdisc mq state UP mode DEFAULT group default qlen 1000
link/ether 8a:26:c0:33:17:2c brd ff:ff:ff:ff:ff:ff promiscuity 1
minmtu 68 maxmtu 9978 addrgenmode none numtxqueues 16 numrxqueues 16
gso_max_size 65536 gso_max_segs 65535 portname pf1vf0 switchid
98039b06d909
16: eth0_pf1vf1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq
state UP mode DEFAULT group default qlen 1000
link/ether 96:34:55:5b:42:80 brd ff:ff:ff:ff:ff:ff promiscuity 0
minmtu 68 maxmtu 9978 addrgenmode none numtxqueues 16 numrxqueues 16
gso_max_size 65536 gso_max_segs 65535 portname pf1vf1 switchid
98039b06d909
17: p2p1_0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
UP mode DEFAULT group default qlen 1000
link/ether 00:11:22:33:44:00 brd ff:ff:ff:ff:ff:ff promiscuity 0
minmtu 68 maxmtu 9978 addrgenmode none numtxqueues 64 numrxqueues 8
gso_max_size 65536 gso_max_segs 65535
18: p2p1_1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
UP mode DEFAULT group default qlen 1000
link/ether 00:11:22:33:44:01 brd ff:ff:ff:ff:ff:ff promiscuity 0
minmtu 68 maxmtu 9978 addrgenmode none numtxqueues 64 numrxqueues 8
gso_max_size 65536 gso_max_segs 65535
# ethtool -i p2p1_0
driver: mlx5_core
version: 5.0-0
firmware-version: 14.24.1000 (MT_2420110034)
bus-info: 0000:05:01.2
supports-statistics: yes
supports-test: yes
supports-eeprom-access: no
supports-register-dump: no
supports-priv-flags: yes
# ethtool -i eth0_pf1vf0
driver: mlx5e_rep
version: 5.0.0-rc7+
firmware-version:
bus-info:
supports-statistics: yes
supports-test: no
supports-eeprom-access: no
supports-register-dump: no
supports-priv-flags: no
the call trace:
[ 586.669600] dump_stack+0x5a/0x73
[ 586.669651] mlx5e_tc_tun_parse+0x2c6/0x3a0 [mlx5_core]
[ 586.669682] __parse_cls_flower.constprop.48+0x1b2/0xca0 [mlx5_core]
[ 586.669746] parse_cls_flower+0x5d/0x110 [mlx5_core]
[ 586.669771] mlx5e_configure_flower+0x40a/0x760 [mlx5_core]
[ 586.669779] tc_setup_cb_call+0x55/0x80
[ 586.669787] fl_change+0x12a1/0x16b8 [cls_flower]
[ 586.669797] tc_new_tfilter+0x570/0x890
[ 586.669808] rtnetlink_rcv_msg+0xed/0x320
[ 586.669817] netlink_rcv_skb+0xcb/0x100
[ 586.669822] netlink_unicast+0x17f/0x230
[ 586.669827] netlink_sendmsg+0x2d2/0x3d0
^ permalink raw reply
* Re: [PATCH v2 bpf-next 4/9] bpf: add bpf helper bpf_skb_ecn_set_ce
From: Martin Lau @ 2019-02-23 7:30 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Lawrence Brakmo, netdev, Alexei Starovoitov, Eric Dumazet,
Kernel Team
In-Reply-To: <fdcd5fbb-0f51-18f9-a13a-52e8009d082e@iogearbox.net>
On Sat, Feb 23, 2019 at 02:14:26AM +0100, Daniel Borkmann wrote:
> On 02/23/2019 02:06 AM, brakmo wrote:
> > This patch adds a new bpf helper BPF_FUNC_skb_ecn_set_ce
> > "int bpf_skb_ecn_set_ce(struct sk_buff *skb)". It is added to
> > BPF_PROG_TYPE_CGROUP_SKB typed bpf_prog which currently can
> > be attached to the ingress and egress path. The helper is needed
> > because his type of bpf_prog cannot modify the skb directly.
> >
> > This helper is used to set the ECN field of ECN capable IP packets to ce
> > (congestion encountered) in the IPv6 or IPv4 header of the skb. It can be
> > used by a bpf_prog to manage egress or ingress network bandwdith limit
> > per cgroupv2 by inducing an ECN response in the TCP sender.
> > This works best when using DCTCP.
> >
> > Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> > ---
> > include/uapi/linux/bpf.h | 10 +++++++++-
> > net/core/filter.c | 14 ++++++++++++++
> > 2 files changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 95b5058fa945..fc646f3eaf9b 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -2365,6 +2365,13 @@ union bpf_attr {
> > * Make a tcp_sock enter CWR state.
> > * Return
> > * 0 on success, or a negative error in case of failure.
> > + *
> > + * int bpf_skb_ecn_set_ce(struct sk_buf *skb)
> > + * Description
> > + * Sets ECN of IP header to ce (congestion encountered) if
> > + * current value is ect (ECN capable). Works with IPv6 and IPv4.
> > + * Return
> > + * 1 if set, 0 if not set.
> > */
> > #define __BPF_FUNC_MAPPER(FN) \
> > FN(unspec), \
> > @@ -2464,7 +2471,8 @@ union bpf_attr {
> > FN(spin_unlock), \
> > FN(sk_fullsock), \
> > FN(tcp_sock), \
> > - FN(tcp_enter_cwr),
> > + FN(tcp_enter_cwr), \
> > + FN(skb_ecn_set_ce),
> >
> > /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> > * function eBPF program intends to call
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index ca57ef25279c..955369c6ed30 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -5444,6 +5444,18 @@ static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = {
> > .ret_type = RET_INTEGER,
> > .arg1_type = ARG_PTR_TO_TCP_SOCK,
> > };
> > +
> > +BPF_CALL_1(bpf_skb_ecn_set_ce, struct sk_buff *, skb)
> > +{
> > + return INET_ECN_set_ce(skb);
>
> Hm, but as mentioned last time, don't we have to ensure here that skb
> is writable (aka skb->data private to us before writing into it)?
INET_ECN_set_ce(skb) is also called from a few net/sched/sch_*.c
but I don't see how they ensure if a skb is writable.
May be I have missed something there that can also be borrowed and
reused here?
Thanks,
Martin
>
> > +}
> > +
> > +static const struct bpf_func_proto bpf_skb_ecn_set_ce_proto = {
> > + .func = bpf_skb_ecn_set_ce,
> > + .gpl_only = false,
> > + .ret_type = RET_INTEGER,
> > + .arg1_type = ARG_PTR_TO_CTX,
> > +};
> > #endif /* CONFIG_INET */
> >
> > bool bpf_helper_changes_pkt_data(void *func)
> > @@ -5610,6 +5622,8 @@ cg_skb_func_proto(enum bpf_func_id func_id, struct bpf_prog *prog)
> > } else {
> > return NULL;
> > }
> > + case BPF_FUNC_skb_ecn_set_ce:
> > + return &bpf_skb_ecn_set_ce_proto;
> > #endif
> > default:
> > return sk_filter_func_proto(func_id, prog);
> >
>
> Thanks,
> Daniel
^ permalink raw reply
* [PATCH net-next v2 0/3] ipv4/v6: icmp: small cleanup and update
From: Kefeng Wang @ 2019-02-23 7:28 UTC (permalink / raw)
To: davem, netdev, eric.dumazet; +Cc: Kefeng Wang
v2:
- Add cover letter and user proper patch subject-prefix suggested-by Eric Dumazet
This patch series contains some small cleanup and update,
1) use icmp/v6_sk_exit when icmp_sk_init fails instead of open-code
2) use new percpu allocation interface for the ipv6.icmp_sk
Kefeng Wang (3):
ipv4: icmp: use icmp_sk_exit()
ipv6: icmp: use icmpv6_sk_exit()
ipv6: icmp: use percpu allocation
include/net/netns/ipv6.h | 2 +-
net/ipv4/icmp.c | 4 +---
net/ipv6/icmp.c | 32 ++++++++++++++------------------
3 files changed, 16 insertions(+), 22 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH net-next v2 3/3] ipv6: icmp: use percpu allocation
From: Kefeng Wang @ 2019-02-23 7:28 UTC (permalink / raw)
To: davem, netdev, eric.dumazet; +Cc: Kefeng Wang
In-Reply-To: <20190223072828.159975-1-wangkefeng.wang@huawei.com>
Use percpu allocation for the ipv6.icmp_sk.
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
include/net/netns/ipv6.h | 2 +-
net/ipv6/icmp.c | 11 +++++------
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index ef1ed529f33c..b028a1dc150d 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -83,7 +83,7 @@ struct netns_ipv6 {
struct fib6_table *fib6_local_tbl;
struct fib_rules_ops *fib6_rules_ops;
#endif
- struct sock **icmp_sk;
+ struct sock * __percpu *icmp_sk;
struct sock *ndisc_sk;
struct sock *tcp_sk;
struct sock *igmp_sk;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index af520014def5..802faa2fcc0e 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -81,7 +81,7 @@
*/
static inline struct sock *icmpv6_sk(struct net *net)
{
- return net->ipv6.icmp_sk[smp_processor_id()];
+ return *this_cpu_ptr(net->ipv6.icmp_sk);
}
static int icmpv6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
@@ -958,8 +958,8 @@ static void __net_exit icmpv6_sk_exit(struct net *net)
int i;
for_each_possible_cpu(i)
- inet_ctl_sock_destroy(net->ipv6.icmp_sk[i]);
- kfree(net->ipv6.icmp_sk);
+ inet_ctl_sock_destroy(*per_cpu_ptr(net->ipv6.icmp_sk, i));
+ free_percpu(net->ipv6.icmp_sk);
}
static int __net_init icmpv6_sk_init(struct net *net)
@@ -967,8 +967,7 @@ static int __net_init icmpv6_sk_init(struct net *net)
struct sock *sk;
int err, i;
- net->ipv6.icmp_sk =
- kcalloc(nr_cpu_ids, sizeof(struct sock *), GFP_KERNEL);
+ net->ipv6.icmp_sk = alloc_percpu(struct sock *);
if (!net->ipv6.icmp_sk)
return -ENOMEM;
@@ -981,7 +980,7 @@ static int __net_init icmpv6_sk_init(struct net *net)
goto fail;
}
- net->ipv6.icmp_sk[i] = sk;
+ *per_cpu_ptr(net->ipv6.icmp_sk, i) = sk;
/* Enough space for 2 64K ICMP packets, including
* sk_buff struct overhead.
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v2 2/3] ipv6: icmp: use icmpv6_sk_exit()
From: Kefeng Wang @ 2019-02-23 7:28 UTC (permalink / raw)
To: davem, netdev, eric.dumazet; +Cc: Kefeng Wang
In-Reply-To: <20190223072828.159975-1-wangkefeng.wang@huawei.com>
Simply use icmpv6_sk_exit() when inet_ctl_sock_create() fail
in icmpv6_sk_init().
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
net/ipv6/icmp.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index bbcdfd299692..af520014def5 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -953,10 +953,19 @@ void icmpv6_flow_init(struct sock *sk, struct flowi6 *fl6,
security_sk_classify_flow(sk, flowi6_to_flowi(fl6));
}
+static void __net_exit icmpv6_sk_exit(struct net *net)
+{
+ int i;
+
+ for_each_possible_cpu(i)
+ inet_ctl_sock_destroy(net->ipv6.icmp_sk[i]);
+ kfree(net->ipv6.icmp_sk);
+}
+
static int __net_init icmpv6_sk_init(struct net *net)
{
struct sock *sk;
- int err, i, j;
+ int err, i;
net->ipv6.icmp_sk =
kcalloc(nr_cpu_ids, sizeof(struct sock *), GFP_KERNEL);
@@ -982,22 +991,10 @@ static int __net_init icmpv6_sk_init(struct net *net)
return 0;
fail:
- for (j = 0; j < i; j++)
- inet_ctl_sock_destroy(net->ipv6.icmp_sk[j]);
- kfree(net->ipv6.icmp_sk);
+ icmpv6_sk_exit(net);
return err;
}
-static void __net_exit icmpv6_sk_exit(struct net *net)
-{
- int i;
-
- for_each_possible_cpu(i) {
- inet_ctl_sock_destroy(net->ipv6.icmp_sk[i]);
- }
- kfree(net->ipv6.icmp_sk);
-}
-
static struct pernet_operations icmpv6_sk_ops = {
.init = icmpv6_sk_init,
.exit = icmpv6_sk_exit,
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v2 1/3] ipv4: icmp: use icmp_sk_exit()
From: Kefeng Wang @ 2019-02-23 7:28 UTC (permalink / raw)
To: davem, netdev, eric.dumazet; +Cc: Kefeng Wang
In-Reply-To: <20190223072828.159975-1-wangkefeng.wang@huawei.com>
Simply use icmp_sk_exit() when inet_ctl_sock_create() fail in icmp_sk_init().
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
net/ipv4/icmp.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 065997f414e6..364cfe5e414b 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -1245,9 +1245,7 @@ static int __net_init icmp_sk_init(struct net *net)
return 0;
fail:
- for_each_possible_cpu(i)
- inet_ctl_sock_destroy(*per_cpu_ptr(net->ipv4.icmp_sk, i));
- free_percpu(net->ipv4.icmp_sk);
+ icmp_sk_exit(net);
return err;
}
--
2.20.1
^ permalink raw reply related
* [PATCH] tools: testing: selftests: Remove duplicate headers
From: Souptick Joarder @ 2019-02-23 7:09 UTC (permalink / raw)
To: bamv2005, shuah, davem, benh, paulus, mpe, adobriyan,
mathieu.desnoyers, peterz, paulmck, boqun.feng, john.stultz, tglx,
sboyd, akpm
Cc: linux-gpio, linux-kselftest, linux-kernel, netdev, linuxppc-dev,
linux-fsdevel, sabyasachi.linux
Remove duplicate headers which are included twice.
Signed-off-by: Sabyasachi Gupta <sabyasachi.linux@gmail.com>
Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
---
tools/testing/selftests/gpio/gpio-mockup-chardev.c | 1 -
tools/testing/selftests/net/udpgso.c | 1 -
tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c | 1 -
tools/testing/selftests/proc/proc-self-syscall.c | 1 -
tools/testing/selftests/rseq/rseq.h | 1 -
tools/testing/selftests/timers/skew_consistency.c | 1 -
tools/testing/selftests/x86/mpx-dig.c | 2 --
7 files changed, 8 deletions(-)
diff --git a/tools/testing/selftests/gpio/gpio-mockup-chardev.c b/tools/testing/selftests/gpio/gpio-mockup-chardev.c
index aaa1e9f..d587c81 100644
--- a/tools/testing/selftests/gpio/gpio-mockup-chardev.c
+++ b/tools/testing/selftests/gpio/gpio-mockup-chardev.c
@@ -12,7 +12,6 @@
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdio.h>
#include <errno.h>
#include <string.h>
#include <fcntl.h>
diff --git a/tools/testing/selftests/net/udpgso.c b/tools/testing/selftests/net/udpgso.c
index e279051..b8265ee 100644
--- a/tools/testing/selftests/net/udpgso.c
+++ b/tools/testing/selftests/net/udpgso.c
@@ -17,7 +17,6 @@
#include <stdbool.h>
#include <stdlib.h>
#include <stdio.h>
-#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c b/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
index 167135b..af1b802 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
@@ -11,7 +11,6 @@
#include <sys/wait.h>
#include <unistd.h>
#include <setjmp.h>
-#include <signal.h>
#include "ebb.h"
diff --git a/tools/testing/selftests/proc/proc-self-syscall.c b/tools/testing/selftests/proc/proc-self-syscall.c
index 5ab5f48..3a4fec3 100644
--- a/tools/testing/selftests/proc/proc-self-syscall.c
+++ b/tools/testing/selftests/proc/proc-self-syscall.c
@@ -20,7 +20,6 @@
#include <sys/stat.h>
#include <fcntl.h>
#include <errno.h>
-#include <unistd.h>
#include <string.h>
#include <stdio.h>
diff --git a/tools/testing/selftests/rseq/rseq.h b/tools/testing/selftests/rseq/rseq.h
index c72eb70..6c1126e7 100644
--- a/tools/testing/selftests/rseq/rseq.h
+++ b/tools/testing/selftests/rseq/rseq.h
@@ -16,7 +16,6 @@
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
-#include <sched.h>
#include <linux/rseq.h>
/*
diff --git a/tools/testing/selftests/timers/skew_consistency.c b/tools/testing/selftests/timers/skew_consistency.c
index 022b711..8066be9 100644
--- a/tools/testing/selftests/timers/skew_consistency.c
+++ b/tools/testing/selftests/timers/skew_consistency.c
@@ -32,7 +32,6 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
-#include <stdlib.h>
#include <string.h>
#include <sys/wait.h>
#include "../kselftest.h"
diff --git a/tools/testing/selftests/x86/mpx-dig.c b/tools/testing/selftests/x86/mpx-dig.c
index c13607e..880fbf6 100644
--- a/tools/testing/selftests/x86/mpx-dig.c
+++ b/tools/testing/selftests/x86/mpx-dig.c
@@ -8,9 +8,7 @@
#include <unistd.h>
#include <stdio.h>
#include <errno.h>
-#include <sys/types.h>
#include <sys/stat.h>
-#include <unistd.h>
#include <sys/mman.h>
#include <string.h>
#include <fcntl.h>
--
1.9.1
^ permalink raw reply related
* [PATCH v2 3/6] sched/cpufreq: Annotate cpufreq_update_util_data pointer with __rcu
From: Joel Fernandes (Google) @ 2019-02-23 6:34 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, kernel-hardening,
kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>
Recently I added an RCU annotation check to rcu_assign_pointer(). All
pointers assigned to RCU protected data are to be annotated with __rcu
inorder to be able to use rcu_assign_pointer() similar to checks in
other RCU APIs.
This resulted in a sparse error: kernel//sched/cpufreq.c:41:9: sparse:
error: incompatible types in comparison expression (different address
spaces)
Fix this by annotating cpufreq_update_util_data pointer with __rcu. This
will also help sparse catch any future RCU misuage bugs.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
kernel/sched/cpufreq.c | 2 +-
kernel/sched/sched.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/cpufreq.c b/kernel/sched/cpufreq.c
index 22bd8980f32f..e316ee7bb2e5 100644
--- a/kernel/sched/cpufreq.c
+++ b/kernel/sched/cpufreq.c
@@ -7,7 +7,7 @@
*/
#include "sched.h"
-DEFINE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
+DEFINE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
/**
* cpufreq_add_update_util_hook - Populate the CPU's update_util_data pointer.
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index d04530bf251f..2ab545d40381 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2166,7 +2166,7 @@ static inline u64 irq_time_read(int cpu)
#endif /* CONFIG_IRQ_TIME_ACCOUNTING */
#ifdef CONFIG_CPU_FREQ
-DECLARE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
+DECLARE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
/**
* cpufreq_update_util - Take a note about CPU utilization changes.
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH v2 4/6] sched_domain: Annotate RCU pointers properly
From: Joel Fernandes (Google) @ 2019-02-23 6:34 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, kernel-hardening,
kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>
The scheduler uses RCU API in various places to access sched_domain
pointers. These cause sparse errors as below.
Many new errors show up because of an annotation check I added to
rcu_assign_pointer(). Let us annotate the pointers correctly which also
will help sparse catch any potential future bugs.
This fixes the following sparse errors:
rt.c:1681:9: error: incompatible types in comparison expression
deadline.c:1904:9: error: incompatible types in comparison expression
core.c:519:9: error: incompatible types in comparison expression
core.c:1634:17: error: incompatible types in comparison expression
fair.c:6193:14: error: incompatible types in comparison expression
fair.c:9883:22: error: incompatible types in comparison expression
fair.c:9897:9: error: incompatible types in comparison expression
sched.h:1287:9: error: incompatible types in comparison expression
topology.c:612:9: error: incompatible types in comparison expression
topology.c:615:9: error: incompatible types in comparison expression
sched.h:1300:9: error: incompatible types in comparison expression
topology.c:618:9: error: incompatible types in comparison expression
sched.h:1287:9: error: incompatible types in comparison expression
topology.c:621:9: error: incompatible types in comparison expression
sched.h:1300:9: error: incompatible types in comparison expression
topology.c:624:9: error: incompatible types in comparison expression
topology.c:671:9: error: incompatible types in comparison expression
stats.c:45:17: error: incompatible types in comparison expression
fair.c:5998:15: error: incompatible types in comparison expression
fair.c:5989:15: error: incompatible types in comparison expression
fair.c:5998:15: error: incompatible types in comparison expression
fair.c:5989:15: error: incompatible types in comparison expression
fair.c:6120:19: error: incompatible types in comparison expression
fair.c:6506:14: error: incompatible types in comparison expression
fair.c:6515:14: error: incompatible types in comparison expression
fair.c:6623:9: error: incompatible types in comparison expression
fair.c:5970:17: error: incompatible types in comparison expression
fair.c:8642:21: error: incompatible types in comparison expression
fair.c:9253:9: error: incompatible types in comparison expression
fair.c:9331:9: error: incompatible types in comparison expression
fair.c:9519:15: error: incompatible types in comparison expression
fair.c:9533:14: error: incompatible types in comparison expression
fair.c:9542:14: error: incompatible types in comparison expression
fair.c:9567:14: error: incompatible types in comparison expression
fair.c:9597:14: error: incompatible types in comparison expression
fair.c:9421:16: error: incompatible types in comparison expression
fair.c:9421:16: error: incompatible types in comparison expression
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
include/linux/sched/topology.h | 4 ++--
kernel/sched/sched.h | 14 +++++++-------
kernel/sched/topology.c | 10 +++++-----
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
index c31d3a47a47c..4819c9e01e42 100644
--- a/include/linux/sched/topology.h
+++ b/include/linux/sched/topology.h
@@ -76,8 +76,8 @@ struct sched_domain_shared {
struct sched_domain {
/* These fields must be setup */
- struct sched_domain *parent; /* top domain must be null terminated */
- struct sched_domain *child; /* bottom domain must be null terminated */
+ struct sched_domain __rcu *parent; /* top domain must be null terminated */
+ struct sched_domain __rcu *child; /* bottom domain must be null terminated */
struct sched_group *groups; /* the balancing groups of the domain */
unsigned long min_interval; /* Minimum balance interval ms */
unsigned long max_interval; /* Maximum balance interval ms */
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 2ab545d40381..ca6a79f57e7a 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -866,8 +866,8 @@ struct rq {
atomic_t nr_iowait;
#ifdef CONFIG_SMP
- struct root_domain *rd;
- struct sched_domain *sd;
+ struct root_domain *rd;
+ struct sched_domain __rcu *sd;
unsigned long cpu_capacity;
unsigned long cpu_capacity_orig;
@@ -1305,13 +1305,13 @@ static inline struct sched_domain *lowest_flag_domain(int cpu, int flag)
return sd;
}
-DECLARE_PER_CPU(struct sched_domain *, sd_llc);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_llc);
DECLARE_PER_CPU(int, sd_llc_size);
DECLARE_PER_CPU(int, sd_llc_id);
-DECLARE_PER_CPU(struct sched_domain_shared *, sd_llc_shared);
-DECLARE_PER_CPU(struct sched_domain *, sd_numa);
-DECLARE_PER_CPU(struct sched_domain *, sd_asym_packing);
-DECLARE_PER_CPU(struct sched_domain *, sd_asym_cpucapacity);
+DECLARE_PER_CPU(struct sched_domain_shared __rcu *, sd_llc_shared);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_numa);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
extern struct static_key_false sched_asym_cpucapacity;
struct sched_group_capacity {
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 3f35ba1d8fde..0844ee757dad 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -586,13 +586,13 @@ static void destroy_sched_domains(struct sched_domain *sd)
* the cpumask of the domain), this allows us to quickly tell if
* two CPUs are in the same cache domain, see cpus_share_cache().
*/
-DEFINE_PER_CPU(struct sched_domain *, sd_llc);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_llc);
DEFINE_PER_CPU(int, sd_llc_size);
DEFINE_PER_CPU(int, sd_llc_id);
-DEFINE_PER_CPU(struct sched_domain_shared *, sd_llc_shared);
-DEFINE_PER_CPU(struct sched_domain *, sd_numa);
-DEFINE_PER_CPU(struct sched_domain *, sd_asym_packing);
-DEFINE_PER_CPU(struct sched_domain *, sd_asym_cpucapacity);
+DEFINE_PER_CPU(struct sched_domain_shared __rcu *, sd_llc_shared);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_numa);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
DEFINE_STATIC_KEY_FALSE(sched_asym_cpucapacity);
static void update_top_cache_domain(int cpu)
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH v2 5/6] rcuwait: Annotate task_struct with __rcu
From: Joel Fernandes (Google) @ 2019-02-23 6:34 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, kernel-hardening,
kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>
This suppresses sparse error generated due to the recently added
rcu_assign_pointer sparse check.
percpu-rwsem.c:162:9: sparse: error: incompatible types in comparison expression
exit.c:316:16: sparse: error: incompatible types in comparison expression
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
include/linux/rcuwait.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/rcuwait.h b/include/linux/rcuwait.h
index 90bfa3279a01..563290fc194f 100644
--- a/include/linux/rcuwait.h
+++ b/include/linux/rcuwait.h
@@ -18,7 +18,7 @@
* awoken.
*/
struct rcuwait {
- struct task_struct *task;
+ struct task_struct __rcu *task;
};
#define __RCUWAIT_INITIALIZER(name) \
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH v2 6/6] sched: Annotate perf_domain pointer with __rcu
From: Joel Fernandes (Google) @ 2019-02-23 6:34 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, kernel-hardening,
kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>
This fixes the following sparse errors in sched/fair.c:
fair.c:6506:14: error: incompatible types in comparison expression
fair.c:8642:21: error: incompatible types in comparison expression
Using __rcu will also help sparse catch any future bugs.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
kernel/sched/sched.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index ca6a79f57e7a..c8e6514433a9 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -780,7 +780,7 @@ struct root_domain {
* NULL-terminated list of performance domains intersecting with the
* CPUs of the rd. Protected by RCU.
*/
- struct perf_domain *pd;
+ struct perf_domain __rcu *pd;
};
extern struct root_domain def_root_domain;
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH v2 2/6] ixgbe: Fix incorrect RCU API usage
From: Joel Fernandes (Google) @ 2019-02-23 6:34 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, kernel-hardening,
kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>
Recently, I added an RCU annotation check in rcu_assign_pointer. This
caused a sparse error to be reported by the ixgbe driver.
Further looking, it seems the adapter->xdp_prog pointer is not annotated
with __rcu. Annonating it fixed the error, but caused a bunch of other
warnings.
This patch tries to fix all warnings by using RCU API properly. This
makes sense to do because not using RCU properly can result in various
hard to find bugs. This is a best effort fix and is only build tested.
The sparse errors and warnings go away with the change. I request
maintainers / developers in this area to review / test it properly.
The sparse error fixed is:
ixgbe_main.c:10256:25: error: incompatible types in comparison expression
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 15 ++++++++++-----
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 08d85e336bd4..3b14daf27516 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -311,7 +311,7 @@ struct ixgbe_ring {
struct ixgbe_ring *next; /* pointer to next ring in q_vector */
struct ixgbe_q_vector *q_vector; /* backpointer to host q_vector */
struct net_device *netdev; /* netdev ring belongs to */
- struct bpf_prog *xdp_prog;
+ struct bpf_prog __rcu *xdp_prog;
struct device *dev; /* device for DMA mapping */
void *desc; /* descriptor ring memory */
union {
@@ -560,7 +560,7 @@ struct ixgbe_adapter {
unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
/* OS defined structs */
struct net_device *netdev;
- struct bpf_prog *xdp_prog;
+ struct bpf_prog __rcu *xdp_prog;
struct pci_dev *pdev;
struct mii_bus *mii_bus;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index daff8183534b..408a312aa6ba 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2199,7 +2199,7 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter,
u32 act;
rcu_read_lock();
- xdp_prog = READ_ONCE(rx_ring->xdp_prog);
+ xdp_prog = rcu_dereference(rx_ring->xdp_prog);
if (!xdp_prog)
goto xdp_out;
@@ -6547,7 +6547,7 @@ int ixgbe_setup_rx_resources(struct ixgbe_adapter *adapter,
rx_ring->queue_index) < 0)
goto err;
- rx_ring->xdp_prog = adapter->xdp_prog;
+ rcu_assign_pointer(rx_ring->xdp_prog, adapter->xdp_prog);
return 0;
err:
@@ -10246,7 +10246,8 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
if (nr_cpu_ids > MAX_XDP_QUEUES)
return -ENOMEM;
- old_prog = xchg(&adapter->xdp_prog, prog);
+ old_prog = rcu_access_pointer(adapter->xdp_prog);
+ rcu_assign_pointer(adapter->xdp_prog, prog);
/* If transitioning XDP modes reconfigure rings */
if (!!prog != !!old_prog) {
@@ -10271,13 +10272,17 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
static int ixgbe_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
struct ixgbe_adapter *adapter = netdev_priv(dev);
+ struct bpf_prog *prog;
switch (xdp->command) {
case XDP_SETUP_PROG:
return ixgbe_xdp_setup(dev, xdp->prog);
case XDP_QUERY_PROG:
- xdp->prog_id = adapter->xdp_prog ?
- adapter->xdp_prog->aux->id : 0;
+ rcu_read_lock();
+ prog = rcu_dereference(adapter->xdp_prog);
+ xdp->prog_id = prog ? prog->aux->id : 0;
+ rcu_read_unlock();
+
return 0;
case XDP_QUERY_XSK_UMEM:
return ixgbe_xsk_umem_query(adapter, &xdp->xsk.umem,
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH v2 1/6] net: rtnetlink: Fix incorrect RCU API usage
From: Joel Fernandes (Google) @ 2019-02-23 6:34 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, kernel-hardening,
kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>
rtnl_register_internal() and rtnl_unregister_all tries to directly
dereference an RCU protected pointed outside RCU read side section.
While this is Ok to do since a lock is held, let us use the correct
API to avoid programmer bugs in the future.
This also fixes sparse warnings arising from not using RCU API.
net/core/rtnetlink.c:332:13: warning: incorrect type in assignment
(different address spaces) net/core/rtnetlink.c:332:13: expected
struct rtnl_link **tab net/core/rtnetlink.c:332:13: got struct
rtnl_link *[noderef] <asn:4>*<noident>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
net/core/rtnetlink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 5ea1bed08ede..98be4b4818a9 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -188,7 +188,7 @@ static int rtnl_register_internal(struct module *owner,
msgindex = rtm_msgindex(msgtype);
rtnl_lock();
- tab = rtnl_msg_handlers[protocol];
+ tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
if (tab == NULL) {
tab = kcalloc(RTM_NR_MSGTYPES, sizeof(void *), GFP_KERNEL);
if (!tab)
@@ -329,7 +329,7 @@ void rtnl_unregister_all(int protocol)
BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
rtnl_lock();
- tab = rtnl_msg_handlers[protocol];
+ tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
if (!tab) {
rtnl_unlock();
return;
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH v2 0/6] RCU fixes for rcu_assign_pointer() usage
From: Joel Fernandes (Google) @ 2019-02-23 6:34 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, kernel-hardening,
kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
xdp-newbies, Yonghong Song
These patches fix various sparse errors found as a result of the recent check
to add rcu_check_sparse() to rcu_assign_pointer(). The errors in some cases
seem to either missing API usage, or missing annotations. The annotations added
in the series can also help avoid future incorrect usages and bugs so it is a
good idea to do in any case.
RFC v1 -> Patch v2:
Made changes to various scheduler patches (Peter Zijlstra)
Joel Fernandes (Google) (6):
net: rtnetlink: Fix incorrect RCU API usage
ixgbe: Fix incorrect RCU API usage
sched/cpufreq: Annotate cpufreq_update_util_data pointer with __rcu
sched_domain: Annotate RCU pointers properly
rcuwait: Annotate task_struct with __rcu
sched: Annotate perf_domain pointer with __rcu
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 15 ++++++++++-----
include/linux/rcuwait.h | 2 +-
include/linux/sched/topology.h | 4 ++--
kernel/sched/cpufreq.c | 2 +-
kernel/sched/sched.h | 18 +++++++++---------
kernel/sched/topology.c | 10 +++++-----
net/core/rtnetlink.c | 4 ++--
8 files changed, 32 insertions(+), 27 deletions(-)
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply
* RE: [PATCH 2/3] net: ethernet: add c45 PHY support in MDIO read/write functions.
From: Parshuram Raju Thombare @ 2019-02-23 6:27 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
netdev@vger.kernel.org, f.fainelli@gmail.com,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Piotr Sroka, Jan Kotas
In-Reply-To: <20190222214140.GO5894@lunn.ch>
Regards,
Parshuram Thombare
>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Saturday, February 23, 2019 3:12 AM
>To: Parshuram Raju Thombare <pthombar@cadence.com>
>Cc: nicolas.ferre@microchip.com; davem@davemloft.net;
>netdev@vger.kernel.org; f.fainelli@gmail.com; hkallweit1@gmail.com; linux-
>kernel@vger.kernel.org; Rafal Ciepiela <rafalc@cadence.com>; Piotr Sroka
><piotrs@cadence.com>; Jan Kotas <jank@cadence.com>
>Subject: Re: [PATCH 2/3] net: ethernet: add c45 PHY support in MDIO read/write
>functions.
>
>EXTERNAL MAIL
>
>
>On Fri, Feb 22, 2019 at 08:12:42PM +0000, Parshuram Thombare wrote:
>> This patch modify MDIO read/write functions to support communication
>> with C45 PHY in Cadence ethernet controller driver.
>
>Hi Parshuram
>
>Are all versions of the MDIO controller capable of doing C45?
>
> Andrew
Now driver support c22 and c45 PHY.
Are you suggesting to add check for C45 PHY using is_c45 in phydev ?
Regards,
Parshuram Thombare
^ permalink raw reply
* RE: [PATCH 3/3] net: ethernet: add support for high speed mac and usxgmii pcs
From: Parshuram Raju Thombare @ 2019-02-23 6:24 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
netdev@vger.kernel.org, f.fainelli@gmail.com,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Piotr Sroka, Jan Kotas
In-Reply-To: <20190222214404.GP5894@lunn.ch>
>> if (macb_is_gem(bp)) {
>> - linkmode_copy(phydev->supported, PHY_GBIT_FEATURES);
>> - if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED)
>> -
> linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>> - phydev->supported);
>> + if (bp->caps & MACB_CAPS_HIGH_SPEED) {
>> + linkmode_copy(phydev->supported,
>PHY_10GBIT_FEATURES);
>> + } else {
>> + u32 bitmask =
>ETHTOOL_LINK_MODE_2500baseT_Full_BIT;
>> +
>> + linkmode_copy(phydev->supported,
>PHY_GBIT_FEATURES);
>> + if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED)
>> + linkmode_set_bit(bitmask, phydev->supported);
>> + }
>
>Same issue again. Somebody could be using a 10G MAC with a 2.5G PHY.
>
> Andrew
Hi Andrew,
Ok, I think this should have been logical AND. I will modify to use phy_set_max_speed()
instead of directly copying linkmodes.
Regards,
Parshuram Thombare
^ permalink raw reply
* RE: [PATCH 1/3] net: ethernet: add support for PCS and 2.5G speed
From: Parshuram Raju Thombare @ 2019-02-23 6:24 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
netdev@vger.kernel.org, f.fainelli@gmail.com,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Piotr Sroka, Jan Kotas
In-Reply-To: <20190222213953.GN5894@lunn.ch>
>> /* mask with MAC supported features */
>> - if (macb_is_gem(bp) && bp->caps &
>MACB_CAPS_GIGABIT_MODE_AVAILABLE)
>> - phy_set_max_speed(phydev, SPEED_1000);
>> - else
>> - phy_set_max_speed(phydev, SPEED_100);
>> + if (macb_is_gem(bp)) {
>> + linkmode_copy(phydev->supported, PHY_GBIT_FEATURES);
>> + if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED)
>> +
> linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>> + phydev->supported);
>> + } else {
>> + linkmode_copy(phydev->supported, PHY_BASIC_FEATURES);
>> + }
>> +
>> + linkmode_copy(phydev->advertising, phydev->supported);
>
>This is not correct. Just because the MAC can do 2.5G does not mean the PHY
>can. So you should not be adding links modes. Also, somebody might be using a
>PHY that can do 2.5G with a MAC which can only do 1G.
>
>The correct thing to do is call phy_set_max_speed() with the maximum speed the
>MAC can do.
Hi Andrew,
Ok, I think this should have been logical AND. I will modify to use phy_set_max_speed()
instead of directly copying linkmodes.
Regards,
Parshuram Thombare
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: add missing of_node_put
From: Himadri Pandya @ 2019-02-23 5:53 UTC (permalink / raw)
To: Andrew Lunn, wen.yang99
Cc: vivien.didelot, f.fainelli, davem, netdev, linux-kernel
In-Reply-To: <20190222143649.GN5653@lunn.ch>
On 22/02/19 8:06 PM, Andrew Lunn wrote:
> On Fri, Feb 22, 2019 at 04:48:18PM +0530, Himadri Pandya wrote:
>> Decrement the reference count on port while returning out of the
>> loop. Issue identified by Coccinelle.
> You and Wen Yang are both fixing the same issue. Maybe you can
> coordinate?
Sure.
- Himadri
>
> Andrew
^ permalink raw reply
* [PATCH] ila: Fix uninitialised return value in ila_xlat_nl_cmd_flush
From: Herbert Xu @ 2019-02-23 5:30 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kbuild, kbuild-all, linux-wireless, Johannes Berg, netdev
In-Reply-To: <20190223042747.GK1711@kadam>
On Sat, Feb 23, 2019 at 07:27:47AM +0300, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
> head: db0342b20f32f584aedff27c9c09e0a4bbe5beff
> commit: 6c4128f658571b2dc7e01058ad09a8e947bc0159 [25/27] rhashtable: Remove obsolete rhashtable_walk_init function
>
> smatch warnings:
> net/ipv6/ila/ila_xlat.c:420 ila_xlat_nl_cmd_flush() error: uninitialized symbol 'ret'.
Thanks for catching this.
---8<---
This patch fixes an uninitialised return value error in
ila_xlat_nl_cmd_flush.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Fixes: 6c4128f65857 ("rhashtable: Remove obsolete...")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c
index ae6cd4cef8db..79d2e43c05c5 100644
--- a/net/ipv6/ila/ila_xlat.c
+++ b/net/ipv6/ila/ila_xlat.c
@@ -383,7 +383,7 @@ int ila_xlat_nl_cmd_flush(struct sk_buff *skb, struct genl_info *info)
struct rhashtable_iter iter;
struct ila_map *ila;
spinlock_t *lock;
- int ret;
+ int ret = 0;
rhashtable_walk_enter(&ilan->xlat.rhash_table, &iter);
rhashtable_walk_start(&iter);
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Masami Hiramatsu @ 2019-02-23 4:51 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Linus Torvalds, David Miller, Masami Hiramatsu, Steven Rostedt,
Andy Lutomirski, Linux List Kernel Mailing, Ingo Molnar,
Andrew Morton, stable, Changbin Du, Jann Horn, Kees Cook,
Andrew Lutomirski, Daniel Borkmann, Netdev, bpf
In-Reply-To: <20190222235618.dxewmv5dukltaoxl@ast-mbp.dhcp.thefacebook.com>
On Fri, 22 Feb 2019 15:56:20 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Fri, Feb 22, 2019 at 03:16:35PM -0800, Linus Torvalds wrote:
> >
> > So a kernel pointer value of 0x12345678 could be a value kernel
> > pointer pointing to some random kmalloc'ed kernel memory, and a user
> > pointer value of 0x12345678 could be a valid _user_ pointer pointing
> > to some user mapping.
> >
> > See?
> >
> > If you access a user pointer, you need to use a user accessor function
> > (eg "get_user()"), while if you access a kernel pointer you need to
> > just dereference it directly (unless you can't trust it, in which case
> > you need to use a _different_ accessor function).
>
> that was clear already.
> Reading 0x12345678 via probe_kernel_read can return valid value
> and via get_user() can return another valid value on _some_ architectures.
>
> > The fact that user and kernel pointers happen to be distinct on x86-64
> > (right now) is just a random implementation detail.
>
> yes and my point that people already rely on this implementation detail.
> Say we implement
> int bpf_probe_read(void *val, void *unsafe_ptr)
> {
> if (probe_kernel_read(val, unsafe_ptr) == OK) {
> return 0;
> } else (get_user(val, unsafe_ptr) == OK) {
> return 0;
> } else {
> *val = 0;
> return -EFAULT;
> }
> }
Note that we can not use get_user() form kprobe handler. If you use it,
you have to prepare fault_handler() and make bpf itself can be aborted.
So, maybe you can use probe_user_read().
Hmm, however, it still doesn't work correctly on "some" architecture,
since whether a pointer (address) points user-space or kernel-space
depends on the context. In kprobe/bpf, the context means where you
put the probe and which pointer you record.
I think only "__user" tag tells us which one is user-space. But
unfortunately, that "__user" tag is only for compiler or checker, not
for runtime binary. Such useful attribute goes away when we execute it.
So, even if we introduce "ustring", ftrace/perf users has to decide to use
it by themselves. As far as I know, DWARF(debuginfo) also doesn't have
that attribute. So perf-probe can not help it from debuginfo.
(Maybe if we introduce C parser, it might be detected...)
> It will preserve existing bpf_probe_read() behavior on x86.
> If x86 implementation changes tomorrow then progs that read user
> addresses may start failing randomly because first probe_kernel_read()
> will be returning random values from kernel memory and that's no good,
> but at least we won't be breaking them today, so we have time to
> introduce bpf_user_read and bpf_kernel_read and folks have time to adopt them.
I see. I think bpf also has to introduce new bpf_probe_read_user() and
keep bpf_probe_read() for kernel dataa only.
> Imo that's much better than making current bpf_probe_read() fail
> on user addresses today and not providing a non disruptive path forward.
Agreed.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: pull-request: bpf 2019-02-23
From: David Miller @ 2019-02-23 4:45 UTC (permalink / raw)
To: daniel; +Cc: ast, netdev, bpf
In-Reply-To: <20190223003625.32021-1-daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Sat, 23 Feb 2019 01:36:25 +0100
> Please consider pulling these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
Pulled, thanks Daniel.
^ permalink raw reply
* Re: [PATCH bpf-next 4/4] tools/bpftool: recognize bpf_prog_info runtime and runcnt
From: Andrii Nakryiko @ 2019-02-23 4:17 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: davem, Daniel Borkmann, netdev, bpf, Kernel Team
In-Reply-To: <20190222233644.1487087-5-ast@kernel.org>
On Fri, Feb 22, 2019 at 3:37 PM Alexei Starovoitov <ast@kernel.org> wrote:
>
> $ bpftool p s
> 1: kprobe tag a56587d488d216c9 gpl runtime 79786 runcnt 8
> loaded_at 2019-02-22T12:22:51-0800 uid 0
> xlated 352B not jited memlock 4096B
>
> $ bpftool --json --pretty p s
> [{
> "id": 1,
> "type": "kprobe",
> "tag": "a56587d488d216c9",
> "gpl_compatible": true,
> "runtime": 79786,
> "runcnt": 8,
> "loaded_at": 1550866971,
> "uid": 0,
> "bytes_xlated": 352,
> "jited": false,
> "bytes_memlock": 4096
> }
> ]
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> tools/bpf/bpftool/Documentation/bpftool-prog.rst | 4 +++-
> tools/bpf/bpftool/prog.c | 7 +++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index 12bc1e2d4b46..102b180d3add 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -171,7 +171,7 @@ EXAMPLES
>
> ::
>
> - 10: xdp name some_prog tag 005a3d2123620c8b gpl
> + 10: xdp name some_prog tag 005a3d2123620c8b gpl runtime 81632 runcnt 10
> loaded_at 2017-09-29T20:11:00+0000 uid 0
> xlated 528B jited 370B memlock 4096B map_ids 10
>
> @@ -184,6 +184,8 @@ EXAMPLES
> "type": "xdp",
> "tag": "005a3d2123620c8b",
> "gpl_compatible": true,
> + "runtime": 81632,
> + "runcnt": 10,
Should these fields be called "run_time" and "run_cnt" for consistency
with other fields? I'd even suggest "run_time_ns" so that there is no
question about time units.
> "loaded_at": 1506715860,
> "uid": 0,
> "bytes_xlated": 528,
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index db978c8d76a8..8065ba11b9d5 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -214,6 +214,10 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
> info->tag[4], info->tag[5], info->tag[6], info->tag[7]);
>
> jsonw_bool_field(json_wtr, "gpl_compatible", info->gpl_compatible);
> + if (info->runtime) {
> + jsonw_uint_field(json_wtr, "runtime", info->runtime);
> + jsonw_uint_field(json_wtr, "runcnt", info->runcnt);
> + }
>
> print_dev_json(info->ifindex, info->netns_dev, info->netns_ino);
>
> @@ -277,6 +281,9 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
> fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
> print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
> printf("%s", info->gpl_compatible ? " gpl" : "");
> + if (info->runtime)
> + printf(" runtime %lld runcnt %lld",
> + info->runtime, info->runcnt);
> printf("\n");
>
> if (info->load_time) {
> --
> 2.20.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