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

Re: ULE linux kernel code




So, after looking through the code,

I think both 2.1 and 2.2 are 100% conformant to the RFC.

2.1 seems a reasonable approach to me, and thinking about this is, it is what I would expect to happen. That is: The driver passes ALL broadcast, any specific unicast and all multicast FRAMES.

It would of course be nice to provide multicast filtering at a place that is lower in the stack to enable the speedy removal of any mcast frame for which there is no IP listening socket for the group. However, I agree this is a tuning measure with some additional complexity.

I'd vote for approach 2.1, as you suggested.

Gorry

Bernhard Collini-Nocker wrote:

Dear all,

a while ago we have noticed that the ULE code in the recent Linux 2.6
kernels (from 2.6.10 onwards, when ULE with optional headers was added)
implements a "non-obvious" handling of IP-multicast and IP-broadcast
packets in the one case when D bit (SNDU Destination Address Absent
flag) is not set (equals 0), or in other words when a SNDU Destination
Address (NPA/MAC) address is present. Instead of applying the filter
only on unicast addresses the filtering is applied on ALL packets.
Consequently multicast/broadcast packets (whose SNDU DestAddr does not
match the one set interface hardware address) are discarded in the
driver. This kind of stringent filtering, however, may confuse
operators/users.

The purpose of this e-mail is to ask you what your expected/suggested
action wrt this is:
1. leave as is (but clearly tell users/operators that IP-multicast ONLY
   works without NPA/MAC destination address, i.e. D bit MUST equal 1
   for multicast/broadcast)
2. modify the Linux kernel source and commit the changes
2.1. add a simple rule to treat highest bit of IP address as
     multicast/broadcast address indicator and leave filtering up to IP
     stack
2.2. add the standard NIC handling for multicast via filters in using a
     list of multicast adresses to be filtered in the driver (or NIC)

There are some pros and cons for each of the proposed actions, some of
which are:
- case 1. would make multiplexing of unicast and multicast flows onto
  the same PID more tricky in the encapsulator
- case 1. is not 100 per cent compliant to the ULE RFC
- case 2. would require some effort
- case 2.1. is easy to implement (see attachment) and in most cases
            (provided that reasonable IP-flow/PID mapping is performed)
            sufficient
- case 2.1. also easily covers/solves the IP6 multicast case (although
            v6 is likely a different story of itself)
- case 2.2. requires most effort, would be 100 per cent ULE RFC conform,
            but probably lacks support from many/most NICs and as such
            performs no better than case 2.1.

Appreciating all your opinions and comments on this subject.

Regards,
Bernhard


PS: Hilmar has provided attached "patch" to satisfy case 2.1. some time
ago and I think it is reasonable to share it with you.


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

--- linux-2.6.13.2/drivers/media/dvb/dvb-core/dvb_net.c	2005-09-17 03:02:12.000000000 +0200
+++ linux-2.6.12.3/drivers/media/dvb/dvb-core/dvb_net.c	2005-09-27 09:32:21.000000000 +0200
@@ -42,6 +42,8 @@
  *                     Bugfixes and robustness improvements.
  *                     Filtering on dest MAC addresses, if present (D-Bit = 0)
  *                     ULE_DEBUG compile-time option.
+ * Sep 2005: hl:       Fixed Broadcast and multicast handling if MAC address
+ * is present (D-Bit = 0) */ /*
@@ -224,10 +226,7 @@
static int ule_bridged_sndu( struct dvb_net_priv *p )
 {
-	/* BRIDGE SNDU handling sucks in draft-ietf-ipdvb-ule-03.txt.
-	 * This has to be the last extension header, otherwise it won't work.
-	 * Blame the authors!
-	 */
+	/* The BRIDGE SNDU has to be the last extension header, otherwise it won't work. */
 	p->ule_bridged = 1;
 	return 0;
 }
@@ -635,13 +634,24 @@
/* Filter on receiver's destination MAC address, if present. */
 				if (!priv->ule_dbit) {
-					/* The destination MAC address is the next data in the skb. */
-					if (memcmp( priv->ule_skb->data, dev->dev_addr, ETH_ALEN )) {
-						/* MAC addresses don't match.  Drop SNDU. */
-						// printk( KERN_WARNING "Dropping SNDU, MAC address.\n" );
-						dev_kfree_skb( priv->ule_skb );
-						goto sndu_done;
-					}
+					/* Do not filter if the interface is in promiscous mode */
+					if (!(dev->flags & IFF_PROMISC)) {
+						const char baddr[6] = {0xff,};
+						/* Check if the destination MAC address is a broadcast MAC */	
+						/* The destination MAC address is the next data in the skb. */
+						if (memcmp( priv->ule_skb->data, &baddr, ETH_ALEN )) {
+							/* No broadcast, check for the multicast bit in the MAC next */
+							if (!(priv->ule_skb->data[0] & 0x01) && !(dev->flags & IFF_ALLMULTI)) {
+								/* no multicast address as well */
+								if (memcmp( priv->ule_skb->data, dev->dev_addr, ETH_ALEN )) {
+									/* MAC addresses don't match.  Drop SNDU. */
+									// printk( KERN_WARNING "Dropping SNDU, MAC address.\n" );
+									dev_kfree_skb( priv->ule_skb );
+									goto sndu_done;
+								}
+							}
+						}
+					}	
 					if (! priv->ule_bridged) {
 						skb_push( priv->ule_skb, ETH_ALEN + 2 );
 						ethh = (struct ethhdr *)priv->ule_skb->data;