Subject: ULE linux kernel code

From: Bernhard Collini-Nocker <[email-masked]>
Date: Thu, 09 Feb 2006 15:24:34 +0100

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;

---

From: Gorry Fairhurst <[email-masked]>
Date: Thu, 09 Feb 2006 14:44:47 +0000

Thanks Bernhard, 

The Spec probably was not intended to be implemented this way, so I'd
suggest we need to look at this seriously. First, can I check one subtle
question below.

> only on unicast addresses the filtering is applied on ALL packets.
Are broadcast packets (MAC ff:ff:ff:ff:ff:ff) also caught by this filter?
- or are they currently always forwarded?

> driver. This kind of stringent filtering, however, may confuse

<snip>.

---

From: Bernhard Collini-Nocker <[email-masked]>
Date: Thu, 09 Feb 2006 16:23:13 +0100

Hi Gorry,

> The Spec probably was not intended to be implemented this way, so I'd

well, yes and no. Yes, because it results in an atypical behaviour at
the receiver side. No, because it is "intended" (maybe this is too
strong worded and "recommended" is more accurate) to operate
IP-multicast over ULE without SNDU DestAddr.

> Are broadcast packets (MAC ff:ff:ff:ff:ff:ff) also caught by this filter?

The code says YES (in case I did not miss something important somewhere
else :-), read yourself
(excerpt from linux-2.6.15.3/drivers/media/dvb/dvb-core/dvb_net.c)

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;
     }
     if (! priv->ule_bridged) {
          skb_push( priv->ule_skb, ETH_ALEN + 2 );
          ethh = (struct ethhdr *)priv->ule_skb->data;
          memcpy( ethh->h_dest, ethh->h_source, ETH_ALEN );
          memset( ethh->h_source, 0, ETH_ALEN );
          ethh->h_proto = htons( priv->ule_sndu_type );
     } else {
          /* Skip the Receiver destination MAC address. */
          skb_pull( priv->ule_skb, ETH_ALEN );
     }
} else {

> - or are they currently always forwarded?

NO. This is likely to be ok, since Broadcast should remain link-local?

>>driver. This kind of stringent filtering, however, may confuse

Yours,
Bernhard

---

From: Gorry Fairhurst <[email-masked]>
Date: Sat, 11 Feb 2006 10:02:12 +0000

The case of D=0 is indeed the default - I think the intention was that 
both modes would work, and I welcome this initiative to update the 
implementation!

(I'll also reply to the earlier email)

Gorry

---

From: Gorry Fairhurst <[email-masked]>
Date: Tue, 14 Feb 2006 11:30:11 +0000

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:

> ------------------------------------------------------------------------
>