Re: [PATCH 1/2] Add vlan information to net_ls_addr output

[Daniel Kiper]

Hey,

First of all, if you send multiple patches please add cover letter to
the patch series. Her is a good example [1].

On Fri, Mar 04, 2022 at 10:46:35PM -0500, Chad Kimes via Grub-devel wrote:
> Example output:
>   grub> net_ls_addr
>   efinet1 00:11:22:33:44:55 192.168.0.100 vlan100

If you give examples please use addresses from example IP network,
192.0.2.0/24 [2].

> Signed-off-by: Chad Kimes <chkimes@github.com>
> ---
>  grub-core/net/net.c | 18 +++++++++++++++++-
>  include/grub/net.h  |  8 ++++++++
>  2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
> index 4d3eb5c1a..33e35d5b5 100644
> --- a/grub-core/net/net.c
> +++ b/grub-core/net/net.c
> @@ -781,6 +781,20 @@ grub_net_hwaddr_to_str (const 
> grub_net_link_level_address_t *addr, char *str)
>    grub_printf (_("Unsupported hw address type %d\n"), addr->type);
>  }
>
> +void
> +grub_net_vlan_to_str (grub_uint16_t vlantag, char *str)
> +{
> +  str[0] = 0;
> +
> +  /* 12 bits are used to identify the vlan in 802.1Q */
> +  vlantag = vlantag & 0xFFF;
> +
> +  if (vlantag == 0)
> +    return;
> +
> +  grub_snprintf (str, GRUB_NET_MAX_STR_VLAN_LEN, "vlan%u", vlantag);
> +}
> +
>  int
>  grub_net_hwaddr_cmp (const grub_net_link_level_address_t *a,
>                    const grub_net_link_level_address_t *b)
> @@ -1250,9 +1264,11 @@ grub_cmd_listaddrs (struct grub_command *cmd 
> __attribute__ ((unused)),
>    {
>      char bufh[GRUB_NET_MAX_STR_HWADDR_LEN];
>      char bufn[GRUB_NET_MAX_STR_ADDR_LEN];
> +    char bufv[GRUB_NET_MAX_STR_VLAN_LEN];

Please add empty line here.

>      grub_net_hwaddr_to_str (&inf->hwaddress, bufh);
>      grub_net_addr_to_str (&inf->address, bufn);
> -    grub_printf ("%s %s %s\n", inf->name, bufh, bufn);
> +    grub_net_vlan_to_str (inf->vlantag, bufv);
> +    grub_printf ("%s %s %s %s\n", inf->name, bufh, bufn, bufv);
>    }
>    return GRUB_ERR_NONE;
>  }
> diff --git a/include/grub/net.h b/include/grub/net.h
> index 7ae4b6bd8..7fccad8ec 100644
> --- a/include/grub/net.h
> +++ b/include/grub/net.h
> @@ -512,12 +512,20 @@ grub_net_addr_cmp (const 
> grub_net_network_level_address_t *a,
>
>  #define GRUB_NET_MAX_STR_HWADDR_LEN (sizeof ("XX:XX:XX:XX:XX:XX"))
>
> +/*
> +  Max VLAN id = 4094
> + */

Please fix this comment [3].

> +#define GRUB_NET_MAX_STR_VLAN_LEN (sizeof ("vlanXXXX"))

Daniel

[1] https://lists.gnu.org/archive/html/grub-devel/2022-03/msg00081.html
[2] https://datatracker.ietf.org/doc/html/rfc5737
[3] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Comments