Re: [Linuxptp-devel] [PATCHv3 04/10] rtnl: update function rtnl_link_status to get bond slave info

[Hangbin L. <liu...@gm...>]

Hi Richard,

Thanks for the comments. Any suggestions for the other patches.

Regards
Hangbin
On Sun, Aug 27, 2017 at 08:05:30AM -0700, Richard Cochran wrote:
> On Wed, Aug 16, 2017 at 09:32:05PM +0800, Hangbin Liu wrote:
> > @@ -31,6 +31,9 @@
> >  
> >  static int rtnl_len;
> >  static char *rtnl_buf;
> > +static int rtnl_rtattr_parse(struct rtattr *tb[], int max, struct rtattr *rta, int len);
> > +#define rtnl_nested_rtattr_parse(tb, max, rta) \
> > +	(rtnl_rtattr_parse((tb), (max), RTA_DATA(rta), RTA_PAYLOAD(rta)))
> 
> Instead of a #define, why not a static inline function?
> 
> > @@ -84,15 +87,69 @@ int rtnl_link_query(int fd, char *device)
> >  	return 0;
> >  }
> >  
> > -int rtnl_link_status(int fd, rtnl_callback cb, void *ctx)
> > +static inline __u32 rta_getattr_u32(const struct rtattr *rta)
> >  {
> > -	int index, len;
> > +	return *(__u32 *)RTA_DATA(rta);
> > +}
> > +
> > +static inline const char *rta_getattr_str(const struct rtattr *rta)
> > +{
> > +	return (const char *)RTA_DATA(rta);
> > +}
> > +
> > +static int rtnl_rtattr_parse(struct rtattr *tb[], int max, struct rtattr *rta, int len)
> > +{
> > +	unsigned short type;
> > +
> > +	memset(tb, 0, sizeof(struct rtattr *) * max);
> > +	while (RTA_OK(rta, len)) {
> > +		type = rta->rta_type;
> > +		if ((type <= max) && (!tb[type]))
> 
> Test should be  (type < max)  because 'tb[]' has length 'max' and
> tb[max] is out of bounds.
> 
> > +			tb[type] = rta;
> > +		rta = RTA_NEXT(rta, len);
> > +	}
> > +	if (len)
> > +		pr_err("Length mismatch: len %d, rta_len=%d\n", len, rta->rta_len);
> > +	return 0;
> > +}
> > +
> > +static int rtnl_linkinfo_parse(struct rtattr *rta)
> > +{
> > +	int index = -1;
> > +	const char *kind;
> > +	struct rtattr *linkinfo[IFLA_INFO_MAX];
> > +	struct rtattr *bond[IFLA_BOND_MAX];
> > +
> > +	rtnl_nested_rtattr_parse(linkinfo, IFLA_INFO_MAX, rta);
> > +
> > +	if (linkinfo[IFLA_INFO_KIND]) {
> > +		kind = rta_getattr_str(linkinfo[IFLA_INFO_KIND]);
> > +
> > +		if (kind && !strncmp(kind, "bond", 4) &&
> > +		    linkinfo[IFLA_INFO_DATA]) {
> > +			rtnl_nested_rtattr_parse(bond, IFLA_BOND_MAX,
> > +						 linkinfo[IFLA_INFO_DATA]);
> > +
> > +			if (bond[IFLA_BOND_ACTIVE_SLAVE]) {
> > +				index = rta_getattr_u32(bond[IFLA_BOND_ACTIVE_SLAVE]);
> > +			}
> > +		}
> > +	}
> > +	return index;
> > +}
> 
> Thanks,
> Richard