[PATCH] vlan: add support for IFLA_VLAN_PROTOCOL

Susant Sahani susant at redhat.com
Wed May 7 03:44:14 PDT 2014


On 05/07/2014 03:57 PM, Thomas Graf wrote:
> On 05/07/14 at 03:53pm, Susant Sahani wrote:
>> On 05/07/2014 02:29 PM, Thomas Graf wrote:
>>> On 05/07/14 at 12:45pm, Susant Sahani wrote:
>>>> This patch adds support for IFLA_VLAN_PROTOCOL
>>>>
>>>> Signed-off-by: Susant Sahani <susant at redhat.com>
>>> LGTM in general, the formatting seems off as it uses spaces
>>> instead of tabs, can you fix that?
>> Done . Bit struggling with it though :)
>>>> @@ -188,6 +196,9 @@ static void vlan_dump_details(struct rtnl_link *link, struct nl_dump_params *p)
>>>>   	rtnl_link_vlan_flags2str(vi->vi_flags, buf, sizeof(buf));
>>>>   	nl_dump_line(p, "    vlan-info id %d <%s>\n", vi->vi_vlan_id, buf);
>>>> +	if (vi->vi_mask & VLAN_HAS_PROTOCOL)
>>>> +		nl_dump_line(p, "  vlan protocol %d:\n", vi->vi_protocol);
>>>> +
>>> The `:` in there seems unneeded.
>> Removed.
>>> Perhaps merge the protocol on the
>>> same line as the VLAN ID and print the '\n' after the protocol branch?
>> The protocol value could be there or not . Is it OK to print a '0' ?
> You can do:
>
> rtnl_link_vlan_flags2str(vi->vi_flags, buf, sizeof(buf));
> nl_dump_line(p, "    vlan-info id %d <%s>", vi->vi_vlan_id, buf);
>
> if (vi->vi_mask & VLAN_HAS_PROTOCOL)
> 	nl_dump(p, "  protocol %d", vi->vi_protocol);
>
> nl_dump(p, "\n");
>
>
Cook Thanks !


Susant



More information about the libnl mailing list