* [bpf-next PATCH 0/2] xdp/bpf: extend XDP samples/bpf xdp_rxq_info
@ 2018-06-25 14:27 Jesper Dangaard Brouer
2018-06-25 14:27 ` [bpf-next PATCH 1/2] samples/bpf: extend xdp_rxq_info to read packet payload Jesper Dangaard Brouer
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Jesper Dangaard Brouer @ 2018-06-25 14:27 UTC (permalink / raw)
To: netdev, Jesper Dangaard Brouer
Cc: Daniel Borkmann, Toke Høiland-Jørgensen, Alexei Starovoitov
While writing an article about XDP, the samples/bpf xdp_rxq_info
program were extended to cover some more use-cases.
---
Jesper Dangaard Brouer (2):
samples/bpf: extend xdp_rxq_info to read packet payload
samples/bpf: xdp_rxq_info action XDP_TX must adjust MAC-addrs
samples/bpf/xdp_rxq_info_kern.c | 43 +++++++++++++++++++++++++++++++++++++
samples/bpf/xdp_rxq_info_user.c | 45 ++++++++++++++++++++++++++++++++++-----
2 files changed, 82 insertions(+), 6 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [bpf-next PATCH 1/2] samples/bpf: extend xdp_rxq_info to read packet payload
2018-06-25 14:27 [bpf-next PATCH 0/2] xdp/bpf: extend XDP samples/bpf xdp_rxq_info Jesper Dangaard Brouer
@ 2018-06-25 14:27 ` Jesper Dangaard Brouer
2018-06-26 23:53 ` Song Liu
2018-06-25 14:27 ` [bpf-next PATCH 2/2] samples/bpf: xdp_rxq_info action XDP_TX must adjust MAC-addrs Jesper Dangaard Brouer
2018-06-28 21:54 ` [bpf-next PATCH 0/2] xdp/bpf: extend XDP samples/bpf xdp_rxq_info Daniel Borkmann
2 siblings, 1 reply; 10+ messages in thread
From: Jesper Dangaard Brouer @ 2018-06-25 14:27 UTC (permalink / raw)
To: netdev, Jesper Dangaard Brouer
Cc: Daniel Borkmann, Toke Høiland-Jørgensen, Alexei Starovoitov
There is a cost associated with reading the packet data payload
that this test ignored. Add option --read to allow enabling
reading part of the payload.
This sample/tool helps us analyse an issue observed with a NIC
mlx5 (ConnectX-5 Ex) and an Intel(R) Xeon(R) CPU E5-1650 v4.
With no_touch of data:
Running XDP on dev:mlx5p1 (ifindex:8) action:XDP_DROP options:no_touch
XDP stats CPU pps issue-pps
XDP-RX CPU 0 14,465,157 0
XDP-RX CPU 1 14,464,728 0
XDP-RX CPU 2 14,465,283 0
XDP-RX CPU 3 14,465,282 0
XDP-RX CPU 4 14,464,159 0
XDP-RX CPU 5 14,465,379 0
XDP-RX CPU total 86,789,992
When not touching data, we observe that the CPUs have idle cycles.
When reading data the CPUs are 100% busy in softirq.
With reading data:
Running XDP on dev:mlx5p1 (ifindex:8) action:XDP_DROP options:read
XDP stats CPU pps issue-pps
XDP-RX CPU 0 9,620,639 0
XDP-RX CPU 1 9,489,843 0
XDP-RX CPU 2 9,407,854 0
XDP-RX CPU 3 9,422,289 0
XDP-RX CPU 4 9,321,959 0
XDP-RX CPU 5 9,395,242 0
XDP-RX CPU total 56,657,828
The effect seen above is a result of cache-misses occuring when
more RXQs are being used. Based on perf-event observations, our
conclusion is that the CPUs DDIO (Direct Data I/O) choose to
deliver packet into main memory, instead of L3-cache. We also
found, that this can be mitigated by either using less RXQs or by
reducing NICs the RX-ring size.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
samples/bpf/xdp_rxq_info_kern.c | 19 +++++++++++++++++++
samples/bpf/xdp_rxq_info_user.c | 34 ++++++++++++++++++++++++++++------
2 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/samples/bpf/xdp_rxq_info_kern.c b/samples/bpf/xdp_rxq_info_kern.c
index 3fd209291653..61af6210df2f 100644
--- a/samples/bpf/xdp_rxq_info_kern.c
+++ b/samples/bpf/xdp_rxq_info_kern.c
@@ -4,6 +4,8 @@
* Example howto extract XDP RX-queue info
*/
#include <uapi/linux/bpf.h>
+#include <uapi/linux/if_ether.h>
+#include <uapi/linux/in.h>
#include "bpf_helpers.h"
/* Config setup from with userspace
@@ -14,6 +16,11 @@
struct config {
__u32 action;
int ifindex;
+ __u32 options;
+};
+enum cfg_options_flags {
+ NO_TOUCH = 0x0U,
+ READ_MEM = 0x1U,
};
struct bpf_map_def SEC("maps") config_map = {
.type = BPF_MAP_TYPE_ARRAY,
@@ -90,6 +97,18 @@ int xdp_prognum0(struct xdp_md *ctx)
if (key == MAX_RXQs)
rxq_rec->issue++;
+ /* Default: Don't touch packet data, only count packets */
+ if (unlikely(config->options & READ_MEM)) {
+ struct ethhdr *eth = data;
+
+ if (eth + 1 > data_end)
+ return XDP_ABORTED;
+
+ /* Avoid compiler removing this: Drop non 802.3 Ethertypes */
+ if (ntohs(eth->h_proto) < ETH_P_802_3_MIN)
+ return XDP_ABORTED;
+ }
+
return config->action;
}
diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index e4e9ba52bff0..435485d4f49e 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -50,6 +50,7 @@ static const struct option long_options[] = {
{"sec", required_argument, NULL, 's' },
{"no-separators", no_argument, NULL, 'z' },
{"action", required_argument, NULL, 'a' },
+ {"readmem", no_argument, NULL, 'r' },
{0, 0, NULL, 0 }
};
@@ -66,6 +67,11 @@ static void int_exit(int sig)
struct config {
__u32 action;
int ifindex;
+ __u32 options;
+};
+enum cfg_options_flags {
+ NO_TOUCH = 0x0U,
+ READ_MEM = 0x1U,
};
#define XDP_ACTION_MAX (XDP_TX + 1)
#define XDP_ACTION_MAX_STRLEN 11
@@ -109,6 +115,16 @@ static void list_xdp_actions(void)
printf("\n");
}
+static char* options2str(enum cfg_options_flags flag)
+{
+ if (flag == NO_TOUCH)
+ return "no_touch";
+ if (flag & READ_MEM)
+ return "read";
+ fprintf(stderr, "ERR: Unknown config option flags");
+ exit(EXIT_FAIL);
+}
+
static void usage(char *argv[])
{
int i;
@@ -305,7 +321,7 @@ static __u64 calc_errs_pps(struct datarec *r,
static void stats_print(struct stats_record *stats_rec,
struct stats_record *stats_prev,
- int action)
+ int action, __u32 cfg_opt)
{
unsigned int nr_rxqs = bpf_map__def(rx_queue_index_map)->max_entries;
unsigned int nr_cpus = bpf_num_possible_cpus();
@@ -316,8 +332,8 @@ static void stats_print(struct stats_record *stats_rec,
int i;
/* Header */
- printf("\nRunning XDP on dev:%s (ifindex:%d) action:%s\n",
- ifname, ifindex, action2str(action));
+ printf("\nRunning XDP on dev:%s (ifindex:%d) action:%s options:%s\n",
+ ifname, ifindex, action2str(action), options2str(cfg_opt));
/* stats_global_map */
{
@@ -399,7 +415,7 @@ static inline void swap(struct stats_record **a, struct stats_record **b)
*b = tmp;
}
-static void stats_poll(int interval, int action)
+static void stats_poll(int interval, int action, __u32 cfg_opt)
{
struct stats_record *record, *prev;
@@ -410,7 +426,7 @@ static void stats_poll(int interval, int action)
while (1) {
swap(&prev, &record);
stats_collect(record);
- stats_print(record, prev, action);
+ stats_print(record, prev, action, cfg_opt);
sleep(interval);
}
@@ -421,6 +437,7 @@ static void stats_poll(int interval, int action)
int main(int argc, char **argv)
{
+ __u32 cfg_options= NO_TOUCH ; /* Default: Don't touch packet memory */
struct rlimit r = {10 * 1024 * 1024, RLIM_INFINITY};
struct bpf_prog_load_attr prog_load_attr = {
.prog_type = BPF_PROG_TYPE_XDP,
@@ -435,6 +452,7 @@ int main(int argc, char **argv)
int interval = 2;
__u32 key = 0;
+
char action_str_buf[XDP_ACTION_MAX_STRLEN + 1 /* for \0 */] = { 0 };
int action = XDP_PASS; /* Default action */
char *action_str = NULL;
@@ -496,6 +514,9 @@ int main(int argc, char **argv)
action_str = (char *)&action_str_buf;
strncpy(action_str, optarg, XDP_ACTION_MAX_STRLEN);
break;
+ case 'r':
+ cfg_options |= READ_MEM;
+ break;
case 'h':
error:
default:
@@ -522,6 +543,7 @@ int main(int argc, char **argv)
}
}
cfg.action = action;
+ cfg.options = cfg_options;
/* Trick to pretty printf with thousands separators use %' */
if (use_separators)
@@ -542,6 +564,6 @@ int main(int argc, char **argv)
return EXIT_FAIL_XDP;
}
- stats_poll(interval, action);
+ stats_poll(interval, action, cfg_options);
return EXIT_OK;
}
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [bpf-next PATCH 2/2] samples/bpf: xdp_rxq_info action XDP_TX must adjust MAC-addrs
2018-06-25 14:27 [bpf-next PATCH 0/2] xdp/bpf: extend XDP samples/bpf xdp_rxq_info Jesper Dangaard Brouer
2018-06-25 14:27 ` [bpf-next PATCH 1/2] samples/bpf: extend xdp_rxq_info to read packet payload Jesper Dangaard Brouer
@ 2018-06-25 14:27 ` Jesper Dangaard Brouer
2018-06-27 0:09 ` Song Liu
2018-06-28 21:54 ` [bpf-next PATCH 0/2] xdp/bpf: extend XDP samples/bpf xdp_rxq_info Daniel Borkmann
2 siblings, 1 reply; 10+ messages in thread
From: Jesper Dangaard Brouer @ 2018-06-25 14:27 UTC (permalink / raw)
To: netdev, Jesper Dangaard Brouer
Cc: Daniel Borkmann, Toke Høiland-Jørgensen, Alexei Starovoitov
XDP_TX requires also changing the MAC-addrs, else some hardware
may drop the TX packet before reaching the wire. This was
observed with driver mlx5.
If xdp_rxq_info select --action XDP_TX the swapmac functionality
is activated. It is also possible to manually enable via cmdline
option --swapmac. This is practical if wanting to measure the
overhead of writing/updating payload for other action types.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
samples/bpf/xdp_rxq_info_kern.c | 26 +++++++++++++++++++++++++-
samples/bpf/xdp_rxq_info_user.c | 11 +++++++++++
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/samples/bpf/xdp_rxq_info_kern.c b/samples/bpf/xdp_rxq_info_kern.c
index 61af6210df2f..222a83eed1cb 100644
--- a/samples/bpf/xdp_rxq_info_kern.c
+++ b/samples/bpf/xdp_rxq_info_kern.c
@@ -21,6 +21,7 @@ struct config {
enum cfg_options_flags {
NO_TOUCH = 0x0U,
READ_MEM = 0x1U,
+ SWAP_MAC = 0x2U,
};
struct bpf_map_def SEC("maps") config_map = {
.type = BPF_MAP_TYPE_ARRAY,
@@ -52,6 +53,23 @@ struct bpf_map_def SEC("maps") rx_queue_index_map = {
.max_entries = MAX_RXQs + 1,
};
+static __always_inline
+void swap_src_dst_mac(void *data)
+{
+ unsigned short *p = data;
+ unsigned short dst[3];
+
+ dst[0] = p[0];
+ dst[1] = p[1];
+ dst[2] = p[2];
+ p[0] = p[3];
+ p[1] = p[4];
+ p[2] = p[5];
+ p[3] = dst[0];
+ p[4] = dst[1];
+ p[5] = dst[2];
+}
+
SEC("xdp_prog0")
int xdp_prognum0(struct xdp_md *ctx)
{
@@ -98,7 +116,7 @@ int xdp_prognum0(struct xdp_md *ctx)
rxq_rec->issue++;
/* Default: Don't touch packet data, only count packets */
- if (unlikely(config->options & READ_MEM)) {
+ if (unlikely(config->options & (READ_MEM|SWAP_MAC))) {
struct ethhdr *eth = data;
if (eth + 1 > data_end)
@@ -107,6 +125,12 @@ int xdp_prognum0(struct xdp_md *ctx)
/* Avoid compiler removing this: Drop non 802.3 Ethertypes */
if (ntohs(eth->h_proto) < ETH_P_802_3_MIN)
return XDP_ABORTED;
+
+ /* XDP_TX requires changing MAC-addrs, else HW may drop.
+ * Can also be enabled with --swapmac (for test purposes)
+ */
+ if (unlikely(config->options & SWAP_MAC))
+ swap_src_dst_mac(data);
}
return config->action;
diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index 435485d4f49e..248a7eab9531 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -51,6 +51,7 @@ static const struct option long_options[] = {
{"no-separators", no_argument, NULL, 'z' },
{"action", required_argument, NULL, 'a' },
{"readmem", no_argument, NULL, 'r' },
+ {"swapmac", no_argument, NULL, 'm' },
{0, 0, NULL, 0 }
};
@@ -72,6 +73,7 @@ struct config {
enum cfg_options_flags {
NO_TOUCH = 0x0U,
READ_MEM = 0x1U,
+ SWAP_MAC = 0x2U,
};
#define XDP_ACTION_MAX (XDP_TX + 1)
#define XDP_ACTION_MAX_STRLEN 11
@@ -119,6 +121,8 @@ static char* options2str(enum cfg_options_flags flag)
{
if (flag == NO_TOUCH)
return "no_touch";
+ if (flag & SWAP_MAC)
+ return "swapmac";
if (flag & READ_MEM)
return "read";
fprintf(stderr, "ERR: Unknown config option flags");
@@ -517,6 +521,9 @@ int main(int argc, char **argv)
case 'r':
cfg_options |= READ_MEM;
break;
+ case 'm':
+ cfg_options |= SWAP_MAC;
+ break;
case 'h':
error:
default:
@@ -543,6 +550,10 @@ int main(int argc, char **argv)
}
}
cfg.action = action;
+
+ /* XDP_TX requires changing MAC-addrs, else HW may drop */
+ if (action == XDP_TX)
+ cfg_options |= SWAP_MAC;
cfg.options = cfg_options;
/* Trick to pretty printf with thousands separators use %' */
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [bpf-next PATCH 1/2] samples/bpf: extend xdp_rxq_info to read packet payload
2018-06-25 14:27 ` [bpf-next PATCH 1/2] samples/bpf: extend xdp_rxq_info to read packet payload Jesper Dangaard Brouer
@ 2018-06-26 23:53 ` Song Liu
2018-06-27 11:23 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 10+ messages in thread
From: Song Liu @ 2018-06-26 23:53 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Networking, Daniel Borkmann, Toke Høiland-Jørgensen,
Alexei Starovoitov
On Mon, Jun 25, 2018 at 7:27 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> There is a cost associated with reading the packet data payload
> that this test ignored. Add option --read to allow enabling
> reading part of the payload.
>
> This sample/tool helps us analyse an issue observed with a NIC
> mlx5 (ConnectX-5 Ex) and an Intel(R) Xeon(R) CPU E5-1650 v4.
>
> With no_touch of data:
>
> Running XDP on dev:mlx5p1 (ifindex:8) action:XDP_DROP options:no_touch
> XDP stats CPU pps issue-pps
> XDP-RX CPU 0 14,465,157 0
> XDP-RX CPU 1 14,464,728 0
> XDP-RX CPU 2 14,465,283 0
> XDP-RX CPU 3 14,465,282 0
> XDP-RX CPU 4 14,464,159 0
> XDP-RX CPU 5 14,465,379 0
> XDP-RX CPU total 86,789,992
>
> When not touching data, we observe that the CPUs have idle cycles.
> When reading data the CPUs are 100% busy in softirq.
>
> With reading data:
>
> Running XDP on dev:mlx5p1 (ifindex:8) action:XDP_DROP options:read
> XDP stats CPU pps issue-pps
> XDP-RX CPU 0 9,620,639 0
> XDP-RX CPU 1 9,489,843 0
> XDP-RX CPU 2 9,407,854 0
> XDP-RX CPU 3 9,422,289 0
> XDP-RX CPU 4 9,321,959 0
> XDP-RX CPU 5 9,395,242 0
> XDP-RX CPU total 56,657,828
>
> The effect seen above is a result of cache-misses occuring when
> more RXQs are being used. Based on perf-event observations, our
> conclusion is that the CPUs DDIO (Direct Data I/O) choose to
> deliver packet into main memory, instead of L3-cache. We also
> found, that this can be mitigated by either using less RXQs or by
> reducing NICs the RX-ring size.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
> ---
> samples/bpf/xdp_rxq_info_kern.c | 19 +++++++++++++++++++
> samples/bpf/xdp_rxq_info_user.c | 34 ++++++++++++++++++++++++++++------
> 2 files changed, 47 insertions(+), 6 deletions(-)
>
> diff --git a/samples/bpf/xdp_rxq_info_kern.c b/samples/bpf/xdp_rxq_info_kern.c
> index 3fd209291653..61af6210df2f 100644
> --- a/samples/bpf/xdp_rxq_info_kern.c
> +++ b/samples/bpf/xdp_rxq_info_kern.c
> @@ -4,6 +4,8 @@
> * Example howto extract XDP RX-queue info
> */
> #include <uapi/linux/bpf.h>
> +#include <uapi/linux/if_ether.h>
> +#include <uapi/linux/in.h>
> #include "bpf_helpers.h"
>
> /* Config setup from with userspace
> @@ -14,6 +16,11 @@
> struct config {
> __u32 action;
> int ifindex;
> + __u32 options;
> +};
> +enum cfg_options_flags {
> + NO_TOUCH = 0x0U,
> + READ_MEM = 0x1U,
> };
> struct bpf_map_def SEC("maps") config_map = {
> .type = BPF_MAP_TYPE_ARRAY,
> @@ -90,6 +97,18 @@ int xdp_prognum0(struct xdp_md *ctx)
> if (key == MAX_RXQs)
> rxq_rec->issue++;
>
> + /* Default: Don't touch packet data, only count packets */
> + if (unlikely(config->options & READ_MEM)) {
> + struct ethhdr *eth = data;
> +
> + if (eth + 1 > data_end)
> + return XDP_ABORTED;
> +
> + /* Avoid compiler removing this: Drop non 802.3 Ethertypes */
> + if (ntohs(eth->h_proto) < ETH_P_802_3_MIN)
> + return XDP_ABORTED;
> + }
> +
> return config->action;
> }
>
> diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
> index e4e9ba52bff0..435485d4f49e 100644
> --- a/samples/bpf/xdp_rxq_info_user.c
> +++ b/samples/bpf/xdp_rxq_info_user.c
> @@ -50,6 +50,7 @@ static const struct option long_options[] = {
> {"sec", required_argument, NULL, 's' },
> {"no-separators", no_argument, NULL, 'z' },
> {"action", required_argument, NULL, 'a' },
> + {"readmem", no_argument, NULL, 'r' },
> {0, 0, NULL, 0 }
> };
>
> @@ -66,6 +67,11 @@ static void int_exit(int sig)
> struct config {
> __u32 action;
> int ifindex;
> + __u32 options;
> +};
> +enum cfg_options_flags {
> + NO_TOUCH = 0x0U,
> + READ_MEM = 0x1U,
> };
> #define XDP_ACTION_MAX (XDP_TX + 1)
> #define XDP_ACTION_MAX_STRLEN 11
> @@ -109,6 +115,16 @@ static void list_xdp_actions(void)
> printf("\n");
> }
>
> +static char* options2str(enum cfg_options_flags flag)
> +{
> + if (flag == NO_TOUCH)
> + return "no_touch";
> + if (flag & READ_MEM)
> + return "read";
> + fprintf(stderr, "ERR: Unknown config option flags");
> + exit(EXIT_FAIL);
> +}
> +
enum cfg_options_flags is used as a bitmap in other parts of the sample.
So this function is a little weird (with more flags added).
Thanks,
Song
> static void usage(char *argv[])
> {
> int i;
> @@ -305,7 +321,7 @@ static __u64 calc_errs_pps(struct datarec *r,
>
> static void stats_print(struct stats_record *stats_rec,
> struct stats_record *stats_prev,
> - int action)
> + int action, __u32 cfg_opt)
> {
> unsigned int nr_rxqs = bpf_map__def(rx_queue_index_map)->max_entries;
> unsigned int nr_cpus = bpf_num_possible_cpus();
> @@ -316,8 +332,8 @@ static void stats_print(struct stats_record *stats_rec,
> int i;
>
> /* Header */
> - printf("\nRunning XDP on dev:%s (ifindex:%d) action:%s\n",
> - ifname, ifindex, action2str(action));
> + printf("\nRunning XDP on dev:%s (ifindex:%d) action:%s options:%s\n",
> + ifname, ifindex, action2str(action), options2str(cfg_opt));
>
> /* stats_global_map */
> {
> @@ -399,7 +415,7 @@ static inline void swap(struct stats_record **a, struct stats_record **b)
> *b = tmp;
> }
>
> -static void stats_poll(int interval, int action)
> +static void stats_poll(int interval, int action, __u32 cfg_opt)
> {
> struct stats_record *record, *prev;
>
> @@ -410,7 +426,7 @@ static void stats_poll(int interval, int action)
> while (1) {
> swap(&prev, &record);
> stats_collect(record);
> - stats_print(record, prev, action);
> + stats_print(record, prev, action, cfg_opt);
> sleep(interval);
> }
>
> @@ -421,6 +437,7 @@ static void stats_poll(int interval, int action)
>
> int main(int argc, char **argv)
> {
> + __u32 cfg_options= NO_TOUCH ; /* Default: Don't touch packet memory */
> struct rlimit r = {10 * 1024 * 1024, RLIM_INFINITY};
> struct bpf_prog_load_attr prog_load_attr = {
> .prog_type = BPF_PROG_TYPE_XDP,
> @@ -435,6 +452,7 @@ int main(int argc, char **argv)
> int interval = 2;
> __u32 key = 0;
>
> +
> char action_str_buf[XDP_ACTION_MAX_STRLEN + 1 /* for \0 */] = { 0 };
> int action = XDP_PASS; /* Default action */
> char *action_str = NULL;
> @@ -496,6 +514,9 @@ int main(int argc, char **argv)
> action_str = (char *)&action_str_buf;
> strncpy(action_str, optarg, XDP_ACTION_MAX_STRLEN);
> break;
> + case 'r':
> + cfg_options |= READ_MEM;
> + break;
> case 'h':
> error:
> default:
> @@ -522,6 +543,7 @@ int main(int argc, char **argv)
> }
> }
> cfg.action = action;
> + cfg.options = cfg_options;
>
> /* Trick to pretty printf with thousands separators use %' */
> if (use_separators)
> @@ -542,6 +564,6 @@ int main(int argc, char **argv)
> return EXIT_FAIL_XDP;
> }
>
> - stats_poll(interval, action);
> + stats_poll(interval, action, cfg_options);
> return EXIT_OK;
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [bpf-next PATCH 2/2] samples/bpf: xdp_rxq_info action XDP_TX must adjust MAC-addrs
2018-06-25 14:27 ` [bpf-next PATCH 2/2] samples/bpf: xdp_rxq_info action XDP_TX must adjust MAC-addrs Jesper Dangaard Brouer
@ 2018-06-27 0:09 ` Song Liu
2018-06-27 11:20 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 10+ messages in thread
From: Song Liu @ 2018-06-27 0:09 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Networking, Daniel Borkmann, Toke Høiland-Jørgensen,
Alexei Starovoitov
On Mon, Jun 25, 2018 at 7:27 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> XDP_TX requires also changing the MAC-addrs, else some hardware
> may drop the TX packet before reaching the wire. This was
> observed with driver mlx5.
>
> If xdp_rxq_info select --action XDP_TX the swapmac functionality
> is activated. It is also possible to manually enable via cmdline
> option --swapmac. This is practical if wanting to measure the
> overhead of writing/updating payload for other action types.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
> ---
> samples/bpf/xdp_rxq_info_kern.c | 26 +++++++++++++++++++++++++-
> samples/bpf/xdp_rxq_info_user.c | 11 +++++++++++
> 2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/samples/bpf/xdp_rxq_info_kern.c b/samples/bpf/xdp_rxq_info_kern.c
> index 61af6210df2f..222a83eed1cb 100644
> --- a/samples/bpf/xdp_rxq_info_kern.c
> +++ b/samples/bpf/xdp_rxq_info_kern.c
> @@ -21,6 +21,7 @@ struct config {
> enum cfg_options_flags {
> NO_TOUCH = 0x0U,
> READ_MEM = 0x1U,
> + SWAP_MAC = 0x2U,
> };
> struct bpf_map_def SEC("maps") config_map = {
> .type = BPF_MAP_TYPE_ARRAY,
> @@ -52,6 +53,23 @@ struct bpf_map_def SEC("maps") rx_queue_index_map = {
> .max_entries = MAX_RXQs + 1,
> };
>
> +static __always_inline
> +void swap_src_dst_mac(void *data)
> +{
> + unsigned short *p = data;
> + unsigned short dst[3];
> +
> + dst[0] = p[0];
> + dst[1] = p[1];
> + dst[2] = p[2];
> + p[0] = p[3];
> + p[1] = p[4];
> + p[2] = p[5];
> + p[3] = dst[0];
> + p[4] = dst[1];
> + p[5] = dst[2];
> +}
> +
> SEC("xdp_prog0")
> int xdp_prognum0(struct xdp_md *ctx)
> {
> @@ -98,7 +116,7 @@ int xdp_prognum0(struct xdp_md *ctx)
> rxq_rec->issue++;
>
> /* Default: Don't touch packet data, only count packets */
> - if (unlikely(config->options & READ_MEM)) {
> + if (unlikely(config->options & (READ_MEM|SWAP_MAC))) {
> struct ethhdr *eth = data;
>
> if (eth + 1 > data_end)
> @@ -107,6 +125,12 @@ int xdp_prognum0(struct xdp_md *ctx)
> /* Avoid compiler removing this: Drop non 802.3 Ethertypes */
> if (ntohs(eth->h_proto) < ETH_P_802_3_MIN)
> return XDP_ABORTED;
> +
> + /* XDP_TX requires changing MAC-addrs, else HW may drop.
> + * Can also be enabled with --swapmac (for test purposes)
> + */
> + if (unlikely(config->options & SWAP_MAC))
> + swap_src_dst_mac(data);
> }
>
> return config->action;
> diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
> index 435485d4f49e..248a7eab9531 100644
> --- a/samples/bpf/xdp_rxq_info_user.c
> +++ b/samples/bpf/xdp_rxq_info_user.c
> @@ -51,6 +51,7 @@ static const struct option long_options[] = {
> {"no-separators", no_argument, NULL, 'z' },
> {"action", required_argument, NULL, 'a' },
> {"readmem", no_argument, NULL, 'r' },
> + {"swapmac", no_argument, NULL, 'm' },
> {0, 0, NULL, 0 }
> };
>
> @@ -72,6 +73,7 @@ struct config {
> enum cfg_options_flags {
> NO_TOUCH = 0x0U,
> READ_MEM = 0x1U,
> + SWAP_MAC = 0x2U,
> };
> #define XDP_ACTION_MAX (XDP_TX + 1)
> #define XDP_ACTION_MAX_STRLEN 11
> @@ -119,6 +121,8 @@ static char* options2str(enum cfg_options_flags flag)
> {
> if (flag == NO_TOUCH)
> return "no_touch";
> + if (flag & SWAP_MAC)
> + return "swapmac";
> if (flag & READ_MEM)
> return "read";
I guess SWAP_MAC also reads the memory, so it "includes" READ_MEM?
It is OK for now. We may need to refactor this part when adding other
flags in the
future.
Thanks,
Song
> fprintf(stderr, "ERR: Unknown config option flags");
> @@ -517,6 +521,9 @@ int main(int argc, char **argv)
> case 'r':
> cfg_options |= READ_MEM;
> break;
> + case 'm':
> + cfg_options |= SWAP_MAC;
> + break;
> case 'h':
> error:
> default:
> @@ -543,6 +550,10 @@ int main(int argc, char **argv)
> }
> }
> cfg.action = action;
> +
> + /* XDP_TX requires changing MAC-addrs, else HW may drop */
> + if (action == XDP_TX)
> + cfg_options |= SWAP_MAC;
> cfg.options = cfg_options;
>
> /* Trick to pretty printf with thousands separators use %' */
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [bpf-next PATCH 2/2] samples/bpf: xdp_rxq_info action XDP_TX must adjust MAC-addrs
2018-06-27 0:09 ` Song Liu
@ 2018-06-27 11:20 ` Jesper Dangaard Brouer
2018-06-27 17:02 ` Song Liu
0 siblings, 1 reply; 10+ messages in thread
From: Jesper Dangaard Brouer @ 2018-06-27 11:20 UTC (permalink / raw)
To: Song Liu
Cc: Networking, Daniel Borkmann, Toke Høiland-Jørgensen,
Alexei Starovoitov, brouer
On Tue, 26 Jun 2018 17:09:01 -0700
Song Liu <liu.song.a23@gmail.com> wrote:
> On Mon, Jun 25, 2018 at 7:27 AM, Jesper Dangaard Brouer
> <brouer@redhat.com> wrote:
> > XDP_TX requires also changing the MAC-addrs, else some hardware
> > may drop the TX packet before reaching the wire. This was
> > observed with driver mlx5.
> >
> > If xdp_rxq_info select --action XDP_TX the swapmac functionality
> > is activated. It is also possible to manually enable via cmdline
> > option --swapmac. This is practical if wanting to measure the
> > overhead of writing/updating payload for other action types.
> >
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
> > ---
[...]
> >
> > diff --git a/samples/bpf/xdp_rxq_info_kern.c b/samples/bpf/xdp_rxq_info_kern.c
> > index 61af6210df2f..222a83eed1cb 100644
> > --- a/samples/bpf/xdp_rxq_info_kern.c
> > +++ b/samples/bpf/xdp_rxq_info_kern.c
> > @@ -21,6 +21,7 @@ struct config {
> > enum cfg_options_flags {
> > NO_TOUCH = 0x0U,
> > READ_MEM = 0x1U,
> > + SWAP_MAC = 0x2U,
> > };
[...]
> > @@ -98,7 +116,7 @@ int xdp_prognum0(struct xdp_md *ctx)
> > rxq_rec->issue++;
> >
> > /* Default: Don't touch packet data, only count packets */
> > - if (unlikely(config->options & READ_MEM)) {
> > + if (unlikely(config->options & (READ_MEM|SWAP_MAC))) {
> > struct ethhdr *eth = data;
> >
> > if (eth + 1 > data_end)
[...]
> > diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
> > index 435485d4f49e..248a7eab9531 100644
[...]
> > @@ -119,6 +121,8 @@ static char* options2str(enum cfg_options_flags flag)
> > {
> > if (flag == NO_TOUCH)
> > return "no_touch";
> > + if (flag & SWAP_MAC)
> > + return "swapmac";
> > if (flag & READ_MEM)
> > return "read";
>
> I guess SWAP_MAC also reads the memory, so it "includes" READ_MEM?
True (see _kern side)
> It is OK for now. We may need to refactor this part when adding other
> flags in the future.
Sure, do remember that this is only a 'sample' program.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [bpf-next PATCH 1/2] samples/bpf: extend xdp_rxq_info to read packet payload
2018-06-26 23:53 ` Song Liu
@ 2018-06-27 11:23 ` Jesper Dangaard Brouer
2018-06-27 17:03 ` Song Liu
0 siblings, 1 reply; 10+ messages in thread
From: Jesper Dangaard Brouer @ 2018-06-27 11:23 UTC (permalink / raw)
To: Song Liu
Cc: Networking, Daniel Borkmann, Toke Høiland-Jørgensen,
Alexei Starovoitov, brouer
On Tue, 26 Jun 2018 16:53:15 -0700
Song Liu <liu.song.a23@gmail.com> wrote:
> > +static char* options2str(enum cfg_options_flags flag)
> > +{
> > + if (flag == NO_TOUCH)
> > + return "no_touch";
> > + if (flag & READ_MEM)
> > + return "read";
> > + fprintf(stderr, "ERR: Unknown config option flags");
> > + exit(EXIT_FAIL);
> > +}
> > +
>
> enum cfg_options_flags is used as a bitmap in other parts of the sample.
> So this function is a little weird (with more flags added).
Sure, and do I handle this correctly in the next patch.
I'm uncertain what you want me to change?
Do you want me to drop the enum, and use #define instead?
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [bpf-next PATCH 2/2] samples/bpf: xdp_rxq_info action XDP_TX must adjust MAC-addrs
2018-06-27 11:20 ` Jesper Dangaard Brouer
@ 2018-06-27 17:02 ` Song Liu
0 siblings, 0 replies; 10+ messages in thread
From: Song Liu @ 2018-06-27 17:02 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Networking, Daniel Borkmann, Toke Høiland-Jørgensen,
Alexei Starovoitov
On Wed, Jun 27, 2018 at 4:20 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Tue, 26 Jun 2018 17:09:01 -0700
> Song Liu <liu.song.a23@gmail.com> wrote:
>
>> On Mon, Jun 25, 2018 at 7:27 AM, Jesper Dangaard Brouer
>> <brouer@redhat.com> wrote:
>> > XDP_TX requires also changing the MAC-addrs, else some hardware
>> > may drop the TX packet before reaching the wire. This was
>> > observed with driver mlx5.
>> >
>> > If xdp_rxq_info select --action XDP_TX the swapmac functionality
>> > is activated. It is also possible to manually enable via cmdline
>> > option --swapmac. This is practical if wanting to measure the
>> > overhead of writing/updating payload for other action types.
>> >
>> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>> > Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>> > ---
> [...]
>> >
>> > diff --git a/samples/bpf/xdp_rxq_info_kern.c b/samples/bpf/xdp_rxq_info_kern.c
>> > index 61af6210df2f..222a83eed1cb 100644
>> > --- a/samples/bpf/xdp_rxq_info_kern.c
>> > +++ b/samples/bpf/xdp_rxq_info_kern.c
>> > @@ -21,6 +21,7 @@ struct config {
>> > enum cfg_options_flags {
>> > NO_TOUCH = 0x0U,
>> > READ_MEM = 0x1U,
>> > + SWAP_MAC = 0x2U,
>> > };
> [...]
>> > @@ -98,7 +116,7 @@ int xdp_prognum0(struct xdp_md *ctx)
>> > rxq_rec->issue++;
>> >
>> > /* Default: Don't touch packet data, only count packets */
>> > - if (unlikely(config->options & READ_MEM)) {
>> > + if (unlikely(config->options & (READ_MEM|SWAP_MAC))) {
>> > struct ethhdr *eth = data;
>> >
>> > if (eth + 1 > data_end)
> [...]
>
>> > diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
>> > index 435485d4f49e..248a7eab9531 100644
> [...]
>> > @@ -119,6 +121,8 @@ static char* options2str(enum cfg_options_flags flag)
>> > {
>> > if (flag == NO_TOUCH)
>> > return "no_touch";
>> > + if (flag & SWAP_MAC)
>> > + return "swapmac";
>> > if (flag & READ_MEM)
>> > return "read";
>>
>> I guess SWAP_MAC also reads the memory, so it "includes" READ_MEM?
>
> True (see _kern side)
>
>> It is OK for now. We may need to refactor this part when adding other
>> flags in the future.
>
> Sure, do remember that this is only a 'sample' program.
Agreed.
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [bpf-next PATCH 1/2] samples/bpf: extend xdp_rxq_info to read packet payload
2018-06-27 11:23 ` Jesper Dangaard Brouer
@ 2018-06-27 17:03 ` Song Liu
0 siblings, 0 replies; 10+ messages in thread
From: Song Liu @ 2018-06-27 17:03 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Networking, Daniel Borkmann, Toke Høiland-Jørgensen,
Alexei Starovoitov
On Wed, Jun 27, 2018 at 4:23 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Tue, 26 Jun 2018 16:53:15 -0700
> Song Liu <liu.song.a23@gmail.com> wrote:
>
>> > +static char* options2str(enum cfg_options_flags flag)
>> > +{
>> > + if (flag == NO_TOUCH)
>> > + return "no_touch";
>> > + if (flag & READ_MEM)
>> > + return "read";
>> > + fprintf(stderr, "ERR: Unknown config option flags");
>> > + exit(EXIT_FAIL);
>> > +}
>> > +
>>
>> enum cfg_options_flags is used as a bitmap in other parts of the sample.
>> So this function is a little weird (with more flags added).
>
> Sure, and do I handle this correctly in the next patch.
>
> I'm uncertain what you want me to change?
> Do you want me to drop the enum, and use #define instead?
I think it is good as-is for sample code.
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [bpf-next PATCH 0/2] xdp/bpf: extend XDP samples/bpf xdp_rxq_info
2018-06-25 14:27 [bpf-next PATCH 0/2] xdp/bpf: extend XDP samples/bpf xdp_rxq_info Jesper Dangaard Brouer
2018-06-25 14:27 ` [bpf-next PATCH 1/2] samples/bpf: extend xdp_rxq_info to read packet payload Jesper Dangaard Brouer
2018-06-25 14:27 ` [bpf-next PATCH 2/2] samples/bpf: xdp_rxq_info action XDP_TX must adjust MAC-addrs Jesper Dangaard Brouer
@ 2018-06-28 21:54 ` Daniel Borkmann
2 siblings, 0 replies; 10+ messages in thread
From: Daniel Borkmann @ 2018-06-28 21:54 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev
Cc: Daniel Borkmann, Toke Høiland-Jørgensen, Alexei Starovoitov
On 06/25/2018 04:27 PM, Jesper Dangaard Brouer wrote:
> While writing an article about XDP, the samples/bpf xdp_rxq_info
> program were extended to cover some more use-cases.
Applied to bpf-next, thanks guys!
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-06-28 21:54 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-25 14:27 [bpf-next PATCH 0/2] xdp/bpf: extend XDP samples/bpf xdp_rxq_info Jesper Dangaard Brouer
2018-06-25 14:27 ` [bpf-next PATCH 1/2] samples/bpf: extend xdp_rxq_info to read packet payload Jesper Dangaard Brouer
2018-06-26 23:53 ` Song Liu
2018-06-27 11:23 ` Jesper Dangaard Brouer
2018-06-27 17:03 ` Song Liu
2018-06-25 14:27 ` [bpf-next PATCH 2/2] samples/bpf: xdp_rxq_info action XDP_TX must adjust MAC-addrs Jesper Dangaard Brouer
2018-06-27 0:09 ` Song Liu
2018-06-27 11:20 ` Jesper Dangaard Brouer
2018-06-27 17:02 ` Song Liu
2018-06-28 21:54 ` [bpf-next PATCH 0/2] xdp/bpf: extend XDP samples/bpf xdp_rxq_info Daniel Borkmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.