Re: [PATCH net-next] mctp: emit RTM_NEWADDR and RTM_DELADDR

Hi Matt,

Userspace can receive notification of MCTP address changes via
RTNLGRP_MCTP_IFADDR rtnetlink multicast group.

Nice! A couple of minor things:

+static void mctp_addr_notify(struct mctp_dev *mdev, mctp_eid_t eid, int msg_type,
+                            struct sk_buff *req_skb, struct nlmsghdr *req_nlh)

Is it worthwhile keeping the portid argument here, rather than the
entire request skb? Might prevent some confusion with the notify skb.

(I'm OK if we settle on the req_skb approach too though, as it does save
a little repeated code in call sites)

Super minor, but: reverse Christmas tree here (and above)?

+
+       skb = nlmsg_new(mctp_addrinfo_size(), GFP_KERNEL);
+       if (!skb) {
+               rc = -ENOBUFS;
+               goto out;
+       }
+
+       rc = mctp_fill_addrinfo(skb, mdev, eid, msg_type,
+                               portid, req_nlh->nlmsg_seq, 0);
+       if (rc < 0)
+               goto out;
+
+       rtnl_notify(skb, net, portid, RTNLGRP_MCTP_IFADDR, req_nlh,
GFP_KERNEL);
+out:
+       if (rc < 0) {
+               if (skb)
+                       kfree_skb(skb);
+               rtnl_set_sk_err(net, RTNLGRP_MCTP_IFADDR, rc);
+       }

We don't need that `if (skb)` condition on the kfree_skb - Yang has
removed a bunch in 5cfe53cfeb1. We could also remove the rc check if we
use the goto-label only for errors. Maybe something like:

            rtnl_notify(skb, net, portid, RTNLGRP_MCTP_IFADDR, req_nlh, GFP_KERNEL);
            return;
    err:
            kfree_skb(skb);
            rtnl_set_sk_err(net, RTNLGRP_MCTP_IFADDR, rc);
    }

Cheers,


Jeremy