Netdev List
 help / color / mirror / Atom feed
* [PATCH iproute2-next] iplink: add support for reporting multiple XDP programs
From: Jakub Kicinski @ 2018-07-13 20:43 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, dsahern
  Cc: stephen, netdev, oss-drivers, Jakub Kicinski

Kernel now supports attaching XDP programs in the driver
and hardware at the same time.  Print that information
correctly.

In case there are multiple programs attached kernel will
not provide IFLA_XDP_PROG_ID, so don't expect it to be
there (this also improves the printing for very old kernels
slightly, as it avoids unnecessary "prog/xdp" line).

In short mode preserve the current outputs but don't print
IDs if there are multiple.
[...]
6: netdevsim0: <BROADCAST,NOARP> mtu 1500 xdpoffload/id:11 qdisc [...]
[...]
[...]
6: netdevsim0: <BROADCAST,NOARP> mtu 1500 xdpmulti qdisc [...]
[...]

ip link output will keep using prog/xdp prefix if only one program
is attached, but can also print multiple program lines:

    prog/xdp id 8 tag fc7a51d1a693a99e jited

vs:

    prog/xdpdrv id 8 tag fc7a51d1a693a99e jited
    prog/xdpoffload id 9 tag fc7a51d1a693a99e

JSON output gains a new array called "attached" which will
contain the full list of attached programs along with their
attachment modes:

        "xdp": {
            "mode": 3,
            "prog": {
                "id": 11,
                "tag": "fc7a51d1a693a99e",
                "jited": 0
            },
            "attached": [ {
                    "mode": 3,
                    "prog": {
                        "id": 11,
                        "tag": "fc7a51d1a693a99e",
                        "jited": 0
                    }
                } ]
        },

In case there are multiple programs attached the general "xdp"
section will not contain program information:

        "xdp": {
            "mode": 4,
            "attached": [ {
                    "mode": 1,
                    "prog": {
                        "id": 10,
                        "tag": "fc7a51d1a693a99e",
                        "jited": 1
                    }
                },{
                    "mode": 3,
                    "prog": {
                        "id": 11,
                        "tag": "fc7a51d1a693a99e",
                        "jited": 0
                    }
                } ]
        },

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 ip/iplink_xdp.c | 65 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 53 insertions(+), 12 deletions(-)

diff --git a/ip/iplink_xdp.c b/ip/iplink_xdp.c
index dd4fd1fd3a3b..0328bc01a981 100644
--- a/ip/iplink_xdp.c
+++ b/ip/iplink_xdp.c
@@ -91,6 +91,18 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req,
 	return 0;
 }
 
+static void xdp_dump_json_one(struct rtattr *tb[IFLA_XDP_MAX + 1], __u32 attr,
+			      __u8 mode)
+{
+	if (!tb[attr])
+		return;
+
+	open_json_object(NULL);
+	print_uint(PRINT_JSON, "mode", NULL, mode);
+	bpf_dump_prog_info(NULL, rta_getattr_u32(tb[attr]));
+	close_json_object();
+}
+
 static void xdp_dump_json(struct rtattr *tb[IFLA_XDP_MAX + 1])
 {
 	__u32 prog_id = 0;
@@ -104,13 +116,40 @@ static void xdp_dump_json(struct rtattr *tb[IFLA_XDP_MAX + 1])
 	print_uint(PRINT_JSON, "mode", NULL, mode);
 	if (prog_id)
 		bpf_dump_prog_info(NULL, prog_id);
+
+	open_json_array(PRINT_JSON, "attached");
+	xdp_dump_json_one(tb, IFLA_XDP_SKB_PROG_ID, XDP_ATTACHED_SKB);
+	xdp_dump_json_one(tb, IFLA_XDP_DRV_PROG_ID, XDP_ATTACHED_DRV);
+	xdp_dump_json_one(tb, IFLA_XDP_HW_PROG_ID, XDP_ATTACHED_HW);
+	close_json_array(PRINT_JSON, NULL);
+
 	close_json_object();
 }
 
+static void xdp_dump_prog_one(FILE *fp, struct rtattr *tb[IFLA_XDP_MAX + 1],
+			      __u32 attr, bool link, bool details, char *pfx)
+{
+	__u32 prog_id;
+
+	if (!tb[attr])
+		return;
+
+	prog_id = rta_getattr_u32(tb[attr]);
+	if (!details) {
+		if (prog_id && !link && attr == IFLA_XDP_PROG_ID)
+			fprintf(fp, "/id:%u", prog_id);
+		return;
+	}
+
+	if (prog_id) {
+		fprintf(fp, "%s    prog/xdp%s ", _SL_, pfx);
+		bpf_dump_prog_info(fp, prog_id);
+	}
+}
+
 void xdp_dump(FILE *fp, struct rtattr *xdp, bool link, bool details)
 {
 	struct rtattr *tb[IFLA_XDP_MAX + 1];
-	__u32 prog_id = 0;
 	__u8 mode;
 
 	parse_rtattr_nested(tb, IFLA_XDP_MAX, xdp);
@@ -124,27 +163,29 @@ void xdp_dump(FILE *fp, struct rtattr *xdp, bool link, bool details)
 	else if (is_json_context())
 		return details ? (void)0 : xdp_dump_json(tb);
 	else if (details && link)
-		fprintf(fp, "%s    prog/xdp", _SL_);
+		/* don't print mode */;
 	else if (mode == XDP_ATTACHED_DRV)
 		fprintf(fp, "xdp");
 	else if (mode == XDP_ATTACHED_SKB)
 		fprintf(fp, "xdpgeneric");
 	else if (mode == XDP_ATTACHED_HW)
 		fprintf(fp, "xdpoffload");
+	else if (mode == XDP_ATTACHED_MULTI)
+		fprintf(fp, "xdpmulti");
 	else
 		fprintf(fp, "xdp[%u]", mode);
 
-	if (tb[IFLA_XDP_PROG_ID])
-		prog_id = rta_getattr_u32(tb[IFLA_XDP_PROG_ID]);
-	if (!details) {
-		if (prog_id && !link)
-			fprintf(fp, "/id:%u", prog_id);
-		fprintf(fp, " ");
-		return;
+	xdp_dump_prog_one(fp, tb, IFLA_XDP_PROG_ID, link, details, "");
+
+	if (mode == XDP_ATTACHED_MULTI) {
+		xdp_dump_prog_one(fp, tb, IFLA_XDP_SKB_PROG_ID, link, details,
+				  "generic");
+		xdp_dump_prog_one(fp, tb, IFLA_XDP_DRV_PROG_ID, link, details,
+				  "drv");
+		xdp_dump_prog_one(fp, tb, IFLA_XDP_HW_PROG_ID, link, details,
+				  "offload");
 	}
 
-	if (prog_id) {
+	if (!details || !link)
 		fprintf(fp, " ");
-		bpf_dump_prog_info(fp, prog_id);
-	}
 }
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH bpf-next 0/7] xdp: simultaneous driver and HW XDP
From: Jakub Kicinski @ 2018-07-13 20:39 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: alexei.starovoitov, oss-drivers, netdev
In-Reply-To: <427f7c6f-53c9-35b0-a6f6-2d662d38cc26@iogearbox.net>

On Fri, 13 Jul 2018 21:59:22 +0200, Daniel Borkmann wrote:
> On 07/12/2018 05:36 AM, Jakub Kicinski wrote:
> > Hi!
> > 
> > This set is adding support for loading driver and offload XDP
> > at the same time.  This enables advanced use cases where some
> > of the work is offloaded to the NIC and some is done by the host.
> > Separate netlink attributes are added for each mode of operation.
> > Driver callbacks for offload are cleaned up a little, including
> > removal of .prog_attached flag.
> > 
> > Jakub Kicinski (7):
> >   xdp: add per mode attributes for attached programs
> >   xdp: don't make drivers report attachment mode
> >   xdp: factor out common program/flags handling from drivers
> >   xdp: support simultaneous driver and hw XDP attachment
> >   netdevsim: add support for simultaneous driver and hw XDP
> >   selftests/bpf: add test for multiple programs
> >   nfp: add support for simultaneous driver and hw XDP  
> 
> LGTM as well, applied to bpf-next, thanks Jakub!
> 
> This also has iproute2 changes that would need to be sent, right?

Coming right out!

^ permalink raw reply

* Re: [PATCH bpf-next v5 0/3] bpf: btf: print bpftool map data with btf
From: Daniel Borkmann @ 2018-07-13 20:49 UTC (permalink / raw)
  To: Jakub Kicinski, Okash Khawaja
  Cc: Martin KaFai Lau, Alexei Starovoitov, Yonghong Song,
	Quentin Monnet, David S. Miller, netdev, kernel-team,
	linux-kernel
In-Reply-To: <20180711203010.51bab0cf@cakuba.lan>

On 07/12/2018 05:30 AM, Jakub Kicinski wrote:
> On Wed, 11 Jul 2018 20:08:03 -0700, Okash Khawaja wrote:
>> Hi,
>>
>> Here are the changes from v4:
>>
>> patch 2:
>>
>> - sort headers in btf_dumper.c
>> - remove extra parentheses
>> - include asm/byteorder.h
>> - compile error when big and small endian bitfields macro undefined
> 
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Hmm, strange, by accident I just noticed that only your bpf fix ever made
it to patchwork, Okash.

  https://patchwork.ozlabs.org/project/netdev/list/?submitter=74458&state=*

Potentially because you've sent with attachments which got dropped on
the list?

Could you properly submit the series again, and retaining Jakub's Reviewed-by
tag to the patches?

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH v4 net-next 19/19] net/mlx5e: Kconfig, mutually exclude compilation of TLS and IPsec accel
From: Boris Pismenny @ 2018-07-13 20:03 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, davejwatson, aviadye, saeedm
In-Reply-To: <20180712.174400.1595251769011299658.davem@davemloft.net>


