All of lore.kernel.org
 help / color / mirror / Atom feed
* Request a freeze exception for vlantag feature
@ 2013-12-23 16:34 Paulo Flabiano Smorigo/Brazil/IBM
  2013-12-23 17:02 ` Andrey Borzenkov
  0 siblings, 1 reply; 8+ messages in thread
From: Paulo Flabiano Smorigo/Brazil/IBM @ 2013-12-23 16:34 UTC (permalink / raw)
  To: grub-devel

Hello everyone,

With the consent of Vladimir Serbinenko, I committed three patches in  
the next branch. Those patches add support for virtual LAN (VLAN)  
tagging. VLAN tagging allows multiple VLANs in a bridged network to  
share the same physical network link but maintain isolation:

http://en.wikipedia.org/wiki/IEEE_802.1Q

http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=11454e94ea8e78317b9c9ab22855e26890880745
http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=8ff3c31f6d59ca758b3bc0020e5140dcc937edd6
http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=af768f8d4166aae41bbac8575fca7c65a319bfdb


I come here to ask for an freeze exception so it can be also in master  
and in 2.02 release.

Thanks in advance,
--
Paulo Flabiano Smorigo
Software Engineer
Linux Technology Center - IBM Systems & Technology Group



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Request a freeze exception for vlantag feature
  2013-12-23 16:34 Request a freeze exception for vlantag feature Paulo Flabiano Smorigo/Brazil/IBM
@ 2013-12-23 17:02 ` Andrey Borzenkov
  2013-12-26 17:25   ` pfsmorigo
  0 siblings, 1 reply; 8+ messages in thread
From: Andrey Borzenkov @ 2013-12-23 17:02 UTC (permalink / raw)
  To: The development of GNU GRUB

В Пн, 23/12/2013 в 11:34 -0500, Paulo Flabiano Smorigo/Brazil/IBM пишет:
> Hello everyone,
> 
> With the consent of Vladimir Serbinenko, I committed three patches in  
> the next branch. Those patches add support for virtual LAN (VLAN)  
> tagging. VLAN tagging allows multiple VLANs in a bridged network to  
> share the same physical network link but maintain isolation:
> 
> http://en.wikipedia.org/wiki/IEEE_802.1Q
> 
> http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=11454e94ea8e78317b9c9ab22855e26890880745
> http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=8ff3c31f6d59ca758b3bc0020e5140dcc937edd6
> http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=af768f8d4166aae41bbac8575fca7c65a319bfdb
> 
> 

VLAN tag is 12 bits, with 4 remaining bits being packet priority. You
need to mask them off before comparing. 

A couple of words in documentation would be nice (are those variables
for this platform documented at all?)

> I come here to ask for an freeze exception so it can be also in master  
> and in 2.02 release.
>
> Thanks in advance,
> --
> Paulo Flabiano Smorigo
> Software Engineer
> Linux Technology Center - IBM Systems & Technology Group
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Request a freeze exception for vlantag feature
  2013-12-23 17:02 ` Andrey Borzenkov
@ 2013-12-26 17:25   ` pfsmorigo
  2014-01-08 18:57     ` Paulo Flabiano Smorigo
  0 siblings, 1 reply; 8+ messages in thread
From: pfsmorigo @ 2013-12-26 17:25 UTC (permalink / raw)
  To: The development of GNU GRUB

Mon, Dec 23, 2013 at 09:02:57PM +0400, Andrey Borzenkov wrote:
> В Пн, 23/12/2013 в 11:34 -0500, Paulo Flabiano Smorigo/Brazil/IBM пишет:
> > Hello everyone,
> > 
> > With the consent of Vladimir Serbinenko, I committed three patches in  
> > the next branch. Those patches add support for virtual LAN (VLAN)  
> > tagging. VLAN tagging allows multiple VLANs in a bridged network to  
> > share the same physical network link but maintain isolation:
> > 
> > http://en.wikipedia.org/wiki/IEEE_802.1Q
> > 
> > http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=11454e94ea8e78317b9c9ab22855e26890880745
> > http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=8ff3c31f6d59ca758b3bc0020e5140dcc937edd6
> > http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=af768f8d4166aae41bbac8575fca7c65a319bfdb
> > 
> > 
> 
> VLAN tag is 12 bits, with 4 remaining bits being packet priority. You
> need to mask them off before comparing. 
> 

Tks for the feedback. I fixed it. Can you take a look?

http://git.io/ol5y-Q

> A couple of words in documentation would be nice (are those variables
> for this platform documented at all?)

Ok, I'll will add a documentation.


Btw, I need to add it to the commands. I'm not sure if I add a parameter
to net_add_addr and net_bootp or create a new one to set it.

> 
> > I come here to ask for an freeze exception so it can be also in master  
> > and in 2.02 release.
> >
> > Thanks in advance,
> > --
> > Paulo Flabiano Smorigo
> > Software Engineer
> > Linux Technology Center - IBM Systems & Technology Group
> > 
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

