On Tue, 26 Jun 2018 19:46:15 -0700
Saeed Mahameed <sae...@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 <sae...@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
