[PATCH] Do not filter multicast addresses by default

[PATCH] Do not filter multicast addresses by default

[Danny Schweizer]

A Linux PC is connected with another device over Bluetooth PAN using a BNEP interface.

Whenever a packet is tried to be sent over the BNEP interface, the function "bnep_net_xmit()" in "net/bluetooth/bnep/netdev.c" is called. This function calls "bnep_net_mc_filter()", which checks (if the destination address is multicast) if the address is set in a certain multicast filter (&s->mc_filter). If it is not, then it is not sent out.

This filter is only changed in two other functions, found in net/bluetooth/bnep/core.c": in "bnep_ctrl_set_mc_filter()", which is only called if a message of type "BNEP_FILTER_MULTI_ADDR_SET" is received. Otherwise, it is set in "bnep_add_connection()", where it is set to a default value which only adds the broadcast address to the filter:

set_bit(bnep_mc_hash(dev->broadcast), (ulong *) &s->mc_filter);

To sum up, if the BNEP interface does not receive any message of type "BNEP_FILTER_MULTI_ADDR_SET", it will not send out any messages with multicast destination addresses except for broadcast.

However, in the BNEP specification (page 27 in http://grouper.ieee.org/groups/802/15/Bluetooth/BNEP.pdf), it is said that per default, all multicast addresses should not be filtered, i.e. the BNEP interface should be able to send packets with any multicast destination address.

It seems that the default case is wrong: the multicast filter should not block almost all multicast addresses, but should not filter out any.

This leads to the problem that e.g. Neighbor Solicitation messages sent with Bluetooth PAN over the BNEP interface to a multicast destination address other than broadcast are blocked and not sent out.

Therefore, in the default case, we set the mc_filter to ~0LL to not filter out any multicast addresses.
---
 net/bluetooth/bnep/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
index 1641367..8e02289 100644
--- a/net/bluetooth/bnep/core.c
+++ b/net/bluetooth/bnep/core.c
@@ -608,8 +608,12 @@ int bnep_add_connection(struct bnep_connadd_req *req, struct socket *sock)
 	s->msg.msg_flags = MSG_NOSIGNAL;
 
 #ifdef CONFIG_BT_BNEP_MC_FILTER
-	/* Set default mc filter */
-	set_bit(bnep_mc_hash(dev->broadcast), (ulong *) &s->mc_filter);
+	/* 
+	 * Set default mc filter to not filter out any mc addresses
+	 * as defined in the BNEP specification (revision 0.95a)
+	 * http://grouper.ieee.org/groups/802/15/Bluetooth/BNEP.pdf
+	 */
+	s->mc_filter = ~0LL;
 #endif
 
 #ifdef CONFIG_BT_BNEP_PROTO_FILTER
-- 
2.1.4

Re: [PATCH] Do not filter multicast addresses by default

[Marcel Holtmann]

Hi Danny,

> A Linux PC is connected with another device over Bluetooth PAN using a BNEP interface.
> 
> Whenever a packet is tried to be sent over the BNEP interface, the function "bnep_net_xmit()" in "net/bluetooth/bnep/netdev.c" is called. This function calls "bnep_net_mc_filter()", which checks (if the destination address is multicast) if the address is set in a certain multicast filter (&s->mc_filter). If it is not, then it is not sent out.
> 
> This filter is only changed in two other functions, found in net/bluetooth/bnep/core.c": in "bnep_ctrl_set_mc_filter()", which is only called if a message of type "BNEP_FILTER_MULTI_ADDR_SET" is received. Otherwise, it is set in "bnep_add_connection()", where it is set to a default value which only adds the broadcast address to the filter:
> 
> set_bit(bnep_mc_hash(dev->broadcast), (ulong *) &s->mc_filter);
> 
> To sum up, if the BNEP interface does not receive any message of type "BNEP_FILTER_MULTI_ADDR_SET", it will not send out any messages with multicast destination addresses except for broadcast.
> 
> However, in the BNEP specification (page 27 in http://grouper.ieee.org/groups/802/15/Bluetooth/BNEP.pdf), it is said that per default, all multicast addresses should not be filtered, i.e. the BNEP interface should be able to send packets with any multicast destination address.
> 
> It seems that the default case is wrong: the multicast filter should not block almost all multicast addresses, but should not filter out any.
> 
> This leads to the problem that e.g. Neighbor Solicitation messages sent with Bluetooth PAN over the BNEP interface to a multicast destination address other than broadcast are blocked and not sent out.
> 
> Therefore, in the default case, we set the mc_filter to ~0LL to not filter out any multicast addresses.
> ---
> net/bluetooth/bnep/core.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
> index 1641367..8e02289 100644
> --- a/net/bluetooth/bnep/core.c
> +++ b/net/bluetooth/bnep/core.c
> @@ -608,8 +608,12 @@ int bnep_add_connection(struct bnep_connadd_req *req, struct socket *sock)
> 	s->msg.msg_flags = MSG_NOSIGNAL;
> 
> #ifdef CONFIG_BT_BNEP_MC_FILTER
> -	/* Set default mc filter */
> -	set_bit(bnep_mc_hash(dev->broadcast), (ulong *) &s->mc_filter);
> +	/* 
> +	 * Set default mc filter to not filter out any mc addresses
> +	 * as defined in the BNEP specification (revision 0.95a)
> +	 * http://grouper.ieee.org/groups/802/15/Bluetooth/BNEP.pdf
> +	 */
> +	s->mc_filter = ~0LL;
> #endif
> 

Applying: Do not filter multicast addresses by default
/data/kernel/maintainer-bluetooth-next/.git/rebase-apply/patch:15: trailing whitespace.
	/* 
fatal: 1 line adds whitespace errors.
Patch failed at 0001 Do not filter multicast addresses by default

And please prefix the subject line with Bluetooth:

Regards

Marcel

Re: [PATCH] Do not filter multicast addresses by default

[Johan Hedberg]

Hi,

On Mon, Dec 07, 2015, Marcel Holtmann wrote:
> > A Linux PC is connected with another device over Bluetooth PAN using a BNEP interface.
> > 
> > Whenever a packet is tried to be sent over the BNEP interface, the function "bnep_net_xmit()" in "net/bluetooth/bnep/netdev.c" is called. This function calls "bnep_net_mc_filter()", which checks (if the destination address is multicast) if the address is set in a certain multicast filter (&s->mc_filter). If it is not, then it is not sent out.
> > 
> > This filter is only changed in two other functions, found in net/bluetooth/bnep/core.c": in "bnep_ctrl_set_mc_filter()", which is only called if a message of type "BNEP_FILTER_MULTI_ADDR_SET" is received. Otherwise, it is set in "bnep_add_connection()", where it is set to a default value which only adds the broadcast address to the filter:
> > 
> > set_bit(bnep_mc_hash(dev->broadcast), (ulong *) &s->mc_filter);
> > 
> > To sum up, if the BNEP interface does not receive any message of type "BNEP_FILTER_MULTI_ADDR_SET", it will not send out any messages with multicast destination addresses except for broadcast.
> > 
> > However, in the BNEP specification (page 27 in http://grouper.ieee.org/groups/802/15/Bluetooth/BNEP.pdf), it is said that per default, all multicast addresses should not be filtered, i.e. the BNEP interface should be able to send packets with any multicast destination address.
> > 
> > It seems that the default case is wrong: the multicast filter should not block almost all multicast addresses, but should not filter out any.
> > 
> > This leads to the problem that e.g. Neighbor Solicitation messages sent with Bluetooth PAN over the BNEP interface to a multicast destination address other than broadcast are blocked and not sent out.
> > 
> > Therefore, in the default case, we set the mc_filter to ~0LL to not filter out any multicast addresses.
> > ---
> > net/bluetooth/bnep/core.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
> > index 1641367..8e02289 100644
> > --- a/net/bluetooth/bnep/core.c
> > +++ b/net/bluetooth/bnep/core.c
> > @@ -608,8 +608,12 @@ int bnep_add_connection(struct bnep_connadd_req *req, struct socket *sock)
> > 	s->msg.msg_flags = MSG_NOSIGNAL;
> > 
> > #ifdef CONFIG_BT_BNEP_MC_FILTER
> > -	/* Set default mc filter */
> > -	set_bit(bnep_mc_hash(dev->broadcast), (ulong *) &s->mc_filter);
> > +	/* 
> > +	 * Set default mc filter to not filter out any mc addresses
> > +	 * as defined in the BNEP specification (revision 0.95a)
> > +	 * http://grouper.ieee.org/groups/802/15/Bluetooth/BNEP.pdf
> > +	 */
> > +	s->mc_filter = ~0LL;
> > #endif
> > 
> 
> Applying: Do not filter multicast addresses by default
> /data/kernel/maintainer-bluetooth-next/.git/rebase-apply/patch:15: trailing whitespace.
> 	/* 
> fatal: 1 line adds whitespace errors.
> Patch failed at 0001 Do not filter multicast addresses by default
> 
> And please prefix the subject line with Bluetooth:

There's also a missing Signed-off-by and the lines should be wrapped at
some 74 characters (right now it's basically a single line per
paragraph).

Johan

Re: [PATCH] Do not filter multicast addresses by default

[Danny Schweizer]

Hi,

Am 07.12.2015 um 19:02 schrieb Johan Hedberg:
> Hi,
> 
> On Mon, Dec 07, 2015, Marcel Holtmann wrote:
>>> A Linux PC is connected with another device over Bluetooth PAN using a BNEP interface.
>>>
>>> Whenever a packet is tried to be sent over the BNEP interface, the function "bnep_net_xmit()" in "net/bluetooth/bnep/netdev.c" is called. This function calls "bnep_net_mc_filter()", which checks (if the destination address is multicast) if the address is set in a certain multicast filter (&s->mc_filter). If it is not, then it is not sent out.
>>>
>>> This filter is only changed in two other functions, found in net/bluetooth/bnep/core.c": in "bnep_ctrl_set_mc_filter()", which is only called if a message of type "BNEP_FILTER_MULTI_ADDR_SET" is received. Otherwise, it is set in "bnep_add_connection()", where it is set to a default value which only adds the broadcast address to the filter:
>>>
>>> set_bit(bnep_mc_hash(dev->broadcast), (ulong *) &s->mc_filter);
>>>
>>> To sum up, if the BNEP interface does not receive any message of type "BNEP_FILTER_MULTI_ADDR_SET", it will not send out any messages with multicast destination addresses except for broadcast.
>>>
>>> However, in the BNEP specification (page 27 in http://grouper.ieee.org/groups/802/15/Bluetooth/BNEP.pdf), it is said that per default, all multicast addresses should not be filtered, i.e. the BNEP interface should be able to send packets with any multicast destination address.
>>>
>>> It seems that the default case is wrong: the multicast filter should not block almost all multicast addresses, but should not filter out any.
>>>
>>> This leads to the problem that e.g. Neighbor Solicitation messages sent with Bluetooth PAN over the BNEP interface to a multicast destination address other than broadcast are blocked and not sent out.
>>>
>>> Therefore, in the default case, we set the mc_filter to ~0LL to not filter out any multicast addresses.
>>> ---
>>> net/bluetooth/bnep/core.c | 8 ++++++--
>>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
>>> index 1641367..8e02289 100644
>>> --- a/net/bluetooth/bnep/core.c
>>> +++ b/net/bluetooth/bnep/core.c
>>> @@ -608,8 +608,12 @@ int bnep_add_connection(struct bnep_connadd_req *req, struct socket *sock)
>>> 	s->msg.msg_flags = MSG_NOSIGNAL;
>>>
>>> #ifdef CONFIG_BT_BNEP_MC_FILTER
>>> -	/* Set default mc filter */
>>> -	set_bit(bnep_mc_hash(dev->broadcast), (ulong *) &s->mc_filter);
>>> +	/* 
>>> +	 * Set default mc filter to not filter out any mc addresses
>>> +	 * as defined in the BNEP specification (revision 0.95a)
>>> +	 * http://grouper.ieee.org/groups/802/15/Bluetooth/BNEP.pdf
>>> +	 */
>>> +	s->mc_filter = ~0LL;
>>> #endif
>>>
>>
>> Applying: Do not filter multicast addresses by default
>> /data/kernel/maintainer-bluetooth-next/.git/rebase-apply/patch:15: trailing whitespace.
>> 	/* 
>> fatal: 1 line adds whitespace errors.
>> Patch failed at 0001 Do not filter multicast addresses by default
>>
>> And please prefix the subject line with Bluetooth:
> 
> There's also a missing Signed-off-by and the lines should be wrapped at
> some 74 characters (right now it's basically a single line per
> paragraph).
> 
> Johan
> 

Thanks for your comments. I just sent a new version of the patch to the
mailing list.

Regards,
Danny