netdev - Re: [PATCH v14 02/20] netlink: add new family to manage ULP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <253r0n47qvh.fsf@mtr-vdi-124.i-did-not-set--mail-host-address--so-tickle-me>
Date: Mon, 11 Sep 2023 16:59:14 +0300
From: Aurelien Aptel <aaptel@...dia.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: linux-nvme@...ts.infradead.org, netdev@...r.kernel.org,
 sagi@...mberg.me, hch@....de, kbusch@...nel.org, axboe@...com,
 chaitanyak@...dia.com, davem@...emloft.net, aurelien.aptel@...il.com,
 smalin@...dia.com, malin1024@...il.com, ogerlitz@...dia.com,
 yorayz@...dia.com, borisp@...dia.com, galshalom@...dia.com,
 mgurtovoy@...dia.com
Subject: Re: [PATCH v14 02/20] netlink: add new family to manage ULP_DDP
 enablement and stats

Jakub Kicinski <kuba@...nel.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