On Mon, Dec 23, 2013 at 11:49:40AM -0800, Alex Wang wrote:
> Both patches look good to me,
>
> One comment below,
>
>
> > @@ -4432,57 +4432,38 @@ netdev_stats_from_rtnl_link_stats(struct
> > netdev_stats *dst,
> > static int
> > get_stats_via_netlink(const struct netdev *netdev_, struct netdev_stats
> > *stats)
> > {
> > - /* Policy for RTNLGRP_LINK messages.
> > - *
> > - * There are *many* more fields in these messages, but currently we
> > only
> > - * care about these fields. */
> > - static const struct nl_policy rtnlgrp_link_policy[] = {
> > - [IFLA_IFNAME] = { .type = NL_A_STRING, .optional = false },
> > - [IFLA_STATS] = { .type = NL_A_UNSPEC, .optional = true,
> > - .min_len = sizeof(struct rtnl_link_stats) },
> > - };
> > -
> > struct ofpbuf request;
> > struct ofpbuf *reply;
> > - struct ifinfomsg *ifi;
> > - struct nlattr *attrs[ARRAY_SIZE(rtnlgrp_link_policy)];
> > - int ifindex;
> > int error;
> >
> > - error = get_ifindex(netdev_, &ifindex);
> > - if (error) {
> > - return error;
> > - }
> > -
> > ofpbuf_init(&request, 0);
> > - nl_msg_put_nlmsghdr(&request, sizeof *ifi, RTM_GETLINK,
> > NLM_F_REQUEST);
> > - ifi = ofpbuf_put_zeros(&request, sizeof *ifi);
> > - ifi->ifi_family = PF_UNSPEC;
> > - ifi->ifi_index = ifindex;
> > + nl_msg_put_nlmsghdr(&request,
> > + sizeof(struct ifinfomsg) + NL_ATTR_SIZE(IFNAMSIZ),
> > + RTM_GETLINK, NLM_F_REQUEST);
> > + ofpbuf_put_zeros(&request, sizeof(struct ifinfomsg));
> > + nl_msg_put_string(&request, IFLA_IFNAME, netdev_get_name(netdev_));
> >
>
>
> Why do we do "sizeof(struct ifinfomsg) + NL_ATTR_SIZE(IFNAMSIZ)"
> instead of "sizeof(struct ifinfomsg)" or "sizeof(struct ifinfomsg) +
> sizeof(struct nlattr) + NL_ATTR_SIZE(IFNAMSIZ) " ?
NL_ATTR_SIZE includes the size of a struct nlattr:
#define NL_ATTR_SIZE(PAYLOAD_SIZE) (NLA_HDRLEN + NLA_ALIGN(PAYLOAD_SIZE))
> The "nl_msg_put_string(&request, IFLA_IFNAME, netdev_get_name(netdev_));"
> will realloc the "reqest", since we also need to put "struct nlattr".
I think that space is included.
Thanks for the review! I'm going to push this in a minute. (If I'm
wrong about the size calculation, we can fix that later.)
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