On 7/12/2018 8:44 PM, David Miller wrote:
> From: Boris Pismenny <borisp@mellanox.com>
> Date: Thu, 12 Jul 2018 22:25:57 +0300
> 
>> We currently have no devices that support both TLS and IPsec using the
>> accel framework, and the current code does not support both IPsec and
>> TLS. This patch prevents such combinations.
>>
>> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
>> ---
>>   drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
>> index 2545296..d3e8c70 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
>> @@ -93,6 +93,7 @@ config MLX5_EN_TLS
>>   	depends on TLS_DEVICE
>>   	depends on TLS=y || MLX5_CORE=m
>>   	depends on MLX5_ACCEL
>> +	depends on !MLX5_EN_IPSEC
>>   	default n
> 
> You absolutely cannot do this.
> 
> You are forcing a distribution to pick one offload or the other at
> build time, that's insane.
> 
> Please find a way to support both offloads in the driver.  It is
> absolutely valid for a distribution to ship the driver in a state that
> supports both offloads and you must therefore support this properly.
> 
> Thank you.
> 

Thanks Dave.

We currently have no devices that support both TLS and IPsec using the
accel framework, and the current code does not support both IPsec and
TLS. The purpose of this patch was to prevent such cases using Kconfig.

Looking a bit more carefully at the code. We don't need this patch, 
because we still don't have a deviceID for both TLS and IPsec, so the 
problematic flow cannot happen. So we've just been over zealous here. 
I'll remove this patch and send a v5.

^ permalink raw reply

* [PATCH 4/5] ceph: use timespec64 for r_mtime
From: Arnd Bergmann @ 2018-07-13 20:18 UTC (permalink / raw)
  To: Ilya Dryomov, Yan Zheng, Sage Weil
  Cc: y2038, linux-fsdevel, ceph-devel, Arnd Bergmann, Alex Elder,
	Jens Axboe, David S. Miller, linux-block, linux-kernel, netdev
In-Reply-To: <20180713201923.3200799-1-arnd@arndb.de>

The request mtime field is used all over ceph, and is currently
represented as a 'timespec' structure in Linux. This changes it to
timespec64 to allow times beyond 2038, modifying all users at the
same time.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: undo an unneeded change pointed out by Yan Zheng.
---
 drivers/block/rbd.c             |  2 +-
 fs/ceph/addr.c                  | 12 ++++++------
 fs/ceph/file.c                  |  8 ++++----
 include/linux/ceph/osd_client.h |  6 +++---
 net/ceph/osd_client.c           |  8 ++++----
 5 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index d81c653b9bf6..a9a11fe484e5 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1452,7 +1452,7 @@ static void rbd_osd_req_format_write(struct rbd_obj_request *obj_request)
 	struct ceph_osd_request *osd_req = obj_request->osd_req;
 
 	osd_req->r_flags = CEPH_OSD_FLAG_WRITE;
-	ktime_get_real_ts(&osd_req->r_mtime);
+	ktime_get_real_ts64(&osd_req->r_mtime);
 	osd_req->r_data_offset = obj_request->ex.oe_off;
 }
 
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 292b3d72d725..d44d51e69e76 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -574,7 +574,7 @@ static u64 get_writepages_data_length(struct inode *inode,
  */
 static int writepage_nounlock(struct page *page, struct writeback_control *wbc)
 {
-	struct timespec ts;
+	struct timespec64 ts;
 	struct inode *inode;
 	struct ceph_inode_info *ci;
 	struct ceph_fs_client *fsc;
@@ -625,7 +625,7 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc)
 		set_bdi_congested(inode_to_bdi(inode), BLK_RW_ASYNC);
 
 	set_page_writeback(page);
-	ts = timespec64_to_timespec(inode->i_mtime);
+	ts = inode->i_mtime;
 	err = ceph_osdc_writepages(&fsc->client->osdc, ceph_vino(inode),
 				   &ci->i_layout, snapc, page_off, len,
 				   ceph_wbc.truncate_seq,
@@ -1134,7 +1134,7 @@ static int ceph_writepages_start(struct address_space *mapping,
 			pages = NULL;
 		}
 
-		req->r_mtime = timespec64_to_timespec(inode->i_mtime);
+		req->r_mtime = inode->i_mtime;
 		rc = ceph_osdc_start_request(&fsc->client->osdc, req, true);
 		BUG_ON(rc);
 		req = NULL;
@@ -1734,7 +1734,7 @@ int ceph_uninline_data(struct file *filp, struct page *locked_page)
 		goto out;
 	}
 
-	req->r_mtime = timespec64_to_timespec(inode->i_mtime);
+	req->r_mtime = inode->i_mtime;
 	err = ceph_osdc_start_request(&fsc->client->osdc, req, false);
 	if (!err)
 		err = ceph_osdc_wait_request(&fsc->client->osdc, req);
@@ -1776,7 +1776,7 @@ int ceph_uninline_data(struct file *filp, struct page *locked_page)
 			goto out_put;
 	}
 
-	req->r_mtime = timespec64_to_timespec(inode->i_mtime);
+	req->r_mtime = inode->i_mtime;
 	err = ceph_osdc_start_request(&fsc->client->osdc, req, false);
 	if (!err)
 		err = ceph_osdc_wait_request(&fsc->client->osdc, req);
@@ -1937,7 +1937,7 @@ static int __ceph_pool_perm_get(struct ceph_inode_info *ci,
 				     0, false, true);
 	err = ceph_osdc_start_request(&fsc->client->osdc, rd_req, false);
 
-	wr_req->r_mtime = timespec64_to_timespec(ci->vfs_inode.i_mtime);
+	wr_req->r_mtime = ci->vfs_inode.i_mtime;
 	err2 = ceph_osdc_start_request(&fsc->client->osdc, wr_req, false);
 
 	if (!err)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index e2679e8a2535..732d14726e80 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -720,7 +720,7 @@ struct ceph_aio_request {
 	struct list_head osd_reqs;
 	unsigned num_reqs;
 	atomic_t pending_reqs;
-	struct timespec mtime;
+	struct timespec64 mtime;
 	struct ceph_cap_flush *prealloc_cf;
 };
 
