Subject: Re: [RFC PATCH 13/24] net: rbridge: Add set_node function

From: Sergei Shtylyov <[email-masked]>
Date: Thu, 25 Sep 2014 15:34:32 +0400

Hello.

On 9/24/2014 7:52 PM, Ahmed Amamou wrote:

> allow daemon to set distant Rbridges information in local database
> daemon has to periodically update distant Rbridges informations in local database
> create a new node only if change on distant RBridge information are detected

> Signed-off-by: Ahmed Amamou <[email-masked]>
> Signed-off-by: Kamel Haddadou <[email-masked]>
> Signed-off-by: François Cachereul <[email-masked]>
> Signed-off-by: William Dauchy <[email-masked]>
> ---
>   net/bridge/rbridge/rbr_netlink.c | 78 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 78 insertions(+)

> diff --git a/net/bridge/rbridge/rbr_netlink.c b/net/bridge/rbridge/rbr_netlink.c
> index 889e6c8..bb893f0 100644
> --- a/net/bridge/rbridge/rbr_netlink.c
> +++ b/net/bridge/rbridge/rbr_netlink.c
> @@ -38,9 +38,87 @@ static struct genl_multicast_group trill_mcgrps[] = {
>   	[TRILL_MCGRP_OFFSET] = {.name = TRILL_MCAST_NAME,},
>   };
>
> +static int create_node(struct net_bridge_port *p, struct rbr *rbr,
> +		       struct rbr_nickinfo *rbr_ni_partial,
> +		       struct genl_info *info)
> +{
> +	size_t size = 0;
> +	size_t old_size = 0;
> +	struct rbr_node *old;
> +	struct rbr_nickinfo *rbr_ni;
> +
> +	if (rbr_ni_partial == NULL)
> +		return -EINVAL;
> +
> +	size = RBR_NI_TOTALSIZE(rbr_ni_partial);
> +	if (size > PAGE_SIZE - sizeof(struct trill_nl_header)) {
> +		pr_warn_ratelimited
> +		    ("create_node: size > (PAGE_SIZE-nickinfo_offset)\n");
> +		return -EINVAL;
> +	}
> +	rbr_ni = kzalloc(size, GFP_KERNEL);
> +	if (!rbr_ni)
> +		return -ENOMEM;
> +	old = rbr->rbr_nodes[rbr_ni_partial->nick];
> +	nla_memcpy(rbr_ni, info->attrs[TRILL_ATTR_BIN], size);
> +	if (old)
> +		old_size = RBR_NI_TOTALSIZE(old->rbr_ni);
> +	/* replace old node by a new one only if nickname information have changed */

    s/have/has/.

> +	if (old == NULL || old_size != size ||
> +	    memcmp(old->rbr_ni, rbr_ni, size)) {
> +		struct rbr_node *new;
> +
> +		new = kzalloc(sizeof(*old), GFP_KERNEL);
> +		if (!new) {
> +			kfree(rbr_ni);
> +			return -ENOMEM;
> +		}
> +		atomic_set(&new->refs, 1);
> +		new->rbr_ni = rbr_ni;
> +		/* avoid deleting node while it is been used for routing */

    Either "has been" or "is being".

> +		rcu_assign_pointer(rbr->rbr_nodes[rbr_ni->nick], new);
> +		if (old)
> +			rbr_node_put(old);
> +	} else {
> +		kfree(rbr_ni);
> +	}
> +
> +	return 0;
> +}
> +
>   static int trill_cmd_set_nicks_info(struct sk_buff *skb, struct genl_info *info)
>   {
> +	struct trill_nl_header *trnlhdr;
> +	struct rbr_nickinfo rbr_ni;
> +	struct net_device *source_port = NULL;
> +	struct net *net = sock_net(skb->sk);
> +	struct net_bridge_port *p = NULL;
> +	int err = -EINVAL;
> +
> +	nla_memcpy(&rbr_ni, info->attrs[TRILL_ATTR_BIN], sizeof(rbr_ni));
> +	if (!VALID_NICK(rbr_ni.nick))
> +		goto fail;
> +
> +	trnlhdr = info->userhdr;
> +	if (trnlhdr->ifindex)
> +		source_port = __dev_get_by_index(net, trnlhdr->ifindex);
> +
> +	if (!source_port)
> +		goto fail;
> +
> +	p = br_port_get_rcu(source_port);
> +	if (!p || !(p->br) || !(p->br->rbr))

    Inner parens not needed.

> +		goto fail;
> +
> +	err = create_node(p, p->br->rbr, &rbr_ni, info);
> +	if (err)
> +		goto fail;
> +
>   	return 0;
> +
> + fail:
> +	printk(KERN_WARNING "trill_cmd_set_nicks_info FAILED\n");

    pr_warn().

[...]

WBR, Sergei