[gPXE-devel] [PATCH] 802.1Q support for gpxe

[michael-dev at fami-braun.de]

Hi Michael,

thanks for that quick reply.

Michael Brown schrieb:
> char name[] in struct net_device may as well be 12 rather than 10, since it's 
> going to get dword-aligned anyway.

done

> 
> Why is ieee8021q_transmit() creating a new iobuf?

As netdev_tx registers the iobuf in netdev->tx_queue, though
the iobuf cannot be registered in multiple lists, and because in case
of an error, netdev_tx of the raw device freeed the iobuf and netdev_tx
of the vlan device will also free the buffer.

> 
> Device open counts should be implemented in net/netdevice.c within 
> netdev_open() and netdev_close(); you can use ib_open() and ib_close() in 
> net/infiniband.c as a reference.  

done

> The NETDEV_OPEN flag must be removed and
> replaced with something like a static inline netdev_is_open() which just 
> checks the netdev's open count.  This change should probably be in its own 
> separate commit.

done

> 
> I would try to avoid calling fetch_uintz_setting() for each transmitted 
> packet.  Better to hold the VLAN tag in a structure member.  For example:
> 
> struct vlan_device {
>   /* Underlying net device */
>   struct net_device *netdev;
>   /* VLAN tag */
>   uint32_t tag;
> };
> 
> with the struct vlan_device forming the private data of the net_device you 
> create.

done

> 
> Lastly, is 802.1Q Ethernet-specific?  In that case, you may as well just treat 
> it as its own static struct ll_protocol, rather than trying to dynamically 
> create a ll_protocol based on the underlying net_device.  (gPXE isn't really 
> set up to handle a struct ll_protocol that might be freed; they're assumed to 
> be static and permanent.)

802.1Q can be used with Ethernet and FDDI. For gpxe, I wanted to support
 ethernet and 802.11 link layer structs. Anyway, the ll_protocol of
netdev only is setup and freed when the device is created / destroyed,
so nobody should hold a reference to the virtual netdev anymore when the
ll_protocol is freed or changed.

Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 01netopen.diff
Type: text/x-patch
Size: 9719 bytes
Desc: not available
Url : http://etherboot.org/pipermail/gpxe-devel/attachments/20100310/83eb3e5b/attachment-0003.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 02vlans.diff
Type: text/x-patch
Size: 21987 bytes
Desc: not available
Url : http://etherboot.org/pipermail/gpxe-devel/attachments/20100310/83eb3e5b/attachment-0004.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 260 bytes
Desc: OpenPGP digital signature
Url : http://etherboot.org/pipermail/gpxe-devel/attachments/20100310/83eb3e5b/attachment-0005.bin