| Message ID | 20180627024615.17856-7-saeedm@mellanox.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | XDP RX device meta data acceleration (WIP) | expand |
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
On Wed, 2018-06-27 at 12:59 +0200, Jesper Dangaard Brouer wrote: > 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. > Sure. > > 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. > Yea i found out the hard way :), but for my final solution i would like to somehow share the same static md info array between netdev and bpf program, so this code was just experimental. > > +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. > Good Idea! > 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.). > We can update the verifier to allow access to any offset between data_meta and data_meta + offset(last meta data) + sizeof(last meta data) ? > 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. > I vote (2). > > > + 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 > >
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 + +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); + 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 + + cpu_idx = reciprocal_scale(hash->hash, MAX_CPUS); + + cpu_lookup = bpf_map_lookup_elem(&cpus_available, &cpu_idx); + if (!cpu_lookup) + return XDP_ABORTED; + cpu_dest = *cpu_lookup; + + if (cpu_dest >= MAX_CPUS) { + rec->issue++; + return XDP_ABORTED; + } + + return bpf_redirect_map(&cpu_map, cpu_dest, 0); +} + SEC("xdp_cpu_map4_ddos_filter_pktgen") int xdp_prognum4_ddos_filter_pktgen(struct xdp_md *ctx) { diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c index f6efaefd485b..3429215d5a7b 100644 --- a/samples/bpf/xdp_redirect_cpu_user.c +++ b/samples/bpf/xdp_redirect_cpu_user.c @@ -679,6 +679,13 @@ int main(int argc, char **argv) return EXIT_FAIL_OPTION; } + /* + * prog_num 4 requires xdp meta data hash + * Vlan is not required but added just for testing.. + */ + if (prog_num == 4) + xdp_flags |= XDP_FLAGS_META_HASH | XDP_FLAGS_META_VLAN; + /* Remove XDP program when program is interrupted */ signal(SIGINT, int_exit);
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. 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(+)