From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3DD1AC282C3 for ; Thu, 24 Jan 2019 13:08:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 14EFC205C9 for ; Thu, 24 Jan 2019 13:08:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727826AbfAXNIr (ORCPT ); Thu, 24 Jan 2019 08:08:47 -0500 Received: from www62.your-server.de ([213.133.104.62]:41894 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726105AbfAXNIq (ORCPT ); Thu, 24 Jan 2019 08:08:46 -0500 Received: from [88.198.220.132] (helo=sslproxy03.your-server.de) by www62.your-server.de with esmtpsa (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89_1) (envelope-from ) id 1gmekO-0001aV-1J; Thu, 24 Jan 2019 14:08:44 +0100 Received: from [178.197.249.22] (helo=linux.home) by sslproxy03.your-server.de with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) (envelope-from ) id 1gmekN-0001sy-KL; Thu, 24 Jan 2019 14:08:43 +0100 Subject: Re: [PATCH bpf-next 3/3] xsk: add sock_diag interface for AF_XDP To: =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= Cc: ast@kernel.org, Netdev , =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= , "Karlsson, Magnus" , Magnus Karlsson References: <20190118130305.11504-1-bjorn.topel@gmail.com> <20190118130305.11504-4-bjorn.topel@gmail.com> <176d1001-161c-7ce4-7ccb-8b49375c28a8@iogearbox.net> From: Daniel Borkmann Message-ID: <959824cf-4e8c-9816-af78-a768b83bf3a0@iogearbox.net> Date: Thu, 24 Jan 2019 14:08:42 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.100.2/25326/Thu Jan 24 03:30:43 2019) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 01/23/2019 03:24 PM, Björn Töpel wrote: > Den ons 23 jan. 2019 kl 14:19 skrev Daniel Borkmann : >> >> On 01/18/2019 02:03 PM, bjorn.topel@gmail.com wrote: >>> From: Björn Töpel >>> >>> This patch adds the sock_diag interface for querying sockets from user >>> space. Tools like iproute2 ss(8) can use this interface to list open >>> AF_XDP sockets. >>> >>> The user-space ABI is defined in linux/xdp_diag.h and includes netlink >>> request and response structs. The request can query sockets and the >>> response contains socket information about the rings, umems, inode and >>> more. >>> >>> Signed-off-by: Björn Töpel >> >> Series looks good, few minor nits inline: >> >>> --- >>> include/uapi/linux/xdp_diag.h | 72 +++++++++++++ >>> net/xdp/Kconfig | 8 ++ >>> net/xdp/Makefile | 1 + >>> net/xdp/xsk.c | 6 +- >>> net/xdp/xsk.h | 12 +++ >>> net/xdp/xsk_diag.c | 192 ++++++++++++++++++++++++++++++++++ >>> 6 files changed, 286 insertions(+), 5 deletions(-) >>> create mode 100644 include/uapi/linux/xdp_diag.h >>> create mode 100644 net/xdp/xsk.h >>> create mode 100644 net/xdp/xsk_diag.c >>> >>> diff --git a/include/uapi/linux/xdp_diag.h b/include/uapi/linux/xdp_diag.h >>> new file mode 100644 >>> index 000000000000..efe8ce281dce >>> --- /dev/null >>> +++ b/include/uapi/linux/xdp_diag.h >>> @@ -0,0 +1,72 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ >>> +/* >>> + * xdp_diag: interface for query/monitor XDP sockets >>> + * Copyright(c) 2019 Intel Corporation. >>> + */ >>> + >>> +#ifndef _LINUX_XDP_DIAG_H >>> +#define _LINUX_XDP_DIAG_H >>> + >>> +#include >>> + >>> +struct xdp_diag_req { >>> + __u8 sdiag_family; >>> + __u8 sdiag_protocol; >>> + __u16 pad; >> >> Presumably this one is for future use? Maybe better as '__u16 :16;' to >> avoid compile errors if someone tries to zero 'pad' member manually? > > The "pad" was there simply to have an explicitly named structure hole. > I'm not following the bitfield argument. Why does that avoid compiler > errors? Mostly in the sense that an application would explicitly set 'pad = 0' whereas pad could later on potentially be renamed and reused otherwise (which __u16 :16 would avoid in first place). But looking at other *_diag_req structs, it's explicitly named as 'pad' elsewhere already, then nevermind, lets have it rather consistent then and keep as is. One small thing I still spotted when looking at it again, in function xsk_diag_handler_dump() the req is unused. Thanks, Daniel