[RFC PATCH] Ethernet/TR device address representations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



There are multiple styles and code implementations to
format 6 byte hardware device addresses like ethernet
MACs in the kernel.

There are multiple styles:
Most are hex, colon separated, upper case "00:01:02:AA:BB:CC"
Some are hex, colon separated, lower case "00:01:02:aa:bb:cc"
Some are hex, space separated.
A few have no separation at all.

There are multiple implementations:
Some printf a single fixed string "%02X:%02X:%02X:%02X:%02X:%02X"
Some use for (i=0;i<6;i++) printk("%02x%s", addr[i], i==5?"":":");
Some use for (i=0;i<6;i++) printk("%s%02x", i==0?"":":", addr[i]);
etc.

As far as I can identify, there are 449 of these in 213 files.

Here is sample patch fragment to start to unify these.

This patch, which uses an automatic stack variable to hold the
formatted address, reduces the kernel size about 6K if compiling
in all the ethernet drivers.

With:
   text	   data	    bss	    dec	    hex	filename
5955300	1131140	 218920	7305360	 6f7890	vmlinux

Without:
   text	   data	    bss	    dec	    hex	filename
5960991	1131124	 218984	7311099	 6f8efb	vmlinux

A simpler version of this patch, using a direct fixed
string without the automatic on the stack increases the
kernel size about 1K if compiling all the ethernet drivers.

Some of the code if changed might cause breakage in user-space
programs.  I've classified all of the places as harmless, likely
harmless, and probably harmful if anyone in interested.

