public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <brouer@redhat•com>
To: Saeed Mahameed <saeedm@dev•mellanox.co.il>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail•com>,
	Daniel Borkmann <borkmann@iogearbox•net>,
	neerav.parikh@intel•com, pjwaskiewicz@gmail•com,
	ttoukan.linux@gmail•com, Tariq Toukan <tariqt@mellanox•com>,
	alexander.h.duyck@intel•com, peter.waskiewicz.jr@intel•com,
	Opher Reviv <opher@mellanox•com>,
	Rony Efraim <ronye@mellanox•com>,
	netdev@vger•kernel.org, Saeed Mahameed <saeedm@mellanox•com>,
	brouer@redhat•com
Subject: Re: [RFC bpf-next 6/6] samples/bpf: Add meta data hash example to xdp_redirect_cpu
Date: Wed, 27 Jun 2018 12:59:14 +0200	[thread overview]
Message-ID: <20180627125914.1a3b52db@redhat.com> (raw)
In-Reply-To: <20180627024615.17856-7-saeedm@mellanox.com>

On Tue, 26 Jun 2018 19:46:15 -0700
Saeed Mahameed <saeedm@dev•mellanox.co.il> wrote:

> Add a new program (prog_num = 4) that will not parse packets and will
> use the meta data hash to spread/redirect traffic into different cpus.

You cannot "steal" prognum 4, as it is already used for
"xdp_prognum4_ddos_filter_pktgen".  Please append your new prog as #5.

> For the new program we set on bpf_set_link_xdp_fd:
> 	xdp_flags |= XDP_FLAGS_META_HASH | XDP_FLAGS_META_VLAN;
> 
> On mlx5 it will succeed since mlx5 already supports these flags.
> 
> The new program will read the value of the hash from the data_meta
> pointer from the xdp_md and will use it to compute the destination cpu.
> 
> Note: I didn't test this patch to show redirect works with the hash!
> I only used it to see that the hash and vlan values are set correctly
> by the driver and can be seen by the xdp program.
> 
> * I faced some difficulties to read the hash value using the helper
> functions defined in the previous patches, but once i used the same logic
> with out these functions it worked ! Will have to figure this out later.
> 
> Signed-off-by: Saeed Mahameed <saeedm@mellanox•com>
> ---
>  samples/bpf/xdp_redirect_cpu_kern.c | 67 +++++++++++++++++++++++++++++
>  samples/bpf/xdp_redirect_cpu_user.c |  7 +++
>  2 files changed, 74 insertions(+)
> 
> diff --git a/samples/bpf/xdp_redirect_cpu_kern.c b/samples/bpf/xdp_redirect_cpu_kern.c
> index 303e9e7161f3..d6b3f55f342a 100644
> --- a/samples/bpf/xdp_redirect_cpu_kern.c
> +++ b/samples/bpf/xdp_redirect_cpu_kern.c
> @@ -376,6 +376,73 @@ int  xdp_prognum3_proto_separate(struct xdp_md *ctx)
>  	return bpf_redirect_map(&cpu_map, cpu_dest, 0);
>  }
>  
> +#if 0
> +xdp_md_info_arr mdi = {
> +	[XDP_DATA_META_HASH] = {.offset = 0, .present = 1},
> +	[XDP_DATA_META_VLAN] = {.offset = sizeof(struct xdp_md_hash), .present = 1},
> +};
> +#endif

Sorry, no global variables avail in the generated BPF byte-code.

