netdev - Re: [PATCH 2/2] IPVS: Add genetlink interface implementation

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <f4845fc0807100533h6867ed00x8975d156b15a866f@mail.gmail.com>
Date:	Thu, 10 Jul 2008 14:33:45 +0200
From:	"Julius Volz" <juliusv@...gle.com>
To:	"Thomas Graf" <tgraf@...g.ch>
Cc:	"Patrick McHardy" <kaber@...sh.net>, netdev@...r.kernel.org,
	lvs-devel@...r.kernel.org, vbusam@...gle.com, horms@...ge.net.au,
	davem@...emloft.net
Subject: Re: [PATCH 2/2] IPVS: Add genetlink interface implementation

On Thu, Jul 10, 2008 at 1:36 PM, Thomas Graf <tgraf@...g.ch> wrote:
> * Julius Volz <juliusv@...gle.com> 2008-07-10 13:20
>> +/* IPVS genetlink family */
>> +static struct genl_family ip_vs_genl_family = {
>> +     .id             = GENL_ID_GENERATE,
>> +     .hdrsize        = 0,
>> +     .name           = IPVS_GENL_NAME,
>> +     .version        = IPVS_GENL_VERSION,
>> +     .maxattr        = IPVS_CMD_MAX
>
> It's not a bug but looks like a typo, maxattr should
> specify the number of first level attributes.

Ah, this is how the family's attrbuf size is set. Looks like a bug
actually, but it hasn't affected anything because the command enum is
bigger than any of the first-level attribute enums. I might have
gotten this from net/irda/irnetlink.c, where it's also set to the
maximum command attribute value.

Note that I use different first level attributes depending on the
command. Rather than calculating the largest needed size, it's
probably best to join all attributes that may ever occur in the first
level into one big enum, right?

>> +static int ip_vs_genl_dump_service(struct sk_buff *skb, struct ip_vs_service *svc,
>> +                                struct netlink_callback *cb)
>> +{
>> +     void *hdr;
>> +
>> +     hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).pid, cb->nlh->nlmsg_seq,
>> +                       &ip_vs_genl_family, NLM_F_MULTI,
>> +                       IPVS_CMD_GET_SERVICES);
>
> Typically, netlink code follows the following semantics WRT to
> commands/message types:
> -> GET_SERVICE (NLM_F_DUMP)
> <- NEW_SERVICE
> <- NEW_SERVICE
> <- NEW_SERVICE

Ok, so I will set the answer message type to IPVS_CMD_NEW_SERVICE (and
accordingly in the other dump cases). For non-dump GET commands, is it
usual to have the response ID be the same as the request?

>> +static int ip_vs_genl_get_cmd(struct sk_buff *skb, struct genl_info *info)
>> +{
>> +     struct sk_buff *msg;
>> +     void *reply;
>> +     int ret, cmd;
>> +
>> +     mutex_lock(&__ip_vs_mutex);
>> +
>> +     cmd = info->genlhdr->cmd;
>> +
>> +     msg = nlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
>
> This should be nlmsg_new(NLMSG_DEFAULT_SIZE, ...) , NLMSG_GOODSIZE is for use
> with skb_alloc().

Ok, changed!

Thanks for the comments!

Julius

-- 
Google Switzerland GmbH
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html