[PATCH] BNEP multicast filter is filtering multicast addresses in default case although differently specified

[PATCH] BNEP multicast filter is filtering multicast addresses in default case although differently specified

[Danny Schweizer]

This patch fixes the Bug 106691 by changing the default multicast filter
to not filter out any multicast addresses.

---
 net/bluetooth/bnep/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
index 1641367..ba2467a 100644
--- a/net/bluetooth/bnep/core.c
+++ b/net/bluetooth/bnep/core.c
@@ -609,7 +609,7 @@ int bnep_add_connection(struct bnep_connadd_req
*req, struct socket *sock)

 #ifdef CONFIG_BT_BNEP_MC_FILTER
 	/* Set default mc filter */
-	set_bit(bnep_mc_hash(dev->broadcast), (ulong *) &s->mc_filter);
+	s->mc_filter = ~0LL;
 #endif

 #ifdef CONFIG_BT_BNEP_PROTO_FILTER
-- 
2.1.4

Re: [PATCH] BNEP multicast filter is filtering multicast addresses in default case although differently specified

[Marcel Holtmann]

Hi Danny,

> This patch fixes the Bug 106691 by changing the default multicast filter
> to not filter out any multicast addresses.
> 
> ---
> net/bluetooth/bnep/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
> index 1641367..ba2467a 100644
> --- a/net/bluetooth/bnep/core.c
> +++ b/net/bluetooth/bnep/core.c
> @@ -609,7 +609,7 @@ int bnep_add_connection(struct bnep_connadd_req
> *req, struct socket *sock)
> 
> #ifdef CONFIG_BT_BNEP_MC_FILTER
> 	/* Set default mc filter */
> -	set_bit(bnep_mc_hash(dev->broadcast), (ulong *) &s->mc_filter);
> +	s->mc_filter = ~0LL;
> #endif

I am not convinced that these two are actually the same expressions. You need to explain this a lot more in detail on why this change is correct.

Regards

Marcel

Re: [PATCH] BNEP multicast filter is filtering multicast addresses in default case although differently specified

[Danny Schweizer]

Hi Marcel,


> Hi Danny,
> 
>> This patch fixes the Bug 106691 by changing the default multicast filter
>> to not filter out any multicast addresses.
>>
>> ---
>> net/bluetooth/bnep/core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
>> index 1641367..ba2467a 100644
>> --- a/net/bluetooth/bnep/core.c
>> +++ b/net/bluetooth/bnep/core.c
>> @@ -609,7 +609,7 @@ int bnep_add_connection(struct bnep_connadd_req
>> *req, struct socket *sock)
>>
>> #ifdef CONFIG_BT_BNEP_MC_FILTER
>> 	/* Set default mc filter */
>> -	set_bit(bnep_mc_hash(dev->broadcast), (ulong *) &s->mc_filter);
>> +	s->mc_filter = ~0LL;
>> #endif
> 
> I am not convinced that these two are actually the same expressions. You need to explain this a lot more in detail on why this change is correct.
> 
> Regards
> 
> Marcel

The new expression should set the default mc_filter such that it does
not filter out any multicast addresses (it sets the mc_filter to all
ones). Up to now, the default mc_filter filters out all multicast
addresses except for the broadcast address. The reason why this is
needed is explained in the description of bug 106691. I will copy it here:

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.


Regards,
Danny

Re: [PATCH] BNEP multicast filter is filtering multicast addresses in default case although differently specified

[Marcel Holtmann]

Hi Danny,

>>> This patch fixes the Bug 106691 by changing the default multicast filter
>>> to not filter out any multicast addresses.
>>> 
>>> ---
>>> net/bluetooth/bnep/core.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
>>> index 1641367..ba2467a 100644
>>> --- a/net/bluetooth/bnep/core.c
>>> +++ b/net/bluetooth/bnep/core.c
>>> @@ -609,7 +609,7 @@ int bnep_add_connection(struct bnep_connadd_req
>>> *req, struct socket *sock)
>>> 
>>> #ifdef CONFIG_BT_BNEP_MC_FILTER
>>> 	/* Set default mc filter */
>>> -	set_bit(bnep_mc_hash(dev->broadcast), (ulong *) &s->mc_filter);
>>> +	s->mc_filter = ~0LL;
>>> #endif
>> 
>> I am not convinced that these two are actually the same expressions. You need to explain this a lot more in detail on why this change is correct.
>> 
> 
> The new expression should set the default mc_filter such that it does
> not filter out any multicast addresses (it sets the mc_filter to all
> ones). Up to now, the default mc_filter filters out all multicast
> addresses except for the broadcast address. The reason why this is
> needed is explained in the description of bug 106691. I will copy it here:
> 
> 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.

and something along these lines should be in the commit message as explanation for your change. And the reference to the specification should be added as an extra comment in the code.

Regards

Marcel

Re: [PATCH] BNEP multicast filter is filtering multicast addresses in default case although differently specified

[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] BNEP multicast filter is filtering multicast addresses in default case although differently specified

[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(-)

I can not apply this patch since it comes in weirdly. Please send a clean copy and shorten the subject line so it fits appropriately. And maybe use git send-email.

Regards

Marcel

[PATCH] BNEP multicast filter is filtering multicast addresses in default case although differently specified

[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