On Mon, Jun 25, 2018 at 7:27 AM, Jesper Dangaard Brouer
<bro...@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 <bro...@redhat.com>
> Signed-off-by: Toke Høiland-Jørgensen <t...@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 %' */
>