@@ -922,7 +922,7 @@ ceph_direct_read_write(struct kiocb *iocb, struct iov_iter *iter,
 	int num_pages = 0;
 	int flags;
 	int ret;
-	struct timespec mtime = timespec64_to_timespec(current_time(inode));
+	struct timespec64 mtime = current_time(inode);
 	size_t count = iov_iter_count(iter);
 	loff_t pos = iocb->ki_pos;
 	bool write = iov_iter_rw(iter) == WRITE;
@@ -1130,7 +1130,7 @@ ceph_sync_write(struct kiocb *iocb, struct iov_iter *from, loff_t pos,
 	int flags;
 	int ret;
 	bool check_caps = false;
-	struct timespec mtime = timespec64_to_timespec(current_time(inode));
+	struct timespec64 mtime = current_time(inode);
 	size_t count = iov_iter_count(from);
 
 	if (ceph_snap(file_inode(file)) != CEPH_NOSNAP)
@@ -1662,7 +1662,7 @@ static int ceph_zero_partial_object(struct inode *inode,
 		goto out;
 	}
 
-	req->r_mtime = timespec64_to_timespec(inode->i_mtime);
+	req->r_mtime = inode->i_mtime;
 	ret = ceph_osdc_start_request(&fsc->client->osdc, req, false);
 	if (!ret) {
 		ret = ceph_osdc_wait_request(&fsc->client->osdc, req);
diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h
index 0d6ee04b4c41..2e6611c1e9a0 100644
--- a/include/linux/ceph/osd_client.h
+++ b/include/linux/ceph/osd_client.h
@@ -199,7 +199,7 @@ struct ceph_osd_request {
 	/* set by submitter */
 	u64 r_snapid;                         /* for reads, CEPH_NOSNAP o/w */
 	struct ceph_snap_context *r_snapc;    /* for writes */
-	struct timespec r_mtime;              /* ditto */
+	struct timespec64 r_mtime;            /* ditto */
 	u64 r_data_offset;                    /* ditto */
 	bool r_linger;                        /* don't resend on failure */
 
@@ -253,7 +253,7 @@ struct ceph_osd_linger_request {
 	struct ceph_osd_request_target t;
 	u32 map_dne_bound;
 
-	struct timespec mtime;
+	struct timespec64 mtime;
 
 	struct kref kref;
 	struct mutex lock;
@@ -508,7 +508,7 @@ extern int ceph_osdc_writepages(struct ceph_osd_client *osdc,
 				struct ceph_snap_context *sc,
 				u64 off, u64 len,
 				u32 truncate_seq, u64 truncate_size,
-				struct timespec *mtime,
+				struct timespec64 *mtime,
 				struct page **pages, int nr_pages);
 
 /* watch/notify */
diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
index a00c74f1154e..a87a021ca9d0 100644
--- a/net/ceph/osd_client.c
+++ b/net/ceph/osd_client.c
@@ -1978,7 +1978,7 @@ static void encode_request_partial(struct ceph_osd_request *req,
 	p += sizeof(struct ceph_blkin_trace_info);
 
 	ceph_encode_32(&p, 0); /* client_inc, always 0 */
-	ceph_encode_timespec(p, &req->r_mtime);
+	ceph_encode_timespec64(p, &req->r_mtime);
 	p += sizeof(struct ceph_timespec);
 
 	encode_oloc(&p, end, &req->r_t.target_oloc);
@@ -4512,7 +4512,7 @@ ceph_osdc_watch(struct ceph_osd_client *osdc,
 	ceph_oid_copy(&lreq->t.base_oid, oid);
 	ceph_oloc_copy(&lreq->t.base_oloc, oloc);
 	lreq->t.flags = CEPH_OSD_FLAG_WRITE;
-	ktime_get_real_ts(&lreq->mtime);
+	ktime_get_real_ts64(&lreq->mtime);
 
 	lreq->reg_req = alloc_linger_request(lreq);
 	if (!lreq->reg_req) {
@@ -4570,7 +4570,7 @@ int ceph_osdc_unwatch(struct ceph_osd_client *osdc,
 	ceph_oid_copy(&req->r_base_oid, &lreq->t.base_oid);
 	ceph_oloc_copy(&req->r_base_oloc, &lreq->t.base_oloc);
 	req->r_flags = CEPH_OSD_FLAG_WRITE;
-	ktime_get_real_ts(&req->r_mtime);
+	ktime_get_real_ts64(&req->r_mtime);
 	osd_req_op_watch_init(req, 0, lreq->linger_id,
 			      CEPH_OSD_WATCH_OP_UNWATCH);
 
@@ -5136,7 +5136,7 @@ int ceph_osdc_writepages(struct ceph_osd_client *osdc, struct ceph_vino vino,
 			 struct ceph_snap_context *snapc,
 			 u64 off, u64 len,
 			 u32 truncate_seq, u64 truncate_size,
-			 struct timespec *mtime,
+			 struct timespec64 *mtime,
 			 struct page **pages, int num_pages)
 {
 	struct ceph_osd_request *req;
-- 
2.9.0

^ permalink raw reply related

* [PATCH 1/5] ceph: use timespec64 in for keepalive
From: Arnd Bergmann @ 2018-07-13 20:18 UTC (permalink / raw)
  To: Ilya Dryomov, Yan Zheng, Sage Weil
  Cc: y2038, linux-fsdevel, ceph-devel, Arnd Bergmann, David S. Miller,
	linux-kernel, netdev

ceph_con_keepalive_expired() is the last user of timespec_add() and some
of the last uses of ktime_get_real_ts().  Replacing this with timespec64
based interfaces  lets us remove that deprecated API.

I'm introducing new ceph_encode_timespec64()/ceph_decode_timespec64()
here that take timespec64 structures and convert to/from ceph_timespec,
which is defined to have an unsigned 32-bit tv_sec member. This extends
the range of valid times to year 2106, avoiding the year 2038 overflow.

The ceph file system portion still uses the old functions for inode
timestamps, this will be done separately after the VFS layer is converted.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/ceph/decode.h    | 20 +++++++++++++++++++-
 include/linux/ceph/messenger.h |  2 +-
 net/ceph/auth_x.c              | 14 +++++++-------
 net/ceph/auth_x.h              |  2 +-
 net/ceph/cls_lock_client.c     |  4 ++--
 net/ceph/messenger.c           | 20 ++++++++++----------
 6 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/include/linux/ceph/decode.h b/include/linux/ceph/decode.h
index d143ac8879c6..45efe09fb7b4 100644
--- a/include/linux/ceph/decode.h
+++ b/include/linux/ceph/decode.h
@@ -194,8 +194,26 @@ ceph_decode_skip_n(p, end, sizeof(u8), bad)
 	} while (0)
 
 /*
- * struct ceph_timespec <-> struct timespec
+ * struct ceph_timespec <-> struct timespec64
  */
+static inline void ceph_decode_timespec64(struct timespec64 *ts,
+					const struct ceph_timespec *tv)
+{
+	/*
+	 * this will still overflow in year 2106. We could extend
+	 * the protocol to steal two more bits from tv_nsec to
+	 * add three more 136 year epochs after that the way ext4
+	 * does if necessary.
+	 */
+	ts->tv_sec = (time64_t)le32_to_cpu(tv->tv_sec);
+	ts->tv_nsec = (long)le32_to_cpu(tv->tv_nsec);
+}
+static inline void ceph_encode_timespec64(struct ceph_timespec *tv,
+					const struct timespec64 *ts)
+{
+	tv->tv_sec = cpu_to_le32((u32)ts->tv_sec);
+	tv->tv_nsec = cpu_to_le32((u32)ts->tv_nsec);
+}
 static inline void ceph_decode_timespec(struct timespec *ts,
 					const struct ceph_timespec *tv)
 {
diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h
index c7dfcb8a1fb2..a718b877c597 100644
--- a/include/linux/ceph/messenger.h
+++ b/include/linux/ceph/messenger.h
@@ -330,7 +330,7 @@ struct ceph_connection {
 	int in_base_pos;     /* bytes read */
 	__le64 in_temp_ack;  /* for reading an ack */
 
-	struct timespec last_keepalive_ack; /* keepalive2 ack stamp */
+	struct timespec64 last_keepalive_ack; /* keepalive2 ack stamp */
 
 	struct delayed_work work;	    /* send|recv work */
 	unsigned long       delay;          /* current delay interval */
diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
index 2f4a1baf5f52..b05c3a540a5a 100644
--- a/net/ceph/auth_x.c
+++ b/net/ceph/auth_x.c
@@ -149,12 +149,12 @@ static int process_one_ticket(struct ceph_auth_client *ac,
 	void *dp, *dend;
 	int dlen;
 	char is_enc;
-	struct timespec validity;
+	struct timespec64 validity;
 	void *tp, *tpend;
 	void **ptp;
 	struct ceph_crypto_key new_session_key = { 0 };
 	struct ceph_buffer *new_ticket_blob;
-	unsigned long new_expires, new_renew_after;
+	time64_t new_expires, new_renew_after;
 	u64 new_secret_id;
 	int ret;
 
@@ -189,11 +189,11 @@ static int process_one_ticket(struct ceph_auth_client *ac,
 	if (ret)
 		goto out;
 
-	ceph_decode_timespec(&validity, dp);
+	ceph_decode_timespec64(&validity, dp);
 	dp += sizeof(struct ceph_timespec);
-	new_expires = get_seconds() + validity.tv_sec;
+	new_expires = ktime_get_real_seconds() + validity.tv_sec;
 	new_renew_after = new_expires - (validity.tv_sec / 4);
-	dout(" expires=%lu renew_after=%lu\n", new_expires,
+	dout(" expires=%llu renew_after=%llu\n", new_expires,
 	     new_renew_after);
 
 	/* ticket blob for service */
@@ -385,13 +385,13 @@ static bool need_key(struct ceph_x_ticket_handler *th)
 	if (!th->have_key)
 		return true;
 
-	return get_seconds() >= th->renew_after;
+	return ktime_get_real_seconds() >= th->renew_after;
 }
 
 static bool have_key(struct ceph_x_ticket_handler *th)
 {
 	if (th->have_key) {
-		if (get_seconds() >= th->expires)
+		if (ktime_get_real_seconds() >= th->expires)
 			th->have_key = false;
 	}
 
diff --git a/net/ceph/auth_x.h b/net/ceph/auth_x.h
index 454cb54568af..57ba99f7736f 100644
--- a/net/ceph/auth_x.h
+++ b/net/ceph/auth_x.h
@@ -22,7 +22,7 @@ struct ceph_x_ticket_handler {
 	u64 secret_id;
 	struct ceph_buffer *ticket_blob;
 
-	unsigned long renew_after, expires;
+	time64_t renew_after, expires;
 };
 
 #define CEPHX_AU_ENC_BUF_LEN	128  /* big enough for encrypted blob */
diff --git a/net/ceph/cls_lock_client.c b/net/ceph/cls_lock_client.c
index 8d2032b2f225..2105a6eaa66c 100644
--- a/net/ceph/cls_lock_client.c
+++ b/net/ceph/cls_lock_client.c
@@ -32,7 +32,7 @@ int ceph_cls_lock(struct ceph_osd_client *osdc,
 	int desc_len = strlen(desc);
 	void *p, *end;
 	struct page *lock_op_page;
-	struct timespec mtime;
+	struct timespec64 mtime;
 	int ret;
 
 	lock_op_buf_size = name_len + sizeof(__le32) +
@@ -63,7 +63,7 @@ int ceph_cls_lock(struct ceph_osd_client *osdc,
 	ceph_encode_string(&p, end, desc, desc_len);
 	/* only support infinite duration */
 	memset(&mtime, 0, sizeof(mtime));
-	ceph_encode_timespec(p, &mtime);
+	ceph_encode_timespec64(p, &mtime);
 	p += sizeof(struct ceph_timespec);
 	ceph_encode_8(&p, flags);
 
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index c6413c360771..3f6336248509 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -1417,11 +1417,11 @@ static void prepare_write_keepalive(struct ceph_connection *con)
 	dout("prepare_write_keepalive %p\n", con);
 	con_out_kvec_reset(con);
 	if (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2) {
-		struct timespec now;
+		struct timespec64 now;
 
-		ktime_get_real_ts(&now);
+		ktime_get_real_ts64(&now);
 		con_out_kvec_add(con, sizeof(tag_keepalive2), &tag_keepalive2);
-		ceph_encode_timespec(&con->out_temp_keepalive2, &now);
+		ceph_encode_timespec64(&con->out_temp_keepalive2, &now);
 		con_out_kvec_add(con, sizeof(con->out_temp_keepalive2),
 				 &con->out_temp_keepalive2);
 	} else {
@@ -2555,7 +2555,7 @@ static int read_keepalive_ack(struct ceph_connection *con)
 	int ret = read_partial(con, size, size, &ceph_ts);
 	if (ret <= 0)
 		return ret;
-	ceph_decode_timespec(&con->last_keepalive_ack, &ceph_ts);
+	ceph_decode_timespec64(&con->last_keepalive_ack, &ceph_ts);
 	prepare_read_tag(con);
 	return 1;
 }
@@ -3223,12 +3223,12 @@ bool ceph_con_keepalive_expired(struct ceph_connection *con,
 {
 	if (interval > 0 &&
 	    (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2)) {
-		struct timespec now;
-		struct timespec ts;
-		ktime_get_real_ts(&now);
-		jiffies_to_timespec(interval, &ts);
-		ts = timespec_add(con->last_keepalive_ack, ts);
-		return timespec_compare(&now, &ts) >= 0;
+		struct timespec64 now;
+		struct timespec64 ts;
+		ktime_get_real_ts64(&now);
+		jiffies_to_timespec64(interval, &ts);
+		ts = timespec64_add(con->last_keepalive_ack, ts);
+		return timespec64_compare(&now, &ts) >= 0;
 	}
 	return false;
 }
-- 
2.9.0

^ permalink raw reply related

* Re: [PATCH] scripts/tags.sh: Add BPF_CALL
From: Daniel Borkmann @ 2018-07-13 20:17 UTC (permalink / raw)
  To: Alexei Starovoitov, Constantine Shulyupin
  Cc: ast, netdev, Andrew Morton, Vlastimil Babka, Arend van Spriel,
	Joey Pabalinas, Kirill A. Shutemov, Matthew Wilcox, open list
In-Reply-To: <20180713181001.2ugugmnmldyrq7li@ast-mbp.dhcp.thefacebook.com>

On 07/13/2018 08:10 PM, Alexei Starovoitov wrote:
> On Thu, Jul 12, 2018 at 08:28:46AM +0300, Constantine Shulyupin wrote:
>> Signed-off-by: Constantine Shulyupin <const@MakeLinux.com>
> 
> Acked-by: Alexei Starovoitov <ast@kernel.org>

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

^ permalink raw reply

* Re: [PATCH bpf-next 0/7] xdp: simultaneous driver and HW XDP
From: Daniel Borkmann @ 2018-07-13 19:59 UTC (permalink / raw)
  To: Jakub Kicinski, alexei.starovoitov; +Cc: oss-drivers, netdev
In-Reply-To: <20180712033644.23954-1-jakub.kicinski@netronome.com>

On 07/12/2018 05:36 AM, Jakub Kicinski wrote:
> Hi!
> 
> This set is adding support for loading driver and offload XDP
> at the same time.  This enables advanced use cases where some
> of the work is offloaded to the NIC and some is done by the host.
> Separate netlink attributes are added for each mode of operation.
> Driver callbacks for offload are cleaned up a little, including
> removal of .prog_attached flag.
> 
> Jakub Kicinski (7):
>   xdp: add per mode attributes for attached programs
>   xdp: don't make drivers report attachment mode
>   xdp: factor out common program/flags handling from drivers
>   xdp: support simultaneous driver and hw XDP attachment
>   netdevsim: add support for simultaneous driver and hw XDP
>   selftests/bpf: add test for multiple programs
>   nfp: add support for simultaneous driver and hw XDP

LGTM as well, applied to bpf-next, thanks Jakub!

This also has iproute2 changes that would need to be sent, right?

^ permalink raw reply

* Re: KASAN: null-ptr-deref Read in smc_ioctl
From: Byoungyoung Lee @ 2018-07-13 20:13 UTC (permalink / raw)
  To: netdev, ubraun, linux-s390
  Cc: LKML, DaeRyong Jeong, Kyungtae Kim,
	Basavesh Ammanaghatta Shivakumar, syzkaller
In-Reply-To: <87k1q4nnit.fsf@gmail.com>


Attached C repro code as well as its kernel config. It takes about 10-30
seconds to reproduce.

C repro: https://kiwi.cs.purdue.edu/static/race-fuzzer/null-ptr-deref-smc_ioctl.c
kernel config (v4.18-rc3): https://kiwi.cs.purdue.edu/static/race-fuzzer/null-ptr-deref-smc_ioctl.c


[  172.890255]
==================================================================
[  172.892790] BUG: KASAN: null-ptr-deref in smc_ioctl+0x5c5/0x7a0
[  172.894579] Read of size 4 at addr 0000000000000020 by task repro.exe/5499
[  172.896648]
[  172.897213] CPU: 0 PID: 5499 Comm: repro.exe Not tainted 4.18.0-rc3#1
[  172.899216] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
[  172.902409] Call Trace:
[  172.903173]  dump_stack+0x18f/0x26c
[  172.904202]  ? dump_stack_print_info.cold.2+0x40/0x40
[  172.905712]  ? kasan_check_write+0x14/0x20
[  172.906926]  ? do_raw_spin_lock+0x9c/0x120
[  172.908111]  ? vprintk_func+0x81/0xe7
[  172.909202]  ? smc_ioctl+0x5c5/0x7a0
[  172.910244]  kasan_report.cold.7+0x13b/0x2f5
[  172.911472]  __asan_load4+0x78/0x80
[  172.912506]  smc_ioctl+0x5c5/0x7a0
[  172.913463]  ? smc_tx_prepared_sends+0x300/0x300
[  172.914722]  ? find_held_lock+0xca/0xf0
[  172.915809]  ? avc_has_extended_perms+0x6d6/0xec0
[  172.917072]  ? lock_downgrade+0x390/0x390
[  172.918156]  ? lock_release+0x550/0x550
[  172.919205]  ? kasan_check_read+0x11/0x20
[  172.920329]  ? rcu_report_qs_rnp+0x410/0x410
[  172.921504]  ? __lock_is_held+0x39/0xc0
[  172.922569]  ? avc_has_extended_perms+0x82b/0xec0
[  172.923887]  sock_do_ioctl+0xcc/0x380
[  172.924897]  ? compat_ifr_data_ioctl+0x150/0x150
[  172.926052]  ? avc_ss_reset+0x100/0x100
[  172.927033]  ? lock_downgrade+0x390/0x390
[  172.928057]  ? kasan_check_read+0x11/0x20
[  172.929076]  ? rcu_is_watching+0x9d/0xe0
[  172.930081]  ? rcu_report_qs_rnp+0x410/0x410
[  172.931174]  ? __sanitizer_cov_trace_switch+0x53/0x90
[  172.932464]  sock_ioctl+0x2bd/0x5a0
[  172.933363]  ? dlci_ioctl_set+0x40/0x40
[  172.934348]  ? ___might_sleep+0x1a4/0x280
[  172.935372]  ? check_same_owner+0x240/0x240
[  172.936452]  ? expand_files.part.8+0x750/0x750
[  172.937535]  ? rcu_note_context_switch+0x500/0x500
[  172.938684]  ? dlci_ioctl_set+0x40/0x40
[  172.939605]  do_vfs_ioctl+0x188/0xf80
[  172.940510]  ? ioctl_preallocate+0x200/0x200
[  172.941538]  ? selinux_capable+0x40/0x40
[  172.942475]  ? get_unused_fd_flags+0xdb/0x110
[  172.943535]  ? __x64_sys_futex+0x3cb/0x540
[  172.944536]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
[  172.945818]  ksys_ioctl+0xa9/0xd0
[  172.946636]  __x64_sys_ioctl+0x43/0x50
[  172.947570]  do_syscall_64+0x182/0x540
[  172.948490]  ? syscall_return_slowpath+0x3f0/0x3f0
[  172.949596]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
[  172.950798]  ? syscall_return_slowpath+0x266/0x3f0
[  172.951867]  ? mark_held_locks+0x25/0xb0
[  172.952798]  ? entry_SYSCALL_64_after_hwframe+0x59/0xbe
[  172.953967]  ? trace_hardirqs_off_caller+0xb5/0x120
[  172.955059]  ? trace_hardirqs_off_thunk+0x1a/0x1c
[  172.956121]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  172.957263] RIP: 0033:0x452a09
[  172.957961] Code: e8 dc f8 01 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00
48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f
05 <48> 3d 01 f0 ff ff 0f 83 2b af fb ff c3 66 2e 0f 1f 84 00 00 00 00
[  172.962330] RSP: 002b:00007f40bc5cacd8 EFLAGS: 00000297 ORIG_RAX: 0000000000000010
[  172.963917] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000452a09
[  172.965405] RDX: 0000000020000180 RSI: 0000000000008905 RDI: 0000000000000058
[  172.966883] RBP: 00007f40bc5cad10 R08: 0000000000000000 R09: 0000000000000000
[  172.968369] R10: 0000000000000000 R11: 0000000000000297 R12: 0000000000000000
[  172.969862] R13: 0000000000000000 R14: 00007f40bc5cb9c0 R15: 00007f40bc5cb700


Thanks,
Byoungyoung


Byoungyoung Lee <lifeasageek@gmail.com> writes:

> Reporting the crash: KASAN: null-ptr-deref Read in smc_ioctl
>
> This crash has been found in v4.18-rc3 using RaceFuzzer (a modified
> version of Syzkaller), which we describe more at the end of this
> report.
>
> Our analysis shows that the race occurs when invoking two syscalls
> concurrently, ioctl$sock_inet_tcp_SIOCATMARK() and listen(). More
> specifically, two code lines, `if (smc->sk.sk_state == SMC_LISTEN)` in
> smc_ioctl() and `sk->sk_state = SMC_LISTEN` in smc_listen() are racing
> as switching its execution order results in different execution
> behaviors, which in turn raises null-ptr-deref.  More details on the
> thread interleaving raising the crash are follows.
>
> Thread interleaving:
> CPU0 (smc_ioctl)                                        CPU1 (smc_listen)
> =====							=====
>
> // net/smc/af_smc.c#L1524 (v4.18-rc3)
> if (smc->sk.sk_state == SMC_LISTEN)
>
>                                                         // net/smc/af_smc.c#L1106 (v4.18-rc3)
>                                                         sk->sk_state = SMC_LISTEN;
>                                                         // ...
>                                                         release_sock(lsk);
>
> if (smc->sk.sk_state == SMC_INIT ||
>     smc->sk.sk_state == SMC_CLOSED) {
>   // ...
> } else {
>
>   // ...
>   answ = smc_curs_diff(conn->rmb_desc->len, // null-ptr-deref
>                        &cons, &urg) == 1;
> }
>
> Note that all the other cases in smc_ioctl() seem to have similar race
> issues. For example, running the SIOCOUTQNSD case leads to yet another
> crashes, such as "KASAN: null-ptr-deref Read in smc_tx_prepared_sends"
> or "general protection fault in smc_tx_prepared_sends". In particular,
> "general protection fault in smc_tx_prepared_sends" is recently
> spotted by Syzkaller
> (https://syzkaller.appspot.com/bug?id=02252298a71214aad90c45a91f86ad3d3c9c3588).
>
> ==================================================================
> BUG: KASAN: null-ptr-deref in smc_ioctl+0x5c5/0x7a0 net/smc/af_smc.c:1536
> Read of size 4 at addr 0000000000000020 by task syz-executor0/5046
>
> CPU: 0 PID: 5046 Comm: syz-executor0 Not tainted 4.18.0-rc3 #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x18f/0x26c lib/dump_stack.c:113
>  kasan_report_error mm/kasan/report.c:352 [inline]
>  kasan_report.cold.7+0x13b/0x2f5 mm/kasan/report.c:412
>  check_memory_region_inline mm/kasan/kasan.c:260 [inline]
>  __asan_load4+0x78/0x80 mm/kasan/kasan.c:698
>  smc_ioctl+0x5c5/0x7a0 net/smc/af_smc.c:1536
>  sock_do_ioctl+0xcc/0x380 net/socket.c:969
>  sock_ioctl+0x2bd/0x5a0 net/socket.c:1093
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  file_ioctl fs/ioctl.c:500 [inline]
>  do_vfs_ioctl+0x188/0xf80 fs/ioctl.c:684
>  ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
>  __do_sys_ioctl fs/ioctl.c:708 [inline]
>  __se_sys_ioctl fs/ioctl.c:706 [inline]
>  __x64_sys_ioctl+0x43/0x50 fs/ioctl.c:706
>  do_syscall_64+0x182/0x540 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x44b939
> Code: 8d 6b fc ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 5b 6b fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007f6708050b48 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 000000000071bee0 RCX: 000000000044b939
> RDX: 0000000020000180 RSI: 0000000000008905 RDI: 0000000000000015
> RBP: 00000000000066a8 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007f67080516d4
> R13: 00000000ffffffff R14: 00000000006f2748 R15: 0000000000000001
> ==================================================================
>
> = About RaceFuzzer
>
> RaceFuzzer is a customized version of Syzkaller, specifically tailored
> to find race condition bugs in the Linux kernel. While we leverage
> many different technique, the notable feature of RaceFuzzer is in
> leveraging a custom hypervisor (QEMU/KVM) to interleave the
> scheduling. In particular, we modified the hypervisor to intentionally
> stall a per-core execution, which is similar to supporting per-core
> breakpoint functionality. This allows RaceFuzzer to force the kernel
> to deterministically trigger racy condition (which may rarely happen
> in practice due to randomness in scheduling).
>
> RaceFuzzer's C repro always pinpoints two racy syscalls. Since C
> repro's scheduling synchronization should be performed at the user
> space, its reproducibility is limited (reproduction may take from 1
> second to 10 minutes (or even more), depending on a bug). This is
> because, while RaceFuzzer precisely interleaves the scheduling at the
> kernel's instruction level when finding this bug, C repro cannot fully
> utilize such a feature. Please disregard all code related to
> "should_hypercall" in the C repro, as this is only for our debugging
> purposes using our own hypervisor.

^ permalink raw reply

* [PATCH net-next] liquidio: fix hang when re-binding VF host drv after running DPDK VF driver
From: Felix Manlunas @ 2018-07-13 19:50 UTC (permalink / raw)
  To: davem
  Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
	felix.manlunas, ricardo.farrington

From: Rick Farrington <ricardo.farrington@cavium.com>

When configuring SLI_PKTn_OUTPUT_CONTROL, VF driver was assuming that IPTR
mode was disabled by reset, which was not true.  Since DPDK driver had
set IPTR mode previously, the VF driver (which uses buf-ptr-only mode) was
not properly handling DROQ packets (i.e. it saw zero-length packets).

This represented an invalid hardware configuration which the driver could
not handle.

Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
 drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c | 3 +++
 drivers/net/ethernet/cavium/liquidio/cn23xx_vf_device.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c b/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
index 929d485..e088ded 100644
--- a/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
@@ -493,6 +493,9 @@ static void cn23xx_pf_setup_global_output_regs(struct octeon_device *oct)
 	for (q_no = srn; q_no < ern; q_no++) {
 		reg_val = octeon_read_csr(oct, CN23XX_SLI_OQ_PKT_CONTROL(q_no));
 
+		/* clear IPTR */
+		reg_val &= ~CN23XX_PKT_OUTPUT_CTL_IPTR;
+
 		/* set DPTR */
 		reg_val |= CN23XX_PKT_OUTPUT_CTL_DPTR;
 
diff --git a/drivers/net/ethernet/cavium/liquidio/cn23xx_vf_device.c b/drivers/net/ethernet/cavium/liquidio/cn23xx_vf_device.c
index 9338a00..1f8b7f6 100644
--- a/drivers/net/ethernet/cavium/liquidio/cn23xx_vf_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/cn23xx_vf_device.c
@@ -165,6 +165,9 @@ static void cn23xx_vf_setup_global_output_regs(struct octeon_device *oct)
 		reg_val =
 		    octeon_read_csr(oct, CN23XX_VF_SLI_OQ_PKT_CONTROL(q_no));
 
+		/* clear IPTR */
+		reg_val &= ~CN23XX_PKT_OUTPUT_CTL_IPTR;
+
 		/* set DPTR */
 		reg_val |= CN23XX_PKT_OUTPUT_CTL_DPTR;
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v2 bpf 2/5] bpf: fix rcu annotations in compute_effective_progs()
From: Roman Gushchin @ 2018-07-13 19:41 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel-team, Roman Gushchin, Alexei Starovoitov,
	Daniel Borkmann
In-Reply-To: <20180713194114.2711-1-guro@fb.com>

The progs local variable in compute_effective_progs() is marked
as __rcu, which is not correct. This is a local pointer, which
is initialized by bpf_prog_array_alloc(), which also now
returns a generic non-rcu pointer.

The real rcu-protected pointer is *array (array is a pointer
to an RCU-protected pointer), so the assignment should be performed
using rcu_assign_pointer().

Fixes: 324bda9e6c5a ("bpf: multi program support for cgroup+bpf")
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
 kernel/bpf/cgroup.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 3d83ee7df381..badabb0b435c 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -95,7 +95,7 @@ static int compute_effective_progs(struct cgroup *cgrp,
 				   enum bpf_attach_type type,
 				   struct bpf_prog_array __rcu **array)
 {
-	struct bpf_prog_array __rcu *progs;
+	struct bpf_prog_array *progs;
 	struct bpf_prog_list *pl;
 	struct cgroup *p = cgrp;
 	int cnt = 0;
@@ -120,13 +120,12 @@ static int compute_effective_progs(struct cgroup *cgrp,
 					    &p->bpf.progs[type], node) {
 				if (!pl->prog)
 					continue;
-				rcu_dereference_protected(progs, 1)->
-					progs[cnt++] = pl->prog;
+				progs->progs[cnt++] = pl->prog;
 			}
 		p = cgroup_parent(p);
 	} while (p);
 
-	*array = progs;
+	rcu_assign_pointer(*array, progs);
 	return 0;
 }
 
-- 
2.14.4

^ permalink raw reply related

* [PATCH v2 bpf 5/5] bpf: add missing rcu_dereference() in bpf_prog_array_copy()
From: Roman Gushchin @ 2018-07-13 19:41 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel-team, Roman Gushchin, Alexei Starovoitov,
	Daniel Borkmann
In-Reply-To: <20180713194114.2711-1-guro@fb.com>

The old_array argument in bpf_prog_array_copy() is marked as __rcu,
so the dereferencing should be performed using rcu_dereference().
As we do this a couple of times, and we want to be sure,
that we copy a single array, let's safe the result of dereferencing
in a local variable and use it further.

This fixes the following sparse warnings:
kernel/bpf/core.c:1653:31: warning: incorrect type in assignment (different address spaces)
kernel/bpf/core.c:1681:15: warning: incorrect type in assignment (different address spaces)
kernel/bpf/core.c:1687:31: warning: incorrect type in assignment (different address spaces)

Fixes: e87c6bc3852b ("bpf: permit multiple bpf attachments
for a single perf event")
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
 include/linux/bpf.h | 2 +-
 kernel/bpf/core.c   | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 329026baef6e..3cfc8095d2e0 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -363,7 +363,7 @@ void bpf_prog_array_delete_safe(struct bpf_prog_array __rcu *progs,
 int bpf_prog_array_copy_info(struct bpf_prog_array __rcu *array,
 			     u32 *prog_ids, u32 request_cnt,
 			     u32 *prog_cnt);
-int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
+int bpf_prog_array_copy(struct bpf_prog_array __rcu *__old_array,
 			struct bpf_prog *exclude_prog,
 			struct bpf_prog *include_prog,
 			struct bpf_prog_array **new_array);
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 722ae6913dc0..26bdc99fc807 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1634,11 +1634,12 @@ void bpf_prog_array_delete_safe(struct bpf_prog_array __rcu *progs,
 		}
 }
 
-int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
+int bpf_prog_array_copy(struct bpf_prog_array __rcu *__old_array,
 			struct bpf_prog *exclude_prog,
 			struct bpf_prog *include_prog,
 			struct bpf_prog_array **new_array)
 {
+	struct bpf_prog_array *old_array = rcu_dereference(__old_array);
 	int new_prog_cnt, carry_prog_cnt = 0;
 	struct bpf_prog **existing_prog;
 	struct bpf_prog_array *array;
-- 
2.14.4

^ permalink raw reply related

* [PATCH v2 bpf 4/5] bpf: add missing rcu_dereference() in bpf_prog_array_delete_safe()
From: Roman Gushchin @ 2018-07-13 19:41 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel-team, Roman Gushchin, Alexei Starovoitov,
	Daniel Borkmann
In-Reply-To: <20180713194114.2711-1-guro@fb.com>

There is a missing rcu_dereference() in bpf_prog_array_delete_safe().
The progs argument is a __rcu pointer, so dereferencing should be
performed using rcu_dereference(), as, for example, in
bpf_prog_array_length().

This patch helps to remove the following sparse warning:
kernel/bpf/core.c:1629:34: warning: incorrect type in initializer (different address spaces)

Fixes: 324bda9e6c5a ("bpf: multi program support for cgroup+bpf")
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
 kernel/bpf/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index fdf961f70deb..722ae6913dc0 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1625,7 +1625,7 @@ int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
 void bpf_prog_array_delete_safe(struct bpf_prog_array __rcu *progs,
 				struct bpf_prog *old_prog)
 {
-	struct bpf_prog **prog = progs->progs;
+	struct bpf_prog **prog = rcu_dereference(progs)->progs;
 
 	for (; *prog; prog++)
 		if (*prog == old_prog) {
-- 
2.14.4

^ permalink raw reply related

* [PATCH v2 bpf 3/5] bpf: bpf_prog_array_free() should take a generic non-rcu pointer
From: Roman Gushchin @ 2018-07-13 19:41 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel-team, Roman Gushchin, Alexei Starovoitov,
	Daniel Borkmann
In-Reply-To: <20180713194114.2711-1-guro@fb.com>

bpf_prog_array_free() should take a generic non-rcu pointer
as an argument, as freeing the objects assumes that we're
holding an exclusive rights on it.

rcu_access_pointer() can be used to convert a __rcu pointer to
a generic pointer before passing it to bpf_prog_array_free(),
if necessary.

This patch eliminates the following sparse warning:
kernel/bpf/core.c:1556:9: warning: incorrect type in argument 1 (different address spaces)
kernel/bpf/core.c:1556:9:    expected struct callback_head *head
kernel/bpf/core.c:1556:9:    got struct callback_head [noderef] <asn:4>*<noident>

Fixes: 324bda9e6c5a ("bpf: multi program support for cgroup+bpf")
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
 drivers/media/rc/bpf-lirc.c |  6 +++---
 include/linux/bpf.h         |  2 +-
 kernel/bpf/cgroup.c         | 11 ++++++-----
 kernel/bpf/core.c           |  5 ++---
 kernel/trace/bpf_trace.c    |  8 ++++----
 5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/media/rc/bpf-lirc.c b/drivers/media/rc/bpf-lirc.c
index fcfab6635f9c..509b262aa0dc 100644
--- a/drivers/media/rc/bpf-lirc.c
+++ b/drivers/media/rc/bpf-lirc.c
@@ -135,7 +135,7 @@ static int lirc_bpf_attach(struct rc_dev *rcdev, struct bpf_prog *prog)
 		goto unlock;
 
 	rcu_assign_pointer(raw->progs, new_array);
-	bpf_prog_array_free(old_array);
+	bpf_prog_array_free(rcu_access_pointer(old_array));
 
 unlock:
 	mutex_unlock(&ir_raw_handler_lock);
@@ -173,7 +173,7 @@ static int lirc_bpf_detach(struct rc_dev *rcdev, struct bpf_prog *prog)
 		goto unlock;
 
 	rcu_assign_pointer(raw->progs, new_array);
-	bpf_prog_array_free(old_array);
+	bpf_prog_array_free(rcu_access_pointer(old_array));
 unlock:
 	mutex_unlock(&ir_raw_handler_lock);
 	return ret;
@@ -204,7 +204,7 @@ void lirc_bpf_free(struct rc_dev *rcdev)
 	while (*progs)
 		bpf_prog_put(*progs++);
 
-	bpf_prog_array_free(rcdev->raw->progs);
+	bpf_prog_array_free(rcu_access_pointer(rcdev->raw->progs));
 }
 
 int lirc_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 943fb08d8287..329026baef6e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -353,7 +353,7 @@ struct bpf_prog_array {
 };
 
 struct bpf_prog_array *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags);
-void bpf_prog_array_free(struct bpf_prog_array __rcu *progs);
+void bpf_prog_array_free(struct bpf_prog_array *progs);
 int bpf_prog_array_length(struct bpf_prog_array __rcu *progs);
 int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
 				__u32 __user *prog_ids, u32 cnt);
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index badabb0b435c..9ac0c9b51d0d 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -37,7 +37,8 @@ void cgroup_bpf_put(struct cgroup *cgrp)
 			kfree(pl);
 			static_branch_dec(&cgroup_bpf_enabled_key);
 		}
-		bpf_prog_array_free(cgrp->bpf.effective[type]);
+		bpf_prog_array_free(rcu_access_pointer(
+					    cgrp->bpf.effective[type]));
 	}
 }
 
@@ -139,7 +140,7 @@ static void activate_effective_progs(struct cgroup *cgrp,
 	/* free prog array after grace period, since __cgroup_bpf_run_*()
 	 * might be still walking the array
 	 */
-	bpf_prog_array_free(old_array);
+	bpf_prog_array_free(rcu_access_pointer(old_array));
 }
 
 /**
@@ -168,7 +169,7 @@ int cgroup_bpf_inherit(struct cgroup *cgrp)
 	return 0;
 cleanup:
 	for (i = 0; i < NR; i++)
-		bpf_prog_array_free(arrays[i]);
+		bpf_prog_array_free(rcu_access_pointer(arrays[i]));
 	return -ENOMEM;
 }
 
@@ -270,7 +271,7 @@ int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
 	css_for_each_descendant_pre(css, &cgrp->self) {
 		struct cgroup *desc = container_of(css, struct cgroup, self);
 
-		bpf_prog_array_free(desc->bpf.inactive);
+		bpf_prog_array_free(rcu_access_pointer(desc->bpf.inactive));
 		desc->bpf.inactive = NULL;
 	}
 
@@ -372,7 +373,7 @@ int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
 	css_for_each_descendant_pre(css, &cgrp->self) {
 		struct cgroup *desc = container_of(css, struct cgroup, self);
 
-		bpf_prog_array_free(desc->bpf.inactive);
+		bpf_prog_array_free(rcu_access_pointer(desc->bpf.inactive));
 		desc->bpf.inactive = NULL;
 	}
 
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 253aa8e79c7b..fdf961f70deb 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1548,10 +1548,9 @@ struct bpf_prog_array *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags)
 	return &empty_prog_array.hdr;
 }
 
-void bpf_prog_array_free(struct bpf_prog_array __rcu *progs)
+void bpf_prog_array_free(struct bpf_prog_array *progs)
 {
-	if (!progs ||
-	    progs == (struct bpf_prog_array __rcu *)&empty_prog_array.hdr)
+	if (!progs || progs == &empty_prog_array.hdr)
 		return;
 	kfree_rcu(progs, rcu);
 }
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 0ae6829804bc..5dace25d3088 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -995,8 +995,8 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
 
 	/* set the new array to event->tp_event and set event->prog */
 	event->prog = prog;
-	rcu_assign_pointer(event->tp_event->prog_array, new_array);
-	bpf_prog_array_free(old_array);
+	rcu_swap_protected(event->tp_event->prog_array, new_array, 1);
+	bpf_prog_array_free(new_array);
 
 unlock:
 	mutex_unlock(&bpf_event_mutex);
@@ -1021,8 +1021,8 @@ void perf_event_detach_bpf_prog(struct perf_event *event)
 	if (ret < 0) {
 		bpf_prog_array_delete_safe(old_array, event->prog);
 	} else {
-		rcu_assign_pointer(event->tp_event->prog_array, new_array);
-		bpf_prog_array_free(old_array);
+		rcu_swap_protected(event->tp_event->prog_array, new_array, 1);
+		bpf_prog_array_free(new_array);
 	}
 
 	bpf_prog_put(event->prog);
-- 
2.14.4

^ permalink raw reply related

* [PATCH v2 bpf 1/5] bpf: bpf_prog_array_alloc() should return a generic non-rcu pointer
From: Roman Gushchin @ 2018-07-13 19:41 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel-team, Roman Gushchin, Alexei Starovoitov,
	Daniel Borkmann

Currently the return type of the bpf_prog_array_alloc() is
struct bpf_prog_array __rcu *, which is not quite correct.
Obviously, the returned pointer is a generic pointer, which
is valid for an indefinite amount of time and it's not shared
with anyone else, so there is no sense in marking it as __rcu.

This change eliminate the following sparse warnings:
kernel/bpf/core.c:1544:31: warning: incorrect type in return expression (different address spaces)
kernel/bpf/core.c:1544:31:    expected struct bpf_prog_array [noderef] <asn:4>*
kernel/bpf/core.c:1544:31:    got void *
kernel/bpf/core.c:1548:17: warning: incorrect type in return expression (different address spaces)
kernel/bpf/core.c:1548:17:    expected struct bpf_prog_array [noderef] <asn:4>*
kernel/bpf/core.c:1548:17:    got struct bpf_prog_array *<noident>
kernel/bpf/core.c:1681:15: warning: incorrect type in assignment (different address spaces)
kernel/bpf/core.c:1681:15:    expected struct bpf_prog_array *array
kernel/bpf/core.c:1681:15:    got struct bpf_prog_array [noderef] <asn:4>*

Fixes: 324bda9e6c5a ("bpf: multi program support for cgroup+bpf")
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
 include/linux/bpf.h | 2 +-
 kernel/bpf/core.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 8827e797ff97..943fb08d8287 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -352,7 +352,7 @@ struct bpf_prog_array {
 	struct bpf_prog *progs[0];
 };
 
-struct bpf_prog_array __rcu *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags);
+struct bpf_prog_array *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags);
 void bpf_prog_array_free(struct bpf_prog_array __rcu *progs);
 int bpf_prog_array_length(struct bpf_prog_array __rcu *progs);
 int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 1e5625d46414..253aa8e79c7b 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1538,7 +1538,7 @@ static struct {
 	.null_prog = NULL,
 };
 
-struct bpf_prog_array __rcu *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags)
+struct bpf_prog_array *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags)
 {
 	if (prog_cnt)
 		return kzalloc(sizeof(struct bpf_prog_array) +
-- 
2.14.4

^ permalink raw reply related

* Re: [PATCH net-next v2 3/7] net: mvneta: increase number of buffers in RX and TX queue
From: Russell King - ARM Linux @ 2018-07-13 19:17 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: David S. Miller, linux-kernel, netdev, Andrew Lunn, Jason Cooper,
	Antoine Tenart, Maxime Chevallier, Nadav Haklai,
	Yelena Krivosheev, Thomas Petazzoni, Miquèl Raynal,
	Marcin Wojtas, Dmitri Epshtein, linux-arm-kernel,
	Sebastian Hesselbarth
In-Reply-To: <20180713161841.11202-4-gregory.clement@bootlin.com>

On Fri, Jul 13, 2018 at 06:18:37PM +0200, Gregory CLEMENT wrote:
> From: Yelena Krivosheev <yelena@marvell.com>
> 
> The initial values were too small leading to poor performance when using
> the software buffer management.

What does this do to latency when a large transfer is also ongoing
(iow, the classic bufferbloat issue) ?

> 
> Signed-off-by: Yelena Krivosheev <yelena@marvell.com>
> [gregory: extract from a larger patch]
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
>  drivers/net/ethernet/marvell/mvneta.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index f4e3943a745d..c22df28b07c8 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -295,10 +295,10 @@
>  #define MVNETA_RSS_LU_TABLE_SIZE	1
>  
>  /* Max number of Rx descriptors */
> -#define MVNETA_MAX_RXD 128
> +#define MVNETA_MAX_RXD 512
>  
>  /* Max number of Tx descriptors */
> -#define MVNETA_MAX_TXD 532
> +#define MVNETA_MAX_TXD 1024
>  
>  /* Max number of allowed TCP segments for software TSO */
>  #define MVNETA_MAX_TSO_SEGS 100
> -- 
> 2.18.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up

^ permalink raw reply

* Re: MACsec hardware offloading
From: Antoine Tenart @ 2018-07-13 18:52 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Antoine Tenart, davem, sd, f.fainelli, thomas.petazzoni,
	alexandre.belloni, allan.nielsen, netdev, jiri
In-Reply-To: <20180713162002.GA4314@lunn.ch>

Hi Andrew,

On Fri, Jul 13, 2018 at 06:20:02PM +0200, Andrew Lunn wrote:
> On Fri, Jul 13, 2018 at 04:46:08PM +0200, Antoine Tenart wrote:
> 
> > One important point about adding MACsec offloading in Linux is this can
> > be done in either the MAC or the PHY. While I'll be working on making
> > this work in a PHY, I know for sure some MAC do have the same capability
> > (including for example the Intel ixgbe NIC).
> 
> I see in your current code, you check if the netdev has a phydev, and
> if the phydev supports macsec, and then go straight to the phy. That
> means there is no need to modify the MAC driver, it should just work.
> If not, you call the MAC.

Yes. I'm just not sure if the PHY implementation should have precedence
over the MAC one, or the opposite, or if the user should be able to
select the provider to use.

> I would add support for the PHY to return -EOPNOTSUPP and then fall
> back to trying the MAC.

Good idea!

> Also, your current checks are
> inconsistent. macsec_hw_offload_capable() checks for
> phydev->drv->macsec, where as macsec_hw_offload() just checks for
> dev->real_dev->phydev->drv. It looks like
> dev->real_dev->phydev->drv->macsec could be a NULL pointer and bad
> things happen.

OK, I'll be careful and fix this.

> For switchdev, when we offload to a switch, the switch nearly always
> has the option to say it could not accept the offload. We then fall
> back to performing the needed action in software. At the moment, i
> don't see you checking the return code of macsec_hw_offload(). So it
> is not clear to me if this software fallback works.

Right, that should be supported. Although I'm not sure all offloading
helpers can have a fallback (especially if they depend on a previous
one), but I'll try to have this kind of behaviour in place whenever
possible. Or maybe it'll just work for all helpers, as the MACsec state
will always be kept in the software implementation part.

> The Switchdev API is also transaction based, with a prepare and a
> commit phase. All resource allocation happens in the prepare phase and
> at this stage, you can return errors indicating offload is not
> possible. The commit phase is not allowed to fail. You probably want
> to go look at the mailing list archive and look at why this
> architecture was decided on.

Thanks for the hint, I'll have a look at this design pattern.

> Maybe the MAC part of this should actually use the switchdev API?  You
> then get a lot of infrastructure for free.

I'll have a look at this as well. With the prepare/commit logic already
in place, it could be easier to implement.

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH 11/18] wireless/ti: change strncpy+truncation to strlcpy
From: Rustad, Mark D @ 2018-07-13 18:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dominique Martinet, Kalle Valo, David S. Miller, Thomas Gleixner,
	Kate Stewart, Philippe Ombredanne, Joe Perches,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20180713073810.GA31984@kroah.com>

[-- Attachment #1: Type: text/plain, Size: 617 bytes --]

On Jul 13, 2018, at 12:38 AM, Greg Kroah-Hartman  
<gregkh@linuxfoundation.org> wrote:

> On Fri, Jul 13, 2018 at 03:25:49AM +0200, Dominique Martinet wrote:
>> Generated by scripts/coccinelle/misc/strncpy_truncation.cocci
>>
>> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
>
> I don't know about other maintainers, but I know I wouldn't take such a
> horrid changelog description as this :)
>
> good luck!
>
> greg k-h

I would be very concerned about the potential for information leak, because  
of the different behavior of the two functions.

--
Mark Rustad, Networking Division, Intel Corporation

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply

* Re: [PATCH v3 net-next] net/sched: add skbprio scheduler
From: Cong Wang @ 2018-07-13 18:26 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Michel Machado, Nishanth Devarajan, Jamal Hadi Salim, Jiri Pirko,
	David Miller, Linux Kernel Network Developers, Cody Doucette
In-Reply-To: <20180713130452.GJ8880@localhost.localdomain>

On Fri, Jul 13, 2018 at 6:04 AM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Thu, Jul 12, 2018 at 11:05:45PM -0700, Cong Wang wrote:
> > On Wed, Jul 11, 2018 at 12:33 PM Marcelo Ricardo Leitner
> > <marcelo.leitner@gmail.com> wrote:
> > >
> > > On Tue, Jul 10, 2018 at 07:25:53PM -0700, Cong Wang wrote:
> > > > On Mon, Jul 9, 2018 at 2:40 PM Marcelo Ricardo Leitner
> > > > <marcelo.leitner@gmail.com> wrote:
> > > > >
> > > > > On Mon, Jul 09, 2018 at 05:03:31PM -0400, Michel Machado wrote:
> > > > > >    Changing TC_PRIO_MAX from 15 to 63 risks breaking backward compatibility
> > > > > > with applications.
> > > > >
> > > > > If done, it needs to be done carefully, indeed. I don't know if it's
> > > > > doable, neither I know how hard is your requirement for 64 different
> > > > > priorities.
> > > >
> > > > struct tc_prio_qopt {
> > > >         int     bands;                  /* Number of bands */
> > > >         __u8    priomap[TC_PRIO_MAX+1]; /* Map: logical priority -> PRIO band */
> > > > };
> > > >
> > > > How would you do it carefully?
> > >
> > > quick shot, multiplex v1 and v2 formats based on bands and sizeof():
> > >
> > > #define TCQ_PRIO_BANDS_V1       16
> > > #define TCQ_PRIO_BANDS_V2       64
> > > #define TC_PRIO_MAX_V2          64
> > >
> > > struct tc_prio_qopt_v2 {
> > >         int     bands;                  /* Number of bands */
> > >         __u8    priomap[TC_PRIO_MAX_V2+1]; /* Map: logical priority -> PRIO band */
> > > };
> > >
> >
> > Good try, but:
> >
> > 1. You don't take padding into account, although the difference
> > between 16 and 64 is big here. If it were 16 and 20, almost certainly
> > wouldn't work.
>
> It still would work, no matter how much padding you have, as currently
> you can't use more than 3 bands.

I am lost.

With your proposal above, you have 16 bands for V1 and 64 bands
for V2, where does 3 come from???


>
> >
> > 2. What if I compile a new iproute2 on an old kernel? The iproute2
> > will use V2, while old kernel has no knowledge of V2, so it only
> > copies a part of V2 in the end....
>
> Yes, and that's not a problem:
> - Either bands is > 3 and it will return EINVAL, protecting from
>   reading beyond the buffer.
> - Or 2 <= bands <= 3 and it will handle it as a _v1 struct, and use
>   only the original size.

Again why 3 not 16 or 64 ??

Also, why does an old kernel has the logic in its binary to determine
this?

>
> iproute2 (or other app) may still use _v1 if it wants, btw.

Yes, old iproute2 must still have v1, what's point? Are you
suggesting new iproute2 should still have v1 after you propose
v1 and v2 for kernel?

I must seriously miss something. Please help.

Thanks!

^ permalink raw reply

* Re: [PATCH v3 net-next] net/sched: add skbprio scheduler
From: Cong Wang @ 2018-07-13 18:17 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Michel Machado, Nishanth Devarajan, Jamal Hadi Salim, Jiri Pirko,
	David Miller, Linux Kernel Network Developers, Cody Doucette
In-Reply-To: <20180713130012.GI8880@localhost.localdomain>

On Fri, Jul 13, 2018 at 6:00 AM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Thu, Jul 12, 2018 at 10:07:30PM -0700, Cong Wang wrote:
> > On Wed, Jul 11, 2018 at 11:37 AM Marcelo Ricardo Leitner
> > <marcelo.leitner@gmail.com> wrote:
> > >
> > > On Tue, Jul 10, 2018 at 07:32:43PM -0700, Cong Wang wrote:
> > > > On Mon, Jul 9, 2018 at 12:53 PM Marcelo Ricardo Leitner
> > > > <marcelo.leitner@gmail.com> wrote:
> > > > >
> > > > > On Mon, Jul 09, 2018 at 02:18:33PM -0400, Michel Machado wrote:
> > > > > >
> > > > > >    2. sch_prio.c does not have a global limit on the number of packets on
> > > > > > all its queues, only a limit per queue.
> > > > >
> > > > > It can be useful to sch_prio.c as well, why not?
> > > > > prio_enqueue()
> > > > > {
> > > > > ...
> > > > > +       if (count > sch->global_limit)
> > > > > +               prio_tail_drop(sch);   /* to be implemented */
> > > > >         ret = qdisc_enqueue(skb, qdisc, to_free);
> > > > >
> > > >
> > > > Isn't the whole point of sch_prio offloading the queueing to
> > > > each class? If you need a limit, there is one for each child
> > > > qdisc if you use for example pfifo or bfifo (depending on you
> > > > want to limit bytes or packets).
> > >
> > > Yes, but Michel wants to drop from other lower priorities if needed,
> > > and that's not possible if you handle the limit already in a child
> > > qdisc as they don't know about their siblings. The idea in the example
> > > above is to discard it from whatever lower priority is needed, then
> > > queue it. (ok, the example missed to check the priority level)
> >
> > So it disproves your point of adding a flag to sch_prio, right?
>
> I don't see how?

Interesting, you said "Michel wants to drop from other lower
priorities if needed", but sch_prio has no knowledge of this,
you confirmed with "...if you handle the limit already in a child
qdisc as they don't know about their siblings."

The if clause is true as the limit is indeed handled by its child
qdiscs as designed.

Therefore, a simple of adding a flag to sch_prio, as you
suggested and demonstrated above, doesn't work, as
confirmed by your own words.

What am I missing here?

Are you go further by suggesting moving the limit out of prio?
Or are you going to expand your definition of "adding a flag"?
Perhaps two flags? :)

I am very open for discussion to see how far we can go.

>
> >
> > Also, you have to re-introduce qdisc->ops->drop() if you really want
> > to go this direction.
>
> Again, yes. What's the deal with it?
>

Nothing, just want to tell you ops->drop() is nothing new, to help
your discussion.


> >
> > >
> > > As for the different units, sch_prio holds a count of how many packets
> > > are queued on its children, and that's what would be used for the limit.
> > >
> > > >
> > > > Also, what's your plan for backward compatibility here?
> > >
> > > say:
> > >   if (sch->global_limit && count > sch->global_limit)
> > > as in, only do the limit check/enforcing if needed.
> >
> > Obviously doesn't work, users could pass 0 to effectively
> > disable the qdisc from enqueue'ing any packet.
>
> If you only had considered the right 'limit' variable, you would be
> right here.

Yeah, that is exactly what you propose, isn't it? :)

Thanks!

^ permalink raw reply

* Re: [net-next PATCH] net: ipv4: fix listify ip_rcv_finish in case of forwarding
From: Eric Dumazet @ 2018-07-13 18:14 UTC (permalink / raw)
  To: Edward Cree, Or Gerlitz, Jesper Dangaard Brouer
  Cc: Saeed Mahameed, netdev@vger.kernel.org
In-Reply-To: <3d08d6ae-a4cc-f9ad-f752-ba66ca13240b@solarflare.com>



On 07/13/2018 07:19 AM, Edward Cree wrote:
> On 12/07/18 21:10, Or Gerlitz wrote:
>> On Wed, Jul 11, 2018 at 11:06 PM, Jesper Dangaard Brouer
>> <brouer@redhat.com> wrote:
>>> One reason I didn't "just" send a patch, is that Edward so-fare only
>>> implemented netif_receive_skb_list() and not napi_gro_receive_list().
>> sfc does't support gro?! doesn't make sense.. Edward?
> sfc has a flag EFX_RX_PKT_TCP set according to bits in the RX event, we
>  call napi_{get,gro}_frags() (via efx_rx_packet_gro()) for TCP packets and
>  netif_receive_skb() (or now the list handling) (via efx_rx_deliver()) for
>  non-TCP packets.  So we avoid the GRO overhead for non-TCP workloads.
> 
>> Same TCP performance
>>
>> with GRO and no rx-batching
>>
>> or
>>
>> without GRO and yes rx-batching
>>
>> is by far not intuitive result
> I'm also surprised by this.  If I can find the time I'll try to do similar
>  experiments on sfc.
> Jesper, are the CPU utilisations similar in both cases?  You're sure your
>  stream isn't TX-limited?

1) Make sure to test the case where packets of X flows are interleaved on the wire,
instead of being nice with the receiver (trains of packets for each flow)

(Typical case on a fabric, since switches will mix the ingress traffic to one egress port)

2) Do not test TCP_STREAM traffic, but TCP_RR
(RPC like traffic where GRO really cuts down number of ACK packets)

  TCP_STREAM can hide the GRO gain, since ACK are naturally decimated under sufficient
  load.

^ permalink raw reply

* Re: [PATCH bpf-next 0/7] xdp: simultaneous driver and HW XDP
From: Alexei Starovoitov @ 2018-07-13 18:08 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: daniel, oss-drivers, netdev
In-Reply-To: <20180712033644.23954-1-jakub.kicinski@netronome.com>

On Wed, Jul 11, 2018 at 08:36:37PM -0700, Jakub Kicinski wrote:
> Hi!
> 
> This set is adding support for loading driver and offload XDP
> at the same time.  This enables advanced use cases where some
> of the work is offloaded to the NIC and some is done by the host.
> Separate netlink attributes are added for each mode of operation.
> Driver callbacks for offload are cleaned up a little, including
> removal of .prog_attached flag.

for the set:
Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [PATCH] scripts/tags.sh: Add BPF_CALL
From: Alexei Starovoitov @ 2018-07-13 18:10 UTC (permalink / raw)
  To: Constantine Shulyupin
  Cc: ast, daniel, netdev, Andrew Morton, Vlastimil Babka,
	Arend van Spriel, Joey Pabalinas, Kirill A. Shutemov,
	Matthew Wilcox, open list
In-Reply-To: <20180712052850.28396-1-const@MakeLinux.com>

On Thu, Jul 12, 2018 at 08:28:46AM +0300, Constantine Shulyupin wrote:
> Signed-off-by: Constantine Shulyupin <const@MakeLinux.com>

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [PATCH v4 08/18] net: davinci_emac: potentially get the MAC address from MTD
From: Bartosz Golaszewski @ 2018-07-13 18:00 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Ladislav Michl, Florian Fainelli, Kevin Hilman, Russell King,
	Grygorii Strashko, David S . Miller, Srinivas Kandagatla,
	Lukas Wunner, Rob Herring, Dan Carpenter, Ivan Khoronzhuk,
	David Lechner, Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet,
	Linux ARM, Linux Kernel Mailing List, linux-omap, netdev,
	Bartosz 
In-Reply-To: <c0ffc590-8064-3b02-9b5c-fcc24628d390@ti.com>

2018-07-04 11:04 GMT+02:00 Sekhar Nori <nsekhar@ti.com>:
> On Wednesday 04 July 2018 01:59 PM, Bartosz Golaszewski wrote:
>> 2018-07-04 9:09 GMT+02:00 Ladislav Michl <ladis@linux-mips.org>:
>>> On Tue, Jul 03, 2018 at 09:39:51AM -0700, Florian Fainelli wrote:
>>>>
>>>>
>>>> On 06/29/2018 02:40 AM, Bartosz Golaszewski wrote:
>>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>
>>>>> On da850-evm board we can read the MAC address from MTD. It's currently
>>>>> done in the relevant board file, but we want to get rid of all the MAC
>>>>> reading callbacks from the board file (SPI and NAND). Move the reading
>>>>> of the MAC address from SPI to the emac driver's probe function.
>>>>
>>>> This should be made something generic to all drivers, not just something
>>>> the davinci_emac driver does, something like this actually:
>>>>
>>>> https://lkml.org/lkml/2018/3/24/312
>>>
>>> ...and that's would also make it work when MAC address is stored
>>> in 24c08 EEPROM, which is quite common.
>>>
>>
>> This is what the second patch for davinci_emac in this series does. I
>> agree that this should become more generic at some point - we should
>> probably have a routine somewhere in net that would try to get the MAC
>> address from all possible sources (nvmem, of etc.). This is somewhat
>> related to the work I want to do on nvmem to make the at24 setup()
>> callback more generic.
>>
>> Unfortunately we don't have it yet and I will not have time to work on
>> it before v4.20 so if there are no serious objections, I'd like to get
>> this series merged for v4.19 and then we can refactor the MAC reading
>> later.
>>
>> How does it sound?
>
> I don't think the series introduces any regressions. We need to have MTD
> and SPI flash built into the kernel even today to get mac address on
> DA850 EVM. So from that perspective, I don't have objections (I need to
> actually test still).
>
> OTOH, it will be nice to do the conversion once and not piecemeal. That
> way there is less churn and scope for regressions.
>
> So from a mach-davinci perspective, I don't have a very strong position
> either way.
>
> Thanks,
> Sekhar

We're getting close to rc5 so I'd like to make a case for this series again.

I understand that there's more to do than just the changes introduced
here, but we shouldn't try to fix several problems in many different
places at once. There's just too many moving pieces. I'd rather start
merging small improvements right away.

The idea behind this series is to remove (almost) all users of
at24_platform_data. The davinci_emac patches are there only because we
need to remove some MAC adress reading stuff from the board files.
Having this code there and calling it back from EEPROM/MTD drivers is
already wrong and we should work towards using nvmem for that anyway.

Currently for MTD the nvmem support series seems to be dead and it's
going to take some time before anything gets upstream.

So I'd like to again ask you to consider picking up the patches from
this series to your respective trees or at the very least: I'd like to
ask Srinivas to pick up the nvmem patches and Sekhar to take the
first, non-controversial batch of davinci platform changes so that
we'll have less code to carry for the next release.

Best regards,
Bartosz Golaszewski

^ permalink raw reply

* imaging solutions for you
From: Simon @ 2018-07-13 12:16 UTC (permalink / raw)
  To: netdev

We are a team, we can process 300+ images per day for you.

If you need any image editing, please let us know.

Photos cut out;
Photos clipping path;
Photos masking;
Photo shadow creation;
Photos retouching;
Beauty Model retouching on skin, face, body;
Glamour retouching;
Products retouching.

We can give you editing test on your photos.

Turnaround time fast
7/24/365 available

Thanks,
Simon

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox