Message ID | 20230511151345.7529-1-larysa.zaremba@intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp4469104vqo; Thu, 11 May 2023 08:35:13 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6YDURrhf30TejUKKuSqHeKMRZwB1MbJ/aP6jzuUnaLjXWIwaoMIBm1ZIW+wc6ppf4JxsdQ X-Received: by 2002:a05:6a20:728b:b0:ff:d437:13fb with SMTP id o11-20020a056a20728b00b000ffd43713fbmr22335496pzk.49.1683819312857; Thu, 11 May 2023 08:35:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683819312; cv=none; d=google.com; s=arc-20160816; b=MmcC5oSnf1axgc7T2EIrR97bvBujZ4r7tVgk8EoGMgKf31dbbzXI3Nkq3xc+xslkl1 D9QRILzMrvEJb2qG+LfY/UNs0DVyWi24t4nuAL3KMlkHMjT1IK8diKi+2yny3adqH9Om TNzfnEMrKI0oTnB0yhyoq/JOzrX0uUGbdIOlSffsfJJCJ9FN0aPdLVUaZfJpvorVr7gC /e2zmw9Q7qpJ8sy3FgWBzl2qjTn0soAXrkjs50LXUbTy4Of/NeJFUoUvTeB8Y/Rbaqm4 sMGZAW9d+HeRR9BNUwKqb8qHF5wflgdKorsJdcnV3hs8rlr/fvK6xkHSXHKizJywY3NT xW4w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=9LEfa7BloCux0gpZPERBpZvr0PiKd8LtXXPn3LceYuM=; b=qbFQPY0SUjgM80wjAltIKUY+S8ircbUvkceOCSGozIXrrXPATHYD1KZuGqgcYkxC4N v/MkW7YqBfX+0El36AhfwuWXXyrtNjE8XmYh96VRH600RHhmlpAWaYHNUme1K7aFvBn3 XIo5QavjllA9yPF63UOIFr2DR7+jSAPxQo9uu1M0QUKwWRBT/X/1VedoyIch4lNiESd6 08qNT7QGWRHeL9vN5vYfULmnERMK2oKeca03N3fuxOykegH7JHl5agaXaCcylkbU++hI HUZdLaHRiR/6/5WYJa3aFffjiG2198d65Ue6CChnary0Bc9qSpDJtap3iwbaVrZ2lw+m AmMg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=jlXojrXq; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id z6-20020aa79e46000000b006432e1ea8e4si7951839pfq.14.2023.05.11.08.34.59; Thu, 11 May 2023 08:35:12 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=jlXojrXq; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238620AbjEKPWB (ORCPT <rfc822;peekingduck44@gmail.com> + 99 others); Thu, 11 May 2023 11:22:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45828 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238612AbjEKPV4 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 11 May 2023 11:21:56 -0400 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A59A0138; Thu, 11 May 2023 08:21:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1683818512; x=1715354512; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=XR/zMIbG4Jcm24Yt1GRfoMUGo5dYfwoSu6slMdxjNv0=; b=jlXojrXqon1WWvbp+EcLZkFcoys1+84ZnRSwfMh/R3uCSRNCo2u/Kths 6FQzaD7kromafBDXORasEL+sclbnkMRuBJJYPzrgKn4RGmrdK0BJHIeF5 z+BmkW3oHyaztnJ+6g8eN6XhvJuZwvE01tcjqESo5cuKWo6rmKljKPZpp WKcmA7k0tpbWPwwgXhbulutpfH77AFa7ZB+ZZyVdxiIixMIW+n4aLJswT OdWVfYk6++m7+sOUxzILk7KpuIHkdHgyTNV33woEOvCJKqrMYJycSFxUA 8c7aE7RoWoVwo9j6n9ot6CGrPeZpyRWyyNbuPJwf5sFof755W8gplIkQi w==; X-IronPort-AV: E=McAfee;i="6600,9927,10707"; a="352760702" X-IronPort-AV: E=Sophos;i="5.99,266,1677571200"; d="scan'208";a="352760702" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 May 2023 08:20:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10707"; a="702782185" X-IronPort-AV: E=Sophos;i="5.99,266,1677571200"; d="scan'208";a="702782185" Received: from irvmail002.ir.intel.com ([10.43.11.120]) by fmsmga007.fm.intel.com with ESMTP; 11 May 2023 08:20:06 -0700 Received: from lincoln.igk.intel.com (lincoln.igk.intel.com [10.102.21.235]) by irvmail002.ir.intel.com (Postfix) with ESMTP id 6D7EE36C0E; Thu, 11 May 2023 16:20:05 +0100 (IST) From: Larysa Zaremba <larysa.zaremba@intel.com> To: Quentin Monnet <quentin@isovalent.com> Cc: Larysa Zaremba <larysa.zaremba@intel.com>, Stanislav Fomichev <sdf@google.com>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Andrii Nakryiko <andrii@kernel.org>, Martin KaFai Lau <martin.lau@linux.dev>, Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>, John Fastabend <john.fastabend@gmail.com>, KP Singh <kpsingh@kernel.org>, Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>, bpf@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH bpf-next] bpftool: specify XDP Hints ifname when loading program Date: Thu, 11 May 2023 17:13:46 +0200 Message-Id: <20230511151345.7529-1-larysa.zaremba@intel.com> X-Mailer: git-send-email 2.35.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1765612519724557874?= X-GMAIL-MSGID: =?utf-8?q?1765612519724557874?= |
Series |
[bpf-next] bpftool: specify XDP Hints ifname when loading program
|
|
Commit Message
Larysa Zaremba
May 11, 2023, 3:13 p.m. UTC
Add ability to specify a network interface used to resolve
XDP Hints kfuncs when loading program through bpftool.
Usage:
bpftool prog load <bpf_obj_path> <pin_path> dev xdpmeta <ifname>
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
tools/bpf/bpftool/prog.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
Comments
2023-05-11 17:13 UTC+0200 ~ Larysa Zaremba <larysa.zaremba@intel.com> > Add ability to specify a network interface used to resolve > XDP Hints kfuncs when loading program through bpftool. > > Usage: > bpftool prog load <bpf_obj_path> <pin_path> dev xdpmeta <ifname> Thanks for this patch! Regarding the command-line syntax, I'm not a big fan of the optional sub-keyword for the device for XDP hints. I must admit I had not anticipated other another use for the "dev" keyword. Instead, have you considered one of the following: 1) Adding a different keyword ("xdpmeta_dev"?) and making it incompatible with "dev" 2) Another alternative would be adding a sub-keyword for offload too: bpftool p l [...] dev <[offload <ifname> | xdpmeta <ifname>]> If the ifname is provided with no sub-keyword, we would consider it for offload for legacy support, possibly warn that the syntax is deprecated. What do you think? > > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com> > --- > tools/bpf/bpftool/prog.c | 28 +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c > index 91b6075b2db3..a9cb96d99277 100644 > --- a/tools/bpf/bpftool/prog.c > +++ b/tools/bpf/bpftool/prog.c > @@ -1517,12 +1517,13 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > struct bpf_program *prog = NULL, *pos; > unsigned int old_map_fds = 0; > const char *pinmaps = NULL; > + __u32 offload_ifindex = 0; > bool auto_attach = false; > + __u32 meta_ifindex = 0; > struct bpf_object *obj; > struct bpf_map *map; > const char *pinfile; > unsigned int i, j; > - __u32 ifindex = 0; > const char *file; > int idx, err; > > @@ -1614,17 +1615,25 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > map_replace[old_map_fds].fd = fd; > old_map_fds++; > } else if (is_prefix(*argv, "dev")) { > + __u32 *cur_ifindex; > + > NEXT_ARG(); > > - if (ifindex) { > - p_err("offload device already specified"); > + if (offload_ifindex || meta_ifindex) { > + p_err("device already specified"); I'd add a note to tell somehow that offload and XDP hints are mutually exclusive. I suspect not all users understand these well. > goto err_free_reuse_maps; > } > + if (is_prefix(*argv, "xdpmeta")) { > + cur_ifindex = &meta_ifindex; > + NEXT_ARG(); > + } else { > + cur_ifindex = &offload_ifindex; > + } > if (!REQ_ARGS(1)) > goto err_free_reuse_maps; > > - ifindex = if_nametoindex(*argv); > - if (!ifindex) { > + *cur_ifindex = if_nametoindex(*argv); > + if (!(*cur_ifindex)) { > p_err("unrecognized netdevice '%s': %s", > *argv, strerror(errno)); > goto err_free_reuse_maps; > @@ -1671,7 +1680,12 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > goto err_close_obj; > } > > - bpf_program__set_ifindex(pos, ifindex); > + if (prog_type == BPF_PROG_TYPE_XDP && meta_ifindex) { > + bpf_program__set_flags(pos, BPF_F_XDP_DEV_BOUND_ONLY); > + bpf_program__set_ifindex(pos, meta_ifindex); > + } else { > + bpf_program__set_ifindex(pos, offload_ifindex); > + } > if (bpf_program__type(pos) != prog_type) > bpf_program__set_type(pos, prog_type); > bpf_program__set_expected_attach_type(pos, expected_attach_type); > @@ -1709,7 +1723,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > idx = 0; > bpf_object__for_each_map(map, obj) { > if (bpf_map__type(map) != BPF_MAP_TYPE_PERF_EVENT_ARRAY) > - bpf_map__set_ifindex(map, ifindex); > + bpf_map__set_ifindex(map, offload_ifindex); > > if (j < old_map_fds && idx == map_replace[j].idx) { > err = bpf_map__reuse_fd(map, map_replace[j++].fd); Could you please also update the following items: - The usage message for "bpftool prog load" near the end of prog.c - The related doc in Documentation/bpftool-prog.rst (command summary near the top, and detailed description later in the doc) - Bash completion (for keyword "dev", look for "_sysfs_get_netdevs" in bash-completion/bpftool). I'm happy to help with this one if necessary. Thanks, Quentin
On Fri, May 12, 2023 at 11:23:00AM +0100, Quentin Monnet wrote: > 2023-05-11 17:13 UTC+0200 ~ Larysa Zaremba <larysa.zaremba@intel.com> > > Add ability to specify a network interface used to resolve > > XDP Hints kfuncs when loading program through bpftool. > > > > Usage: > > bpftool prog load <bpf_obj_path> <pin_path> dev xdpmeta <ifname> > > Thanks for this patch! > > Regarding the command-line syntax, I'm not a big fan of the optional > sub-keyword for the device for XDP hints. I must admit I had not > anticipated other another use for the "dev" keyword. Instead, have you > considered one of the following: > > 1) Adding a different keyword ("xdpmeta_dev"?) and making it > incompatible with "dev" > > 2) Another alternative would be adding a sub-keyword for offload too: > > bpftool p l [...] dev <[offload <ifname> | xdpmeta <ifname>]> > > If the ifname is provided with no sub-keyword, we would consider it for > offload for legacy support, possibly warn that the syntax is deprecated. > > What do you think? > I think first option would look a little bit nicer, but I like the idea to deprecate "dev <ifname>". In my current version, forgetting to add "xdpmeta" resulted in not very descriptive errors, this may confuse new users. So what about: bpftool prog load [...] xdpmeta_dev/offload_dev <ifname> "dev <ifname>" syntax would still work, but with a big warning, like this: 'bpftool prog [...] dev <ifname>' syntax is deprecated. Going further, please use 'offload_dev <ifname>' to offload program to device. For XDP hints applications, use 'xdpmeta_dev <ifname>'. > > > > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com> > > --- > > tools/bpf/bpftool/prog.c | 28 +++++++++++++++++++++------- > > 1 file changed, 21 insertions(+), 7 deletions(-) > > > > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c > > index 91b6075b2db3..a9cb96d99277 100644 > > --- a/tools/bpf/bpftool/prog.c > > +++ b/tools/bpf/bpftool/prog.c > > @@ -1517,12 +1517,13 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > > struct bpf_program *prog = NULL, *pos; > > unsigned int old_map_fds = 0; > > const char *pinmaps = NULL; > > + __u32 offload_ifindex = 0; > > bool auto_attach = false; > > + __u32 meta_ifindex = 0; > > struct bpf_object *obj; > > struct bpf_map *map; > > const char *pinfile; > > unsigned int i, j; > > - __u32 ifindex = 0; > > const char *file; > > int idx, err; > > > > @@ -1614,17 +1615,25 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > > map_replace[old_map_fds].fd = fd; > > old_map_fds++; > > } else if (is_prefix(*argv, "dev")) { > > + __u32 *cur_ifindex; > > + > > NEXT_ARG(); > > > > - if (ifindex) { > > - p_err("offload device already specified"); > > + if (offload_ifindex || meta_ifindex) { > > + p_err("device already specified"); > > I'd add a note to tell somehow that offload and XDP hints are mutually > exclusive. I suspect not all users understand these well. Ok, will do. > > > goto err_free_reuse_maps; > > } > > + if (is_prefix(*argv, "xdpmeta")) { > > + cur_ifindex = &meta_ifindex; > > + NEXT_ARG(); > > + } else { > > + cur_ifindex = &offload_ifindex; > > + } > > if (!REQ_ARGS(1)) > > goto err_free_reuse_maps; > > > > - ifindex = if_nametoindex(*argv); > > - if (!ifindex) { > > + *cur_ifindex = if_nametoindex(*argv); > > + if (!(*cur_ifindex)) { > > p_err("unrecognized netdevice '%s': %s", > > *argv, strerror(errno)); > > goto err_free_reuse_maps; > > @@ -1671,7 +1680,12 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > > goto err_close_obj; > > } > > > > - bpf_program__set_ifindex(pos, ifindex); > > + if (prog_type == BPF_PROG_TYPE_XDP && meta_ifindex) { > > + bpf_program__set_flags(pos, BPF_F_XDP_DEV_BOUND_ONLY); > > + bpf_program__set_ifindex(pos, meta_ifindex); > > + } else { > > + bpf_program__set_ifindex(pos, offload_ifindex); > > + } > > if (bpf_program__type(pos) != prog_type) > > bpf_program__set_type(pos, prog_type); > > bpf_program__set_expected_attach_type(pos, expected_attach_type); > > @@ -1709,7 +1723,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > > idx = 0; > > bpf_object__for_each_map(map, obj) { > > if (bpf_map__type(map) != BPF_MAP_TYPE_PERF_EVENT_ARRAY) > > - bpf_map__set_ifindex(map, ifindex); > > + bpf_map__set_ifindex(map, offload_ifindex); > > > > if (j < old_map_fds && idx == map_replace[j].idx) { > > err = bpf_map__reuse_fd(map, map_replace[j++].fd); > > Could you please also update the following items: > > - The usage message for "bpftool prog load" near the end of prog.c > > - The related doc in Documentation/bpftool-prog.rst (command summary > near the top, and detailed description later in the doc) > > - Bash completion (for keyword "dev", look for "_sysfs_get_netdevs" in > bash-completion/bpftool). I'm happy to help with this one if necessary. Will do all the above in v2. > > Thanks, > Quentin
On 2023-05-12 15:36:47 +0200, Larysa Zaremba wrote: > [You don't often get email from larysa.zaremba@intel.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > On Fri, May 12, 2023 at 11:23:00AM +0100, Quentin Monnet wrote: > > 2023-05-11 17:13 UTC+0200 ~ Larysa Zaremba <larysa.zaremba@intel.com> > > > Add ability to specify a network interface used to resolve > > > XDP Hints kfuncs when loading program through bpftool. > > > > > > Usage: > > > bpftool prog load <bpf_obj_path> <pin_path> dev xdpmeta <ifname> > > > > Thanks for this patch! > > > > Regarding the command-line syntax, I'm not a big fan of the optional > > sub-keyword for the device for XDP hints. I must admit I had not > > anticipated other another use for the "dev" keyword. Instead, have you > > considered one of the following: > > > > 1) Adding a different keyword ("xdpmeta_dev"?) and making it > > incompatible with "dev" > > > > 2) Another alternative would be adding a sub-keyword for offload too: > > > > bpftool p l [...] dev <[offload <ifname> | xdpmeta <ifname>]> > > > > If the ifname is provided with no sub-keyword, we would consider it for > > offload for legacy support, possibly warn that the syntax is deprecated. > > > > What do you think? > > > > I think first option would look a little bit nicer, but I like the idea to > deprecate "dev <ifname>". In my current version, forgetting to add "xdpmeta" > resulted in not very descriptive errors, this may confuse new users. So what > about: I agree the first option looks a little bit nicer, but I think both options would work. > > bpftool prog load [...] xdpmeta_dev/offload_dev <ifname> > > "dev <ifname>" syntax would still work, but with a big warning, like this: > > 'bpftool prog [...] dev <ifname>' syntax is deprecated. Going further, please > use 'offload_dev <ifname>' to offload program to device. For XDP hints > applications, use 'xdpmeta_dev <ifname>'. > > > > > > > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com> > > > --- > > > tools/bpf/bpftool/prog.c | 28 +++++++++++++++++++++------- > > > 1 file changed, 21 insertions(+), 7 deletions(-) > > > > > > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c > > > index 91b6075b2db3..a9cb96d99277 100644 > > > --- a/tools/bpf/bpftool/prog.c > > > +++ b/tools/bpf/bpftool/prog.c > > > @@ -1517,12 +1517,13 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > > > struct bpf_program *prog = NULL, *pos; > > > unsigned int old_map_fds = 0; > > > const char *pinmaps = NULL; > > > + __u32 offload_ifindex = 0; > > > bool auto_attach = false; > > > + __u32 meta_ifindex = 0; > > > struct bpf_object *obj; > > > struct bpf_map *map; > > > const char *pinfile; > > > unsigned int i, j; > > > - __u32 ifindex = 0; > > > const char *file; > > > int idx, err; > > > > > > @@ -1614,17 +1615,25 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > > > map_replace[old_map_fds].fd = fd; > > > old_map_fds++; > > > } else if (is_prefix(*argv, "dev")) { > > > + __u32 *cur_ifindex; > > > + > > > NEXT_ARG(); > > > > > > - if (ifindex) { > > > - p_err("offload device already specified"); > > > + if (offload_ifindex || meta_ifindex) { > > > + p_err("device already specified"); > > > > I'd add a note to tell somehow that offload and XDP hints are mutually > > exclusive. I suspect not all users understand these well. > > Ok, will do. > > > > > > goto err_free_reuse_maps; > > > } > > > + if (is_prefix(*argv, "xdpmeta")) { > > > + cur_ifindex = &meta_ifindex; > > > + NEXT_ARG(); > > > + } else { > > > + cur_ifindex = &offload_ifindex; > > > + } > > > if (!REQ_ARGS(1)) > > > goto err_free_reuse_maps; > > > > > > - ifindex = if_nametoindex(*argv); > > > - if (!ifindex) { > > > + *cur_ifindex = if_nametoindex(*argv); > > > + if (!(*cur_ifindex)) { > > > p_err("unrecognized netdevice '%s': %s", > > > *argv, strerror(errno)); > > > goto err_free_reuse_maps; > > > @@ -1671,7 +1680,12 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > > > goto err_close_obj; > > > } > > > > > > - bpf_program__set_ifindex(pos, ifindex); > > > + if (prog_type == BPF_PROG_TYPE_XDP && meta_ifindex) { > > > + bpf_program__set_flags(pos, BPF_F_XDP_DEV_BOUND_ONLY); > > > + bpf_program__set_ifindex(pos, meta_ifindex); > > > + } else { > > > + bpf_program__set_ifindex(pos, offload_ifindex); > > > + } > > > if (bpf_program__type(pos) != prog_type) > > > bpf_program__set_type(pos, prog_type); > > > bpf_program__set_expected_attach_type(pos, expected_attach_type); > > > @@ -1709,7 +1723,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > > > idx = 0; > > > bpf_object__for_each_map(map, obj) { > > > if (bpf_map__type(map) != BPF_MAP_TYPE_PERF_EVENT_ARRAY) > > > - bpf_map__set_ifindex(map, ifindex); > > > + bpf_map__set_ifindex(map, offload_ifindex); > > > > > > if (j < old_map_fds && idx == map_replace[j].idx) { > > > err = bpf_map__reuse_fd(map, map_replace[j++].fd); > > > > Could you please also update the following items: > > > > - The usage message for "bpftool prog load" near the end of prog.c > > > > - The related doc in Documentation/bpftool-prog.rst (command summary > > near the top, and detailed description later in the doc) > > > > - Bash completion (for keyword "dev", look for "_sysfs_get_netdevs" in > > bash-completion/bpftool). I'm happy to help with this one if necessary. > > Will do all the above in v2. > > > > > Thanks, > > Quentin
2023-05-15 18:30 UTC+0200 ~ Niklas Söderlund <niklas.soderlund@corigine.com> > On 2023-05-12 15:36:47 +0200, Larysa Zaremba wrote: >> [You don't often get email from larysa.zaremba@intel.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] >> >> On Fri, May 12, 2023 at 11:23:00AM +0100, Quentin Monnet wrote: >>> 2023-05-11 17:13 UTC+0200 ~ Larysa Zaremba <larysa.zaremba@intel.com> >>>> Add ability to specify a network interface used to resolve >>>> XDP Hints kfuncs when loading program through bpftool. >>>> >>>> Usage: >>>> bpftool prog load <bpf_obj_path> <pin_path> dev xdpmeta <ifname> >>> >>> Thanks for this patch! >>> >>> Regarding the command-line syntax, I'm not a big fan of the optional >>> sub-keyword for the device for XDP hints. I must admit I had not >>> anticipated other another use for the "dev" keyword. Instead, have you >>> considered one of the following: >>> >>> 1) Adding a different keyword ("xdpmeta_dev"?) and making it >>> incompatible with "dev" >>> >>> 2) Another alternative would be adding a sub-keyword for offload too: >>> >>> bpftool p l [...] dev <[offload <ifname> | xdpmeta <ifname>]> >>> >>> If the ifname is provided with no sub-keyword, we would consider it for >>> offload for legacy support, possibly warn that the syntax is deprecated. >>> >>> What do you think? >>> >> >> I think first option would look a little bit nicer, but I like the idea to >> deprecate "dev <ifname>". In my current version, forgetting to add "xdpmeta" >> resulted in not very descriptive errors, this may confuse new users. So what >> about: > > I agree the first option looks a little bit nicer, but I think both > options would work. > >> >> bpftool prog load [...] xdpmeta_dev/offload_dev <ifname> >> >> "dev <ifname>" syntax would still work, but with a big warning, like this: >> >> 'bpftool prog [...] dev <ifname>' syntax is deprecated. Going further, please >> use 'offload_dev <ifname>' to offload program to device. For XDP hints >> applications, use 'xdpmeta_dev <ifname>'. OK let's go with this Thanks, Quentin
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index 91b6075b2db3..a9cb96d99277 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -1517,12 +1517,13 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) struct bpf_program *prog = NULL, *pos; unsigned int old_map_fds = 0; const char *pinmaps = NULL; + __u32 offload_ifindex = 0; bool auto_attach = false; + __u32 meta_ifindex = 0; struct bpf_object *obj; struct bpf_map *map; const char *pinfile; unsigned int i, j; - __u32 ifindex = 0; const char *file; int idx, err; @@ -1614,17 +1615,25 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) map_replace[old_map_fds].fd = fd; old_map_fds++; } else if (is_prefix(*argv, "dev")) { + __u32 *cur_ifindex; + NEXT_ARG(); - if (ifindex) { - p_err("offload device already specified"); + if (offload_ifindex || meta_ifindex) { + p_err("device already specified"); goto err_free_reuse_maps; } + if (is_prefix(*argv, "xdpmeta")) { + cur_ifindex = &meta_ifindex; + NEXT_ARG(); + } else { + cur_ifindex = &offload_ifindex; + } if (!REQ_ARGS(1)) goto err_free_reuse_maps; - ifindex = if_nametoindex(*argv); - if (!ifindex) { + *cur_ifindex = if_nametoindex(*argv); + if (!(*cur_ifindex)) { p_err("unrecognized netdevice '%s': %s", *argv, strerror(errno)); goto err_free_reuse_maps; @@ -1671,7 +1680,12 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) goto err_close_obj; } - bpf_program__set_ifindex(pos, ifindex); + if (prog_type == BPF_PROG_TYPE_XDP && meta_ifindex) { + bpf_program__set_flags(pos, BPF_F_XDP_DEV_BOUND_ONLY); + bpf_program__set_ifindex(pos, meta_ifindex); + } else { + bpf_program__set_ifindex(pos, offload_ifindex); + } if (bpf_program__type(pos) != prog_type) bpf_program__set_type(pos, prog_type); bpf_program__set_expected_attach_type(pos, expected_attach_type); @@ -1709,7 +1723,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) idx = 0; bpf_object__for_each_map(map, obj) { if (bpf_map__type(map) != BPF_MAP_TYPE_PERF_EVENT_ARRAY) - bpf_map__set_ifindex(map, ifindex); + bpf_map__set_ifindex(map, offload_ifindex); if (j < old_map_fds && idx == map_replace[j].idx) { err = bpf_map__reuse_fd(map, map_replace[j++].fd);