-- 
--
Paulo Flabiano Smorigo
IBM Linux Technology Center



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Request a freeze exception for vlantag feature
  2013-12-26 17:25   ` pfsmorigo
@ 2014-01-08 18:57     ` Paulo Flabiano Smorigo
  2014-01-09  3:05       ` Andrey Borzenkov
  0 siblings, 1 reply; 8+ messages in thread
From: Paulo Flabiano Smorigo @ 2014-01-08 18:57 UTC (permalink / raw)
  To: The development of GNU GRUB

Thu, Dec 26, 2013 at 03:25:28PM -0200, pfsmorigo@linux.vnet.ibm.com wrote:
> Mon, Dec 23, 2013 at 09:02:57PM +0400, Andrey Borzenkov wrote:
> > В Пн, 23/12/2013 в 11:34 -0500, Paulo Flabiano Smorigo/Brazil/IBM пишет:
> > > Hello everyone,
> > > 
> > > With the consent of Vladimir Serbinenko, I committed three patches in  
> > > the next branch. Those patches add support for virtual LAN (VLAN)  
> > > tagging. VLAN tagging allows multiple VLANs in a bridged network to  
> > > share the same physical network link but maintain isolation:
> > > 
> > > http://en.wikipedia.org/wiki/IEEE_802.1Q
> > > 
> > > http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=11454e94ea8e78317b9c9ab22855e26890880745
> > > http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=8ff3c31f6d59ca758b3bc0020e5140dcc937edd6
> > > http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=af768f8d4166aae41bbac8575fca7c65a319bfdb
> > > 
> > > 
> > 
> > VLAN tag is 12 bits, with 4 remaining bits being packet priority. You
> > need to mask them off before comparing. 
> > 
> 
> Tks for the feedback. I fixed it. Can you take a look?
> 
> http://git.io/ol5y-Q
> 
> > A couple of words in documentation would be nice (are those variables
> > for this platform documented at all?)
> 
> Ok, I'll will add a documentation.
> 
> 
> Btw, I need to add it to the commands. I'm not sure if I add a parameter
> to net_add_addr and net_bootp or create a new one to set it.
> 
> > 
> > > I come here to ask for an freeze exception so it can be also in master  
> > > and in 2.02 release.
> > >
> > > Thanks in advance,
> > > --
> > > Paulo Flabiano Smorigo
> > > Software Engineer
> > > Linux Technology Center - IBM Systems & Technology Group
> > > 
> > > 
> > > _______________________________________________
> > > Grub-devel mailing list
> > > Grub-devel@gnu.org
> > > https://lists.gnu.org/mailman/listinfo/grub-devel
> > 
> > 
> > 
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> -- 
> --
> Paulo Flabiano Smorigo
> IBM Linux Technology Center
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

This is a reviewed version of the patch. I added vlantag option to the net_add_addr
and net_bootp commands.

---
 grub-core/net/arp.c                    |  8 +++---
 grub-core/net/bootp.c                  | 24 ++++++++++++++---
 grub-core/net/drivers/ieee1275/ofnet.c |  6 ++++-
 grub-core/net/ethernet.c               | 20 +++++++-------
 grub-core/net/ip.c                     | 22 +++++++--------
 grub-core/net/net.c                    | 49 +++++++++++++++++++++++++++++++---
 include/grub/net.h                     | 11 +++++++-
 include/grub/net/arp.h                 |  2 +-
 include/grub/net/ip.h                  |  2 +-
 9 files changed, 110 insertions(+), 34 deletions(-)

diff --git a/grub-core/net/arp.c b/grub-core/net/arp.c
index 2e3cf44..8485ff2 100644
--- a/grub-core/net/arp.c
+++ b/grub-core/net/arp.c
@@ -122,7 +122,7 @@ grub_net_arp_send_request (struct grub_net_network_level_interface *inf,
 
 grub_err_t
 grub_net_arp_receive (struct grub_net_buff *nb, struct grub_net_card *card,
-                      grub_uint16_t *vlantag)
+                      grub_uint16_t vlantag_vid)
 {
   struct arphdr *arp_header = (struct arphdr *) nb->data;
   grub_uint8_t *sender_hardware_address;
@@ -158,11 +158,11 @@ grub_net_arp_receive (struct grub_net_buff *nb, struct grub_net_card *card,
   FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
   {
     /* Verify vlantag id */
-    if (inf->card == card && inf->vlantag != *vlantag)
+    if (inf->card == card && inf->vlantag.vid != vlantag_vid)
       {
         grub_dprintf ("net", "invalid vlantag! %x != %x\n",
-                      inf->vlantag, *vlantag);
-        break;
+                      inf->vlantag.vid, vlantag_vid);
+        continue;
       }
 
     /* Am I the protocol address target? */
diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index 6310ed4..4c4217a 100644
--- a/grub-core/net/bootp.c
+++ b/grub-core/net/bootp.c
@@ -425,10 +425,12 @@ grub_cmd_bootp (struct grub_command *cmd __attribute__ ((unused)),
   unsigned j = 0;
   int interval;
   grub_err_t err;
+  grub_int8_t vlan_pos = -1;
 
   FOR_NET_CARDS (card)
   {
-    if (argc > 0 && grub_strcmp (card->name, args[0]) != 0)
+    if (argc > 0 && grub_strcmp (card->name, args[0]) != 0 &&
+        grub_strcmp (args[0], "vlan") != 0)
       continue;
     ncards++;
   }
@@ -443,7 +445,8 @@ grub_cmd_bootp (struct grub_command *cmd __attribute__ ((unused)),
   j = 0;
   FOR_NET_CARDS (card)
   {
-    if (argc > 0 && grub_strcmp (card->name, args[0]) != 0)
+    if (argc > 0 && grub_strcmp (card->name, args[0]) != 0 &&
+        grub_strcmp (args[0], "vlan") != 0)
       continue;
     ifaces[j].card = card;
     ifaces[j].next = &ifaces[j+1];
@@ -462,6 +465,21 @@ grub_cmd_bootp (struct grub_command *cmd __attribute__ ((unused)),
     ifaces[j].address.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_DHCP_RECV;
     grub_memcpy (&ifaces[j].hwaddress, &card->default_address, 
 		 sizeof (ifaces[j].hwaddress));
+
+    if (grub_strcmp (args[0], "vlan") == 0)
+      vlan_pos = 0;
+
+    if (grub_strcmp (args[1], "vlan") == 0)
+      vlan_pos = 1;
+
+    if (vlan_pos != -1)
+      {
+        if (argc == vlan_pos + 4)
+          grub_set_vlantag (&ifaces[j].vlantag, &args[vlan_pos]);
+        else
+          return grub_error (GRUB_ERR_BAD_ARGUMENT,
+                             N_("five arguments expected"));
+      }
     j++;
   }
   ifaces[ncards - 1].next = grub_net_network_level_interfaces;
@@ -570,7 +588,7 @@ void
 grub_bootp_init (void)
 {
   cmd_bootp = grub_register_command ("net_bootp", grub_cmd_bootp,
-				     N_("[CARD]"),
+				     N_("[CARD] [vlan PCP DEI VID]"),
 				     N_("perform a bootp autoconfiguration"));
   cmd_getdhcp = grub_register_command ("net_get_dhcp_option", grub_cmd_dhcpopt,
 				       N_("VAR INTERFACE NUMBER DESCRIPTION"),
diff --git a/grub-core/net/drivers/ieee1275/ofnet.c b/grub-core/net/drivers/ieee1275/ofnet.c
index 956d12d..ffcb943 100644
--- a/grub-core/net/drivers/ieee1275/ofnet.c
+++ b/grub-core/net/drivers/ieee1275/ofnet.c
@@ -212,7 +212,11 @@ grub_ieee1275_parse_bootargs (const char *devpath, char *bootpath,
                                   hw_addr.mac, sizeof(hw_addr.mac), 0);
       inter = grub_net_add_addr ((*card)->name, *card, &client_addr, &hw_addr,
                                  flags);
-      inter->vlantag = vlantag;
+
+      inter->vlantag.pcp = vlantag >> 12;
+      inter->vlantag.dei = (vlantag >> 11) & 0x1;
+      inter->vlantag.vid = vlantag & 0x1fff;
+
       grub_net_add_ipv4_local (inter,
                           __builtin_ctz (~grub_le_to_cpu32 (subnet_mask.ipv4)));
 
diff --git a/grub-core/net/ethernet.c b/grub-core/net/ethernet.c
index 4d7ceed..ae195bc 100644
--- a/grub-core/net/ethernet.c
+++ b/grub-core/net/ethernet.c
@@ -58,13 +58,12 @@ send_ethernet_packet (struct grub_net_network_level_interface *inf,
   struct etherhdr *eth;
   grub_err_t err;
   grub_uint8_t etherhdr_size;
-  grub_uint16_t vlantag_id = VLANTAG_IDENTIFIER;
 
   etherhdr_size = sizeof (*eth);
   COMPILE_TIME_ASSERT (sizeof (*eth) + 4 < GRUB_NET_MAX_LINK_HEADER_SIZE);
 
   /* Increase ethernet header in case of vlantag */
-  if (inf->vlantag != 0)
+  if (inf->vlantag.vid != 0)
     etherhdr_size += 4;
 
   err = grub_netbuff_push (nb, etherhdr_size);
@@ -86,15 +85,18 @@ send_ethernet_packet (struct grub_net_network_level_interface *inf,
     }
 
   /* Check and add a vlan-tag if needed. */
-  if (inf->vlantag != 0)
+  if (inf->vlantag.vid != 0)
     {
+      grub_uint32_t vlantag;
+      vlantag = (VLANTAG_IDENTIFIER << 16) + (inf->vlantag.pcp << 12) +
+                (inf->vlantag.dei << 11) + inf->vlantag.vid;
+
       /* Move eth type to the right */
       grub_memcpy ((char *) nb->data + etherhdr_size - 2,
                    (char *) nb->data + etherhdr_size - 6, 2);
 
       /* Add the tag in the middle */
-      grub_memcpy ((char *) nb->data + etherhdr_size - 6, &vlantag_id, 2);
-      grub_memcpy ((char *) nb->data + etherhdr_size - 4, (char *) &(inf->vlantag), 2);
+      grub_memcpy ((char *) nb->data + etherhdr_size - 6, &vlantag, 4);
     }
 
   return inf->card->driver->send (inf->card, nb);
@@ -112,7 +114,7 @@ grub_net_recv_ethernet_packet (struct grub_net_buff *nb,
   grub_net_link_level_address_t src_hwaddress;
   grub_err_t err;
   grub_uint8_t etherhdr_size = sizeof (*eth);
-  grub_uint16_t vlantag = 0;
+  grub_uint16_t vlantag_vid = 0;
 
 
   /* Check if a vlan-tag is present. If so, the ethernet header is 4 bytes */
@@ -120,7 +122,7 @@ grub_net_recv_ethernet_packet (struct grub_net_buff *nb,
   /* is reseted to the original size. */
   if (grub_get_unaligned16 (nb->data + etherhdr_size - 2) == VLANTAG_IDENTIFIER)
     {
-      vlantag = grub_get_unaligned16 (nb->data + etherhdr_size);
+      vlantag_vid = grub_get_unaligned16 (nb->data + etherhdr_size) & 0x1fff;
       etherhdr_size += 4;
       /* Move eth type to the original position */
       grub_memcpy((char *) nb->data + etherhdr_size - 6,
@@ -157,14 +159,14 @@ grub_net_recv_ethernet_packet (struct grub_net_buff *nb,
     {
       /* ARP packet. */
     case GRUB_NET_ETHERTYPE_ARP:
-      grub_net_arp_receive (nb, card, &vlantag);
+      grub_net_arp_receive (nb, card, vlantag_vid);
       grub_netbuff_free (nb);
       return GRUB_ERR_NONE;
       /* IP packet.  */
     case GRUB_NET_ETHERTYPE_IP:
     case GRUB_NET_ETHERTYPE_IP6:
       return grub_net_recv_ip_packets (nb, card, &hwaddress, &src_hwaddress,
-                                       &vlantag);
+                                       vlantag_vid);
     }
   grub_netbuff_free (nb);
   return GRUB_ERR_NONE;
diff --git a/grub-core/net/ip.c b/grub-core/net/ip.c
index b21bc6f..9919caa 100644
--- a/grub-core/net/ip.c
+++ b/grub-core/net/ip.c
@@ -225,7 +225,7 @@ handle_dgram (struct grub_net_buff *nb,
 	      grub_net_ip_protocol_t proto,
 	      const grub_net_network_level_address_t *source,
 	      const grub_net_network_level_address_t *dest,
-              grub_uint16_t *vlantag,
+              grub_uint16_t vlantag_vid,
 	      grub_uint8_t ttl)
 {
   struct grub_net_network_level_interface *inf = NULL;
@@ -293,10 +293,10 @@ handle_dgram (struct grub_net_buff *nb,
       break;
 
     /* Verify vlantag id */
-    if (inf->card == card && inf->vlantag != *vlantag)
+    if (inf->card == card && inf->vlantag.vid != vlantag_vid)
       {
         grub_dprintf ("net", "invalid vlantag! %x != %x\n",
-                      inf->vlantag, *vlantag);
+                      inf->vlantag.vid, vlantag_vid);
         break;
       }
 
@@ -389,7 +389,7 @@ grub_net_recv_ip4_packets (struct grub_net_buff *nb,
 			   struct grub_net_card *card,
 			   const grub_net_link_level_address_t *hwaddress,
 			   const grub_net_link_level_address_t *src_hwaddress,
-                           grub_uint16_t *vlantag)
+                           grub_uint16_t vlantag_vid)
 {
   struct iphdr *iph = (struct iphdr *) nb->data;
   grub_err_t err;
@@ -464,7 +464,7 @@ grub_net_recv_ip4_packets (struct grub_net_buff *nb,
       dest.ipv4 = iph->dest;
 
       return handle_dgram (nb, card, src_hwaddress, hwaddress, iph->protocol,
-			   &source, &dest, vlantag, iph->ttl);
+			   &source, &dest, vlantag_vid, iph->ttl);
     }
 
   for (prev = &reassembles, rsm = *prev; rsm; prev = &rsm->next, rsm = *prev)
@@ -600,7 +600,7 @@ grub_net_recv_ip4_packets (struct grub_net_buff *nb,
       dest.ipv4 = dst;
 
       return handle_dgram (ret, card, src_hwaddress,
-			   hwaddress, proto, &source, &dest, vlantag,
+			   hwaddress, proto, &source, &dest, vlantag_vid,
 			   ttl);
     }
 }
@@ -656,7 +656,7 @@ grub_net_recv_ip6_packets (struct grub_net_buff *nb,
 			   struct grub_net_card *card,
 			   const grub_net_link_level_address_t *hwaddress,
 			   const grub_net_link_level_address_t *src_hwaddress,
-                           grub_uint16_t *vlantag)
+                           grub_uint16_t vlantag_vid)
 {
   struct ip6hdr *iph = (struct ip6hdr *) nb->data;
   grub_err_t err;
@@ -707,7 +707,7 @@ grub_net_recv_ip6_packets (struct grub_net_buff *nb,
   grub_memcpy (dest.ipv6, &iph->dest, sizeof (dest.ipv6));
 
   return handle_dgram (nb, card, src_hwaddress, hwaddress, iph->protocol,
-		       &source, &dest, vlantag, iph->ttl);
+		       &source, &dest, vlantag_vid, iph->ttl);
 }
 
 grub_err_t
@@ -715,16 +715,16 @@ grub_net_recv_ip_packets (struct grub_net_buff *nb,
 			  struct grub_net_card *card,
 			  const grub_net_link_level_address_t *hwaddress,
 			  const grub_net_link_level_address_t *src_hwaddress,
-                          grub_uint16_t *vlantag)
+                          grub_uint16_t vlantag_vid)
 {
   struct iphdr *iph = (struct iphdr *) nb->data;
 
   if ((iph->verhdrlen >> 4) == 4)
     return grub_net_recv_ip4_packets (nb, card, hwaddress, src_hwaddress,
-                                      vlantag);
+                                      vlantag_vid);
   if ((iph->verhdrlen >> 4) == 6)
     return grub_net_recv_ip6_packets (nb, card, hwaddress, src_hwaddress,
-                                      vlantag);
+                                      vlantag_vid);
   grub_dprintf ("net", "Bad IP version: %d\n", (iph->verhdrlen >> 4));
   grub_netbuff_free (nb);
   return GRUB_ERR_NONE;
diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 8f9d183..6afe80f 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1014,6 +1014,26 @@ grub_net_add_ipv4_local (struct grub_net_network_level_interface *inter,
   return 0;
 }
 
+grub_err_t
+grub_set_vlantag (struct grub_net_vlantag *vlantag, char **args)
+{
+      grub_uint16_t value;
+
+      value = grub_strtoul (args[1], 0, 0);
+      if (value <= 7)
+        vlantag->pcp = value;
+
+      value = grub_strtoul (args[2], 0, 0);
+      if (value <= 1)
+        vlantag->dei = value;
+
+      value = grub_strtoul (args[3], 0, 0);
+      if (value <= 4095)
+        vlantag->vid = value;
+
+      return GRUB_ERR_NONE;
+}
+
 /* FIXME: support MAC specifying.  */
 static grub_err_t
 grub_cmd_addaddr (struct grub_command *cmd __attribute__ ((unused)),
@@ -1024,8 +1044,9 @@ grub_cmd_addaddr (struct grub_command *cmd __attribute__ ((unused)),
   grub_err_t err;
   grub_net_interface_flags_t flags = 0;
   struct grub_net_network_level_interface *inf;
+  grub_int8_t vlan_pos = -1;
 
-  if (argc != 3)
+  if (argc < 3)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("three arguments expected"));
   
   FOR_NET_CARDS (card)
@@ -1047,6 +1068,22 @@ grub_cmd_addaddr (struct grub_command *cmd __attribute__ ((unused)),
 
   inf = grub_net_add_addr (args[0], card, &addr, &card->default_address,
 			   flags);
+
+  if (grub_strcmp (args[3], "vlan") == 0)
+    vlan_pos = 3;
+
+  if (grub_strcmp (args[4], "vlan") == 0)
+    vlan_pos = 4;
+
+  if (vlan_pos != -1)
+    {
+      if (argc == vlan_pos + 4)
+        grub_set_vlantag (&inf->vlantag, &args[vlan_pos]);
+      else
+        return grub_error (GRUB_ERR_BAD_ARGUMENT,
+                           N_("seven arguments expected"));
+    }
+
   if (inf)
     grub_net_add_ipv4_local (inf, -1);
 
@@ -1257,7 +1294,13 @@ grub_cmd_listaddrs (struct grub_command *cmd __attribute__ ((unused)),
     char bufn[GRUB_NET_MAX_STR_ADDR_LEN];
     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_printf ("%s %s %s", inf->name, bufh, bufn);
+
+    if (inf->vlantag.vid > 0)
+        grub_printf (" vlan %u %u %u\n", inf->vlantag.pcp, inf->vlantag.dei,
+                     inf->vlantag.vid);
+    else
+        grub_printf ("\n");
   }
   return GRUB_ERR_NONE;
 }
@@ -1725,7 +1768,7 @@ GRUB_MOD_INIT(net)
   cmd_addaddr = grub_register_command ("net_add_addr", grub_cmd_addaddr,
 					/* TRANSLATORS: HWADDRESS stands for
 					   "hardware address".  */
-				       N_("SHORTNAME CARD ADDRESS [HWADDRESS]"),
+				       N_("SHORTNAME CARD ADDRESS [HWADDRESS] [vlan PCP DEI VID]"),
 				       N_("Add a network address."));
   cmd_slaac = grub_register_command ("net_ipv6_autoconf",
 				     grub_cmd_ipv6_autoconf,
diff --git a/include/grub/net.h b/include/grub/net.h
index cce3eda..7c0847b 100644
--- a/include/grub/net.h
+++ b/include/grub/net.h
@@ -268,6 +268,13 @@ typedef struct grub_net
 
 extern grub_net_t (*EXPORT_VAR (grub_net_open)) (const char *name);
 
+struct grub_net_vlantag
+{
+    grub_uint8_t pcp;
+    grub_uint8_t dei;
+    grub_uint16_t vid;
+};
+
 struct grub_net_network_level_interface
 {
   struct grub_net_network_level_interface *next;
@@ -279,7 +286,7 @@ struct grub_net_network_level_interface
   grub_net_interface_flags_t flags;
   struct grub_net_bootp_packet *dhcp_ack;
   grub_size_t dhcp_acklen;
-  grub_uint16_t vlantag;
+  struct grub_net_vlantag vlantag;
   void *data;
 };
 
@@ -395,6 +402,8 @@ grub_net_add_route_gw (const char *name,
 		       grub_net_network_level_netaddress_t target,
 		       grub_net_network_level_address_t gw);
 
+grub_err_t
+grub_set_vlantag (struct grub_net_vlantag *vlantag, char **args);
 
 #define GRUB_NET_BOOTP_MAC_ADDR_LEN	16
 
diff --git a/include/grub/net/arp.h b/include/grub/net/arp.h
index 8d9d081..56336b3 100644
--- a/include/grub/net/arp.h
+++ b/include/grub/net/arp.h
@@ -23,7 +23,7 @@
 
 extern grub_err_t grub_net_arp_receive (struct grub_net_buff *nb,
                                         struct grub_net_card *card,
-                                        grub_uint16_t *vlantag);
+                                        grub_uint16_t vlantag_vid);
 
 grub_err_t
 grub_net_arp_send_request (struct grub_net_network_level_interface *inf,
diff --git a/include/grub/net/ip.h b/include/grub/net/ip.h
index 39062df..e79dec9 100644
--- a/include/grub/net/ip.h
+++ b/include/grub/net/ip.h
@@ -49,7 +49,7 @@ grub_net_recv_ip_packets (struct grub_net_buff *nb,
 			  struct grub_net_card *card,
 			  const grub_net_link_level_address_t *hwaddress,
 			  const grub_net_link_level_address_t *src_hwaddress,
-                          grub_uint16_t *vlantag);
+                          grub_uint16_t vlantag_vid);
 
 grub_err_t
 grub_net_send_ip_packet (struct grub_net_network_level_interface *inf,







^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: Request a freeze exception for vlantag feature
  2014-01-08 18:57     ` Paulo Flabiano Smorigo