Comments?  Useful?  Moronic?  u:5% m:95%?
-------------------------------

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7fda03d..6e39bf1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -951,6 +951,23 @@ extern void dev_seq_stop(struct seq_file
 
 extern void linkwatch_run_queue(void);
 
+/*
+ *      Display a 6 byte device address (MAC) in a readable format.
+ */
+
+extern char *__dev_addr6_fmt(char* buf, const unsigned char *addr);
+
+#define DEV_ADDR6_FMT "%s"                   /* expands to: "FF:FF:FF:FF:FF:FF" */
+
+#define DEV_ADDR6_BUF char __dev_addr6_buf[sizeof("FF:FF:FF:FF:FF:FF")]
+#define DEV_ADDR6(addr) __dev_addr6_fmt(__dev_addr6_buf,(const unsigned char*)addr)
+#define DEV_ADDR6_BUF_2 char __dev_addr6_buf_2[sizeof("FF:FF:FF:FF:FF:FF")]
+#define DEV_ADDR6_2(addr) __dev_addr6_fmt(__dev_addr6_buf_2,(const unsigned char*)addr)
+#define DEV_ADDR6_BUF_3 char __dev_addr6_buf_3[sizeof("FF:FF:FF:FF:FF:FF")]
+#define DEV_ADDR6_3(addr) __dev_addr6_fmt(__dev_addr6_buf_3,(const unsigned char*)addr)
+#define DEV_ADDR6_BUF_4 char __dev_addr6_buf_4[sizeof("FF:FF:FF:FF:FF:FF")]
+#define DEV_ADDR6_4(addr) __dev_addr6_fmt(__dev_addr6_buf_4,(const unsigned char*)addr)
+
 #endif /* __KERNEL__ */
 
 #endif	/* _LINUX_DEV_H */

diff --git a/net/core/dev.c b/net/core/dev.c
index ffb8207..7b376af 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -324,6 +324,19 @@ void dev_remove_pack(struct packet_type 
 	synchronize_net();
 }
 
+char* __dev_addr6_fmt(char* buf, const unsigned char* addr)
+{
+	if (buf && addr)
+		sprintf(buf,"%02X:%02X:%02X:%02X:%02X:%02X",
+			addr[0] & 0xff, addr[1] & 0xff,
+			addr[2] & 0xff, addr[3] & 0xff,
+			addr[4] & 0xff, addr[5] & 0xff);
+	return buf;
+}
+
+EXPORT_SYMBOL(__dev_addr6_fmt);
+
+
 /******************************************************************************
 
 		      Device Boot-time Settings Routines
diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index f822cd3..ba9b83d 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -1669,6 +1669,7 @@ static int cp_init_one (struct pci_dev *
 	long pciaddr;
 	unsigned int addr_len, i, pci_using_dac;
 	u8 pci_rev;
+	DEV_ADDR6_BUF;
 
 #ifndef MODULE
 	static int version_printed;
@@ -1815,13 +1816,10 @@ static int cp_init_one (struct pci_dev *
 		goto err_out_iomap;
 
 	printk (KERN_INFO "%s: RTL-8139C+ at 0x%lx, "
-		"%02x:%02x:%02x:%02x:%02x:%02x, "
-		"IRQ %d\n",
+		DEV_ADDR6_FMT ", IRQ %d\n",
 		dev->name,
 		dev->base_addr,
-		dev->dev_addr[0], dev->dev_addr[1],
-		dev->dev_addr[2], dev->dev_addr[3],
-		dev->dev_addr[4], dev->dev_addr[5],
+		DEV_ADDR6(dev->dev_addr),
 		dev->irq);
 
 	pci_set_drvdata(pdev, dev);

--------------------------------

Example of a likely harmless conversions and
multiple addresses in a single printk

--- a/drivers/net/tulip/de4x5.c
+++ b/drivers/net/tulip/de4x5.c
@@ -1089,6 +1089,7 @@ de4x5_hw_init(struct net_device *dev, u_
     struct de4x5_private *lp = netdev_priv(dev);
     struct pci_dev *pdev = NULL;
     int i, status=0;
+    DEV_ADDR6_BUF;
 
     gendev->driver_data = dev;
 
@@ -1124,12 +1125,8 @@ de4x5_hw_init(struct net_device *dev, u_
     dev->base_addr = iobase;
     printk ("%s: %s at 0x%04lx", gendev->bus_id, name, iobase);
     
-    printk(", h/w address ");
     status = get_hw_addr(dev);
-    for (i = 0; i < ETH_ALEN - 1; i++) {     /* get the ethernet addr. */
-	printk("%2.2x:", dev->dev_addr[i]);
-    }
-    printk("%2.2x,\n", dev->dev_addr[i]);
+    printk(", h/w address " DEV_ADDR6_FMT "\n", DEV_ADDR6(dev->dev_addr));
     
     if (status != 0) {
 	printk("      which has an Ethernet PROM CRC error.\n");
@@ -5375,14 +5372,12 @@ de4x5_dbg_open(struct net_device *dev)
 {
     struct de4x5_private *lp = netdev_priv(dev);
     int i;
+    DEV_ADDR6_BUF;
     
     if (de4x5_debug & DEBUG_OPEN) {
 	printk("%s: de4x5 opening with irq %d\n",dev->name,dev->irq);
-	printk("\tphysical address: ");
-	for (i=0;i<6;i++) {
-	    printk("%2.2x:",(short)dev->dev_addr[i]);
-	}
-	printk("\n");
+	printk("\tphysical address: " DEV_ADDR6_FMT "\n",
+	       DEV_ADDR6(dev->dev_addr));
 	printk("Descriptor head addresses:\n");
 	printk("\t0x%8.8lx  0x%8.8lx\n",(u_long)lp->rx_ring,(u_long)lp->tx_ring);
 	printk("Descriptor addresses:\nRX: ");
@@ -5479,19 +5474,16 @@ static void
 de4x5_dbg_srom(struct de4x5_srom *p)
 {
     int i;
+    DEV_ADDR6_BUF;
 
     if (de4x5_debug & DEBUG_SROM) {
 	printk("Sub-system Vendor ID: %04x\n", *((u_short *)p->sub_vendor_id));
 	printk("Sub-system ID:        %04x\n", *((u_short *)p->sub_system_id));
 	printk("ID Block CRC:         %02x\n", (u_char)(p->id_block_crc));
 	printk("SROM version:         %02x\n", (u_char)(p->version));
-	printk("# controllers:         %02x\n", (u_char)(p->num_controllers));
+	printk("# controllers:        %02x\n", (u_char)(p->num_controllers));
 
-	printk("Hardware Address:     ");
-	for (i=0;i<ETH_ALEN-1;i++) {
-	    printk("%02x:", (u_char)*(p->ieee_addr+i));
-	}
-	printk("%02x\n", (u_char)*(p->ieee_addr+i));
+	printk("Hardware Address:     " DEV_ADDR6_FMT "\n", DEV_ADDR6(p->ieee_addr));
 	printk("CRC checksum:         %04x\n", (u_short)(p->chksum));
 	for (i=0; i<64; i++) {
 	    printk("%3d %04x\n", i<<1, (u_short)*((u_short *)p+i));
@@ -5505,21 +5497,12 @@ static void
 de4x5_dbg_rx(struct sk_buff *skb, int len)
 {
     int i, j;
+    DEV_ADDR6_BUF;
+    DEV_ADDR6_BUF_2;
 
     if (de4x5_debug & DEBUG_RX) {
-	printk("R: %02x:%02x:%02x:%02x:%02x:%02x <- %02x:%02x:%02x:%02x:%02x:%02x len/SAP:%02x%02x [%d]\n",
-	       (u_char)skb->data[0],
-	       (u_char)skb->data[1],
-	       (u_char)skb->data[2],
-	       (u_char)skb->data[3],
-	       (u_char)skb->data[4],
-	       (u_char)skb->data[5],
-	       (u_char)skb->data[6],
-	       (u_char)skb->data[7],
-	       (u_char)skb->data[8],
-	       (u_char)skb->data[9],
-	       (u_char)skb->data[10],
-	       (u_char)skb->data[11],
+	printk("R: " DEV_ADDR6_FMT " <- " DEV_ADDR6_FMT " len/SAP:%02x%02x [%d]\n",
+	       DEV_ADDR6(skb->data), DEV_ADDR6_2(&skb->data[6]),
 	       (u_char)skb->data[12],
 	       (u_char)skb->data[13],
 	       len);

-
: send the line "unsubscribe linux-net" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux 802.1Q VLAN]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Git]     [Bugtraq]     [Yosemite News and Information]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux PCI]     [Linux Admin]     [Samba]

  Powered by Linux