On 12/15/17 6:06 PM, William Tu wrote: > @@ -343,6 +355,22 @@ get_failed: > invarg("invalid erspan index\n", *argv); > if (erspan_idx & ~((1<<20) - 1) || erspan_idx == 0) > invarg("erspan index must be > 0 and <= > 20-bit\n", *argv); > + } else if (strcmp(*argv, "erspan_ver") == 0) { > + NEXT_ARG(); > + if (get_u8(&erspan_ver, *argv, 0)) > + invarg("invalid erspan version\n", *argv); > + if (erspan_ver != 1 && erspan_ver != 2) > + invarg("erspan version must be 1 or 2\n", > *argv); > + } else if (strcmp(*argv, "erspan_dir") == 0) { > + NEXT_ARG(); > + if (get_u8(&erspan_dir, *argv, 0)) > + invarg("invalid erspan direction\n", *argv); > + if (erspan_dir != 0 && erspan_dir != 1) > + invarg("erspan direction must be 0(Ingress) or > 1(Egress)\n", *argv);
Why not allow ingress and egress as strings and using matches()? e.g., '... erspan_dir in[gress] ...' or '... erspan_dir e[gress] ...' Seems much more intuitive than remembering "0" = ingress and "1" = egress. > @@ -374,8 +402,15 @@ get_failed: > addattr_l(n, 1024, IFLA_GRE_TTL, &ttl, 1); > addattr_l(n, 1024, IFLA_GRE_TOS, &tos, 1); > addattr32(n, 1024, IFLA_GRE_FWMARK, fwmark); > - if (erspan_idx != 0) > - addattr32(n, 1024, IFLA_GRE_ERSPAN_INDEX, erspan_idx); > + if (erspan_ver) { > + addattr8(n, 1024, IFLA_GRE_ERSPAN_VER, erspan_ver); > + if (erspan_ver == 1 && erspan_idx != 0) { > + addattr32(n, 1024, IFLA_GRE_ERSPAN_INDEX, > erspan_idx); > + } else if (erspan_ver == 2) { > + addattr8(n, 1024, IFLA_GRE_ERSPAN_DIR, > erspan_dir); > + addattr16(n, 1024, IFLA_GRE_ERSPAN_HWID, > erspan_hwid); > + } > + } > } else { > addattr_l(n, 1024, IFLA_GRE_COLLECT_METADATA, NULL, 0); > } > @@ -514,7 +549,25 @@ static void gre_print_opt(struct link_util *lu, FILE *f, > struct rtattr *tb[]) > if (tb[IFLA_GRE_ERSPAN_INDEX]) { > __u32 erspan_idx = rta_getattr_u32(tb[IFLA_GRE_ERSPAN_INDEX]); > > - fprintf(f, "erspan_index %u ", erspan_idx); > + print_uint(PRINT_ANY, "erspan_index", "erspan_index %u", > erspan_idx); > + } > + > + if (tb[IFLA_GRE_ERSPAN_VER]) { > + __u8 erspan_ver = rta_getattr_u8(tb[IFLA_GRE_ERSPAN_VER]); > + > + print_uint(PRINT_ANY, "erspan_ver", "erspan_ver %u", > erspan_ver); > + } > + > + if (tb[IFLA_GRE_ERSPAN_DIR]) { > + __u8 erspan_dir = rta_getattr_u8(tb[IFLA_GRE_ERSPAN_DIR]); > + > + print_uint(PRINT_ANY, "erspan_dir", "erspan_dir %u", > erspan_dir); similarly, here can convert the 0/1 to a human string. Same comments for the changes to link_gre6.c