@ 2014-01-09  3:05       ` Andrey Borzenkov
  2014-01-09 16:58         ` Paulo Flabiano Smorigo
  0 siblings, 1 reply; 8+ messages in thread
From: Andrey Borzenkov @ 2014-01-09  3:05 UTC (permalink / raw)
  To: grub-devel

В Wed, 8 Jan 2014 16:57:28 -0200
Paulo Flabiano Smorigo <pfsmorigo@linux.vnet.ibm.com> пишет:

> +
> +      inter->vlantag.pcp = vlantag >> 12;
> +      inter->vlantag.dei = (vlantag >> 11) & 0x1;
> +      inter->vlantag.vid = vlantag & 0x1fff;

That's 13 bits, not 12, right? And this really looks like
overengeneering - do you really want to be able to set static VLAN
priority bits? I do not think it belongs to grub.

> +
> +  if (grub_strcmp (args[3], "vlan") == 0)
> +    vlan_pos = 3;
> +
> +  if (grub_strcmp (args[4], "vlan") == 0)
> +    vlan_pos = 4;

May be it should really start using proper options at this point
keeping existing three argument form as legacy.

net_add_addr --if=... --addr=... --mask=... --vlan=... --hw=... card


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Request a freeze exception for vlantag feature
  2014-01-09  3:05       ` Andrey Borzenkov
@ 2014-01-09 16:58         ` Paulo Flabiano Smorigo
  2014-01-14 19:01           ` Vladimir 'φ-coder/phcoder' Serbinenko
  2014-01-15  5:18           ` Andrey Borzenkov
  0 siblings, 2 replies; 8+ messages in thread