> +SEC("xdp_cpu_map4_hash_separate")
> +int  xdp_prognum4_hash_separate(struct xdp_md *ctx)
> +{
> +	void *data_meta = (void *)(long)ctx->data_meta;
> +	void *data_end  = (void *)(long)ctx->data_end;
> +	void *data      = (void *)(long)ctx->data;
> +	struct xdp_md_hash *hash;
> +	struct xdp_md_vlan *vlan;
> +	struct datarec *rec;
> +	u32 cpu_dest = 0;
> +	u32 cpu_idx = 0;
> +	u32 *cpu_lookup;
> +	u32 key = 0;
> +
> +	/* Count RX packet in map */
> +	rec = bpf_map_lookup_elem(&rx_cnt, &key);
> +	if (!rec)
> +		return XDP_ABORTED;
> +	rec->processed++;
> +
> +	/* for some reason this code fails to be verified */
> +#if 0
> +	hash = xdp_data_meta_get_hash(mdi, data_meta);

This will not work, because it is not implemented as a proper
BPF-helper call.

First, you currently store the xdp_md_info_arr inside the driver, which
makes it hard for a helper to access this.  For helper access, we could
store this in xdp_rxq_info.

Second, in your design it looks like you are introducing a helper per
possible item in xdp_md_info_arr.  I think we can reduce this to a
single helper, that takes a XDP_DATA_META_xxx flag, and returns an
offset.  (The helper could return a direct pointer, but I don't think
the verfier can handle that, as it need to "see" this is related to
data_meta pointer, and that we do the proper boundry checks.).

The BPF prog already have direct memory access to the data_meta area,
and all it really need is an offset.  Sure, the XDP/bpf programmer
could just calculate these offsets as constants, and remember to load
the XDP prog with the flags that corresponds to the calculated offsets.

But I think we can do something even smarter... 

It should be possible to convert/patch the BPF instructions, of the
helper call that returns an offset, to instead avoid the call and
either (1) provide the offset as a constant/IMM or (2) make BPF inst
doing the lookup in xdp_md_info_arr.


> +	if (hash + 1 > data)
> +		return XDP_ABORTED;
> +
> +	vlan = xdp_data_meta_get_vlan(mdi, data_meta);
> +	if (vlan + 1 > data)
> +		return XDP_ABORTED;
> +#endif
> +
> +	/* Work around for the above code */
> +	hash = data_meta; /* since we know hash will appear first */
> +        if (hash + 1 > data)
> +		return XDP_ABORTED;
> +
> +#if 0
> +	// Just for testing
> +	/* We know that vlan will appear after the hash */
> +	vlan = (void *)((char *)data_meta + sizeof(*hash));
> +	if (vlan + 1 > data) {
> +		return XDP_ABORTED;
> +	}
> +#endif

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

  reply	other threads:[~2018-06-27 10:59 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-27  2:46 [RFC bpf-next 0/6] XDP RX device meta data acceleration (WIP) Saeed Mahameed
2018-06-27  2:46 ` [RFC bpf-next 1/6] net: xdp: Add support for meta data flags requests Saeed Mahameed
2018-06-27  2:46 ` [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure Saeed Mahameed
2018-06-27 14:15   ` Jesper Dangaard Brouer
2018-06-27 17:55     ` Saeed Mahameed
2018-07-02  8:01       ` Daniel Borkmann
2018-07-03 23:52         ` Saeed Mahameed
2018-07-03 23:01   ` Alexei Starovoitov
2018-07-04  0:57     ` Saeed Mahameed
2018-07-04  7:51       ` Daniel Borkmann
2018-07-05 17:18         ` Jakub Kicinski
2018-07-06 16:30           ` Alexei Starovoitov
2018-07-06 20:44             ` Waskiewicz Jr, Peter
2018-07-06 23:38               ` Alexei Starovoitov
2018-07-06 23:49                 ` Waskiewicz Jr, Peter
2018-07-07  0:40                   ` Jakub Kicinski
2018-07-07  1:00                     ` Alexei Starovoitov
2018-07-07  1:20                       ` Jakub Kicinski
2018-07-07  2:38                         ` Alexei Starovoitov
2018-07-07  0:45                   ` Alexei Starovoitov
2018-07-06 21:33             ` Jakub Kicinski
2018-07-06 23:42               ` Alexei Starovoitov
2018-07-07  0:08                 ` Jakub Kicinski
2018-07-07  0:53                   ` Alexei Starovoitov
2018-07-07  1:37                     ` David Miller
2018-07-07  1:44                     ` Jakub Kicinski
2018-07-07  2:51                       ` Alexei Starovoitov
2018-07-07  1:27             ` David Miller
2018-06-27  2:46 ` [RFC bpf-next 3/6] net/mlx5e: Store xdp flags and meta data info Saeed Mahameed
2018-06-27  2:46 ` [RFC bpf-next 4/6] net/mlx5e: Pass CQE to RX handlers Saeed Mahameed
2018-06-27  2:46 ` [RFC bpf-next 5/6] net/mlx5e: Add XDP RX meta data support Saeed Mahameed
2018-07-04  8:28   ` Daniel Borkmann
2018-06-27  2:46 ` [RFC bpf-next 6/6] samples/bpf: Add meta data hash example to xdp_redirect_cpu Saeed Mahameed
2018-06-27 10:59   ` Jesper Dangaard Brouer [this message]
2018-06-27 18:04     ` Saeed Mahameed
2018-06-27 16:42 ` [RFC bpf-next 0/6] XDP RX device meta data acceleration (WIP) Parikh, Neerav

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180627125914.1a3b52db@redhat.com \
    --to=brouer@redhat$(echo .)com \
    --cc=alexander.h.duyck@intel$(echo .)com \
    --cc=alexei.starovoitov@gmail$(echo .)com \
    --cc=borkmann@iogearbox$(echo .)net \
    --cc=neerav.parikh@intel$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=opher@mellanox$(echo .)com \
    --cc=peter.waskiewicz.jr@intel$(echo .)com \
    --cc=pjwaskiewicz@gmail$(echo .)com \
    --cc=ronye@mellanox$(echo .)com \
    --cc=saeedm@dev$(echo .)mellanox.co.il \
    --cc=saeedm@mellanox$(echo .)com \
    --cc=tariqt@mellanox$(echo .)com \
    --cc=ttoukan.linux@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox