[PATCH v14 02/20] netlink: add new family to manage ULP_DDP enablement and stats

[Aurelien Aptel]

Jakub Kicinski <kuba at kernel.org> writes:
> Looks mostly good, but we lost dump support. TAL at netdev_nl_dev_get_dumpit(),
> the iteration is not that hard, I reckon we should support dumps.

Ok, we will add dumpit support for ULP_DDP_CMD_GET.

IIUC the dumpit should loop over all the netdev in its callback and
write a ULP_DDP_CMD_GET reply msg for each.

> Are the ifdefs still needed? Can't we compile out all of the family code
> if the config is not selected?

They are not, we will remove the ifdefs.

>> +
>
> nit pointless empty line.

Ok

>> +     if (!data->dev) {
>> +             kfree(data);
>> +             return -EINVAL;
>
> ENOENT ? Maybe also
>
>                 NL_SET_BAD_ATTR(info->extack, info->attrs[ULP_DDP_A_DEV_IFINDEX])
> ?

Ok, sounds good.

>> +     ret = apply_bits(data, &wanted, &wanted_mask, info->extack);
>> +     if (ret < 0)
>> +             return ret;
>
> leaks rsp

Oops, thanks for spotting that.

>> +     notify = !!ret;
>> +     ret = prepare_data(info, data, ULP_DDP_CMD_SET);
>> +     if (ret)
>> +             return ret;
>> +
>> +     ret = fill_data(rsp, data, ULP_DDP_CMD_SET, info->snd_portid, info->snd_seq, 0);
>
> fill_date() can probably use genlmsg_iput() ?

Probably yes, unless it makes the dumpit function more complex.

Thanks