On 1/17/19 7:40 AM, Matt Ellison wrote:
> +static int xfrm_parse_opt(struct link_util *lu, int argc, char **argv,
> + struct nlmsghdr *n)
> +{
> + unsigned int link = 0;
> + __u32 if_id = 0;
> +
> + while (argc > 0) {
> + if (!matches(*argv, "dev")) {
> + NEXT_ARG();
> + link = ll_name_to_index(*argv);
> + if (!link)
> + exit(nodev(*argv));
> + } else if (!matches(*argv, "if_id")) {
> + NEXT_ARG();
> + if (get_u32(&if_id, *argv, 0))
> + invarg("if_id", *argv);
> + } else {
> + xfrm_print_help(lu, argc, argv, stderr);
> + return -1;
> + }
> + argc--; argv++;
> + }
> +
> + addattr32(n, 1024, IFLA_XFRM_IF_ID, if_id);
You always add IF_ID even if not set by user. The kernel code does not
appear to require it so why pass a default value?
> +
> + if (link) {
> + addattr32(n, 1024, IFLA_XFRM_LINK, link);
> + } else {
> + fprintf(stderr, "must specify physical device\n");
> + return -1;
> + }
The kernel code does appear to require this parameter, so why have this
requirement in iproute2?
> +
> + return 0;
> +}
> +
> +static void xfrm_print_opt(struct link_util *lu, FILE *f, struct rtattr
> *tb[])
> +{
> +
> + if (!tb)
> + return;
> +
> + if (tb[IFLA_XFRM_IF_ID]) {
> + __u32 id = rta_getattr_u32(tb[IFLA_XFRM_IF_ID]);
> +
> + print_0xhex(PRINT_ANY, "if_id", "if_id %#llx ", id);
> +
> + }
What about IFLA_XFRM_LINK?
> diff --git a/testsuite/tests/ip/link/add_type_xfrm.t
> b/testsuite/tests/ip/link/add_type_xfrm.t
> new file mode 100755
> index 00000000..78ce28e0
> --- /dev/null
> +++ b/testsuite/tests/ip/link/add_type_xfrm.t
> @@ -0,0 +1,32 @@
> +#!/bin/sh
> +
> +. lib/generic.sh
> +
> +ts_log "[Testing Add XFRM Interface, With IF-ID]"
> +
> +PHYS_DEV="lo"
> +NEW_DEV="$(rand_dev)"
> +IF_ID="0xf"
> +
> +ts_ip "$0" "Add $NEW_DEV xfrm interface" link add dev $NEW_DEV type xfrm
> dev $PHYS_DEV if_id $IF_ID
> +
> +ts_ip "$0" "Show $NEW_DEV xfrm interface" -d link show dev $NEW_DEV
> +test_on "$NEW_DEV"
> +test_on "if_id $IF_ID"
> +
> +ts_ip "$0" "Del $NEW_DEV xfrm interface" link del dev $NEW_DEV
> +
> +
> +ts_log "[Testing Add XFRM Interface, No IF-ID]"
> +
> +PHYS_DEV="lo"
> +NEW_DEV="$(rand_dev)"
> +IF_ID="0xf"
> +
> +ts_ip "$0" "Add $NEW_DEV xfrm interface" link add dev $NEW_DEV type xfrm
> dev $PHYS_DEV
> +
> +ts_ip "$0" "Show $NEW_DEV xfrm interface" -d link show dev $NEW_DEV
> +test_on "$NEW_DEV"
> +test_on_not "if_id $IF_ID"
> +
> +ts_ip "$0" "Del $NEW_DEV xfrm interface" link del dev $NEW_DEV
>
Thanks for adding test cases!
