Re: [RFC 7/9] snet: introduce snet_netlink.c and snet_netlink.h

[Samir Bellabes <sam@synack.fr>]

Patrick McHardy <kaber@trash.net> writes:

> Samir Bellabes wrote:
>> +/**
>> + * snet_nl_register - Handle a REGISTER message
>> + * @skb: the NETLINK buffer
>> + * @info: the Generic NETLINK info block
>> + *
>> + * Description:
>> + * Notify the kernel that an application is listening for events.
>> + * Returns zero on success, negative values on failure.
>> + */
>> +static int snet_nl_register(struct sk_buff *skb, struct genl_info *info)
>> +{
>> +	int ret = -EINVAL;
>> +	u32 version = 0;
>> +	u8 set_resp_flag = 0;
>> +
>> +	atomic_set(&snet_nl_seq, info->snd_seq);
>> +
>> +	if (!info->attrs[SNET_A_VERSION])
>> +		return -EINVAL;
>> +	version = nla_get_u32(info->attrs[SNET_A_VERSION]);
>> +
>> +	if (version == SNET_VERSION) {	/* version is compliant */
>> +		atomic_inc(&snet_num_listeners);
>> +		set_resp_flag = 1;
>> +	}
>> +
>> +	ret = snet_nl_response_flag(info, &snet_genl_family,
>> +				    SNET_C_REGISTER, SNET_A_REGISTERED,
>> +				    set_resp_flag);
>
> Is this really needed? A return value of 0 should already tell userspace
> that the command was successful. If it really wants a seperate success
> message, it can use NLM_F_ACK. This will also automatically take care
> of using the proper sequence number, so the snet_nl_seq handling isn't
> required anymore. Same for all similar cases below.

Indeed, we can use the return value to inform the userspace about the
status of operations. I moved all the code, and deleted the no longer
used function snet_nl_response_flag()

Thanks Patrick,
sam