From: Paulo Flabiano Smorigo @ 2014-01-09 16:58 UTC (permalink / raw)
  To: The development of GNU GRUB

Thu, Jan 09, 2014 at 07:05:16AM +0400, Andrey Borzenkov wrote:
> В Wed, 8 Jan 2014 16:57:28 -0200
> Paulo Flabiano Smorigo <pfsmorigo@linux.vnet.ibm.com> пишет:
> 
> > +
> > +      inter->vlantag.pcp = vlantag >> 12;
> > +      inter->vlantag.dei = (vlantag >> 11) & 0x1;
> > +      inter->vlantag.vid = vlantag & 0x1fff;
> 
> That's 13 bits, not 12, right? And this really looks like
> overengeneering - do you really want to be able to set static VLAN
> priority bits? I do not think it belongs to grub.
> 
> > +
> > +  if (grub_strcmp (args[3], "vlan") == 0)
> > +    vlan_pos = 3;
> > +
> > +  if (grub_strcmp (args[4], "vlan") == 0)
> > +    vlan_pos = 4;
> 

You're right, thanks. I fixed:

diff --git a/grub-core/net/drivers/ieee1275/ofnet.c b/grub-core/net/drivers/ieee1275/ofnet.c
index ffcb943..72359c3 100644
--- a/grub-core/net/drivers/ieee1275/ofnet.c
+++ b/grub-core/net/drivers/ieee1275/ofnet.c
@@ -213,13 +213,12 @@ grub_ieee1275_parse_bootargs (const char *devpath, char *bootpath,
       inter = grub_net_add_addr ((*card)->name, *card, &client_addr, &hw_addr,
                                  flags);
 
-      inter->vlantag.pcp = vlantag >> 12;
-      inter->vlantag.dei = (vlantag >> 11) & 0x1;
-      inter->vlantag.vid = vlantag & 0x1fff;
+      inter->vlantag.pcp = vlantag >> 13;
+      inter->vlantag.dei = (vlantag >> 12) & 0x1;
+      inter->vlantag.vid = vlantag & 0xfff;
 
       grub_net_add_ipv4_local (inter,
                           __builtin_ctz (~grub_le_to_cpu32 (subnet_mask.ipv4)));
-
     }
 
   if (gateway_addr.ipv4 != 0)
diff --git a/grub-core/net/ethernet.c b/grub-core/net/ethernet.c
index ae195bc..7ca14e9 100644
--- a/grub-core/net/ethernet.c
+++ b/grub-core/net/ethernet.c
@@ -88,8 +88,8 @@ send_ethernet_packet (struct grub_net_network_level_interface *inf,
   if (inf->vlantag.vid != 0)
     {
       grub_uint32_t vlantag;
-      vlantag = (VLANTAG_IDENTIFIER << 16) + (inf->vlantag.pcp << 12) +
-                (inf->vlantag.dei << 11) + inf->vlantag.vid;
+      vlantag = (VLANTAG_IDENTIFIER << 16) | (inf->vlantag.pcp << 13) |
+                (inf->vlantag.dei << 12) | inf->vlantag.vid;
 
       /* Move eth type to the right */
       grub_memcpy ((char *) nb->data + etherhdr_size - 2,


Virtual lan priority is an option in PowerPC SMS:

 PowerPC Firmware
 Version ZM770_024
 SMS 1.7 (c) Copyright IBM Corp. 2000,2008 All rights reserved.
-------------------------------------------------------------------------------
 Advanced Setup: BOOTP
Interpartition Logical LAN: U9109.RMD.10F037P-V4-C3-T1

 1.   Bootp Retries    5
 2.   Bootp Blocksize  512
 3.   TFTP  Retries    5
 4.   VLAN  Priority   0
 5.   VLAN  ID         0 (default - not configured)


Maybe we can use the priority and DEI of incoming package as the values for the
following packages. What do you think?

> May be it should really start using proper options at this point
> keeping existing three argument form as legacy.
> 
> net_add_addr --if=... --addr=... --mask=... --vlan=... --hw=... card

I prefer the current format but we can switch to another if it's more suitable.

Andrey, ping me in IRC so we can talk about it.

> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

I ask this patch as a freeze exception. This feature will only add a option for
vlan tag and will not change the default grub workflow. Can I push the current
version in master so it can be included in 2.02? Anyone against?

Thanks!

--
Paulo Flabiano Smorigo
IBM Linux Technology Center



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: Request a freeze exception for vlantag feature
  2014-01-09 16:58         ` Paulo Flabiano Smorigo
@ 2014-01-14 19:01           ` Vladimir 'φ-coder/phcoder' Serbinenko
  2014-01-15  5:18           ` Andrey Borzenkov
  1 sibling, 0 replies; 8+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2014-01-14 19:01 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 4449 bytes --]

On 09.01.2014 17:58, Paulo Flabiano Smorigo wrote:
> Thu, Jan 09, 2014 at 07:05:16AM +0400, Andrey Borzenkov wrote:
>> В Wed, 8 Jan 2014 16:57:28 -0200
>> Paulo Flabiano Smorigo <pfsmorigo@linux.vnet.ibm.com> пишет:
>>
>>> +
>>> +      inter->vlantag.pcp = vlantag >> 12;
>>> +      inter->vlantag.dei = (vlantag >> 11) & 0x1;
>>> +      inter->vlantag.vid = vlantag & 0x1fff;
>>
>> That's 13 bits, not 12, right? And this really looks like
>> overengeneering - do you really want to be able to set static VLAN
>> priority bits? I do not think it belongs to grub.
>>
>>> +
>>> +  if (grub_strcmp (args[3], "vlan") == 0)
>>> +    vlan_pos = 3;
>>> +
>>> +  if (grub_strcmp (args[4], "vlan") == 0)
>>> +    vlan_pos = 4;
>>
> 
> You're right, thanks. I fixed:
> 
> diff --git a/grub-core/net/drivers/ieee1275/ofnet.c b/grub-core/net/drivers/ieee1275/ofnet.c
> index ffcb943..72359c3 100644
> --- a/grub-core/net/drivers/ieee1275/ofnet.c
> +++ b/grub-core/net/drivers/ieee1275/ofnet.c
> @@ -213,13 +213,12 @@ grub_ieee1275_parse_bootargs (const char *devpath, char *bootpath,
>        inter = grub_net_add_addr ((*card)->name, *card, &client_addr, &hw_addr,
>                                   flags);
>  
> -      inter->vlantag.pcp = vlantag >> 12;
> -      inter->vlantag.dei = (vlantag >> 11) & 0x1;
> -      inter->vlantag.vid = vlantag & 0x1fff;
> +      inter->vlantag.pcp = vlantag >> 13;
> +      inter->vlantag.dei = (vlantag >> 12) & 0x1;
> +      inter->vlantag.vid = vlantag & 0xfff;
>  
>        grub_net_add_ipv4_local (inter,
>                            __builtin_ctz (~grub_le_to_cpu32 (subnet_mask.ipv4)));
> -
>      }
>  
>    if (gateway_addr.ipv4 != 0)
> diff --git a/grub-core/net/ethernet.c b/grub-core/net/ethernet.c
> index ae195bc..7ca14e9 100644
> --- a/grub-core/net/ethernet.c
> +++ b/grub-core/net/ethernet.c
> @@ -88,8 +88,8 @@ send_ethernet_packet (struct grub_net_network_level_interface *inf,
>    if (inf->vlantag.vid != 0)
>      {
>        grub_uint32_t vlantag;
> -      vlantag = (VLANTAG_IDENTIFIER << 16) + (inf->vlantag.pcp << 12) +
> -                (inf->vlantag.dei << 11) + inf->vlantag.vid;
> +      vlantag = (VLANTAG_IDENTIFIER << 16) | (inf->vlantag.pcp << 13) |
> +                (inf->vlantag.dei << 12) | inf->vlantag.vid;
>  
>        /* Move eth type to the right */
>        grub_memcpy ((char *) nb->data + etherhdr_size - 2,
> 
> 
> Virtual lan priority is an option in PowerPC SMS:
> 
>  PowerPC Firmware
>  Version ZM770_024
>  SMS 1.7 (c) Copyright IBM Corp. 2000,2008 All rights reserved.
> -------------------------------------------------------------------------------
>  Advanced Setup: BOOTP
> Interpartition Logical LAN: U9109.RMD.10F037P-V4-C3-T1
> 
>  1.   Bootp Retries    5
>  2.   Bootp Blocksize  512
>  3.   TFTP  Retries    5
>  4.   VLAN  Priority   0
>  5.   VLAN  ID         0 (default - not configured)
> 
> 
> Maybe we can use the priority and DEI of incoming package as the values for the
> following packages. What do you think?
> 
What's the advantage to setting it at all? We don't do any QOS.
>> May be it should really start using proper options at this point
>> keeping existing three argument form as legacy.
>>
>> net_add_addr --if=... --addr=... --mask=... --vlan=... --hw=... card
> 
> I prefer the current format but we can switch to another if it's more suitable.
> 
We can go eiher with options or syntax like [vlan <num>]. The only realy
requirement is extendability, we should be able to add new options
easily. The big advantage of option is code reuse from extcmd which
ensures that it's similar handling with other options and less risk of bugs.
> Andrey, ping me in IRC so we can talk about it.
> 
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> I ask this patch as a freeze exception. This feature will only add a option for
> vlan tag and will not change the default grub workflow. Can I push the current
> version in master so it can be included in 2.02? Anyone against?
> 
> Thanks!
> 
> --
> Paulo Flabiano Smorigo
> IBM Linux Technology Center
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 274 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Request a freeze exception for vlantag feature
  2014-01-09 16:58         ` Paulo Flabiano Smorigo
  2014-01-14 19:01           ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2014-01-15  5:18           ` Andrey Borzenkov
  1 sibling, 0 replies; 8+ messages in thread
From: Andrey Borzenkov @ 2014-01-15  5:18 UTC (permalink / raw)
  To: The development of GNU GRUB

On Thu, Jan 9, 2014 at 8:58 PM, Paulo Flabiano Smorigo
<pfsmorigo@linux.vnet.ibm.com> wrote:
>
> Virtual lan priority is an option in PowerPC SMS:
>

Packet priorities are entirely optional. I'd love to see real life
case when network does *not* work without setting CoS field. I have
not heard or seen any.

>
> Maybe we can use the priority and DEI of incoming package as the values for the
> following packages. What do you think?
>

That complicates code and will likely remain untested corner case.

>> May be it should really start using proper options at this point
>> keeping existing three argument form as legacy.
>>
>> net_add_addr --if=... --addr=... --mask=... --vlan=... --hw=... card
>
> I prefer the current format but we can switch to another if it's more suitable.
>

I'm fine with leaving current arguments as is, but new ones should go
in as proper options. It makes no sense to reimplement argument
parsing that we already have. Besides

> +      if (argc == vlan_pos + 4)
> +        grub_set_vlantag (&inf->vlantag, &args[vlan_pos]);

So you want to force everyone to always use "vlan 0 0 N" even though
most users will never need first two numbers (and likely even do not
know what these zeroes are there for).

If you start to add CLI support (besides special support for boot time
configuration) you also need to add printing of VLAN tag in
net_list_addr and support for DHCP over VLAN in net_bootp as well.


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-01-15  5:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-23 16:34 Request a freeze exception for vlantag feature Paulo Flabiano Smorigo/Brazil/IBM
2013-12-23 17:02 ` Andrey Borzenkov
2013-12-26 17:25   ` pfsmorigo
2014-01-08 18:57     ` Paulo Flabiano Smorigo
2014-01-09  3:05       ` Andrey Borzenkov
2014-01-09 16:58         ` Paulo Flabiano Smorigo
2014-01-14 19:01           ` Vladimir 'φ-coder/phcoder' Serbinenko
2014-01-15  5:18           ` Andrey Borzenkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.