[B.A.T.M.A.N.] Combination of flags in tt_query packets

[B.A.T.M.A.N.] Combination of flags in tt_query packets

[Sven Eckelmann]

[-- Attachment #1: Type: Text/Plain, Size: 917 bytes --]

Hi,

just looked a little bit closer at your tt_query implementation and was 
reminded that we had a small discussion in IRC some weeks ago.

<ecsv> can those flags be mixed?
<ordex> mixed? in which way?
<ecsv> request | table -> still a valid package
<ecsv> sry, packet
<ordex> yes it is, because it means "I'm requesting a full table"
<ordex> all the combinations are valid

This sounds wrong when I look at the tt_query packet. There we have a tt_data 
field which can used used exclusive by either TT_REQUEST (x)or TT_RESPONSE. 
Therefore, it is not possible to use both flags at the same time. Can you 
please explain how this is handled or otherwise change it so that only one bit 
in flags is used to decide if it is an response or a request.

Maybe this was the result of the discussion with Marek about the roaming stuff 
- but i don't think that it applies here.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] Combination of flags in tt_query packets

[Antonio Quartulli]

Hi Sven, hi all,

On Sun, Jun 12, 2011 at 11:52:00AM +0200, Sven Eckelmann wrote:
> Hi,
> 
> just looked a little bit closer at your tt_query implementation and was 
> reminded that we had a small discussion in IRC some weeks ago.
> 
> <ecsv> can those flags be mixed?
> <ordex> mixed? in which way?
> <ecsv> request | table -> still a valid package
> <ecsv> sry, packet
> <ordex> yes it is, because it means "I'm requesting a full table"
> <ordex> all the combinations are valid
> 
> This sounds wrong when I look at the tt_query packet. There we have a tt_data 
> field which can used used exclusive by either TT_REQUEST (x)or TT_RESPONSE. 
> Therefore, it is not possible to use both flags at the same time. Can you 
> please explain how this is handled or otherwise change it so that only one bit 
> in flags is used to decide if it is an response or a request.
> 

At the beginning we had one bit only with this meaning:

0 => TT_REQUEST
1 => TT_RESPONSE

But then we had a disussion about:
1) what will we do if we want to add more packet types in the future?
2) is it correct or not to use 0x0 as flag?

Regarding 1) I thought that using a "two bits flag" could help in reserving two 
configurations more for the future (0x0 and 0x3).

Regarding 2) we switched to TT_REQ = 0x01 and TT_RESP = 0x2.

In my opinion, leaving tt_req = 0x0 and tt_resp = 0x1 would be better,
but it seemed to be not so correct.


I hope my explanation is clear.
Morover I think that someone should explain me if using 0x0 as a
meaningful flag is correct or not :P (I know that field & 0x0 will
always be false :P)

> Maybe this was the result of the discussion with Marek about the roaming stuff 
> - but i don't think that it applies here.

No, this is not related to the roaming stuff.


Regards,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

Re: [B.A.T.M.A.N.] Combination of flags in tt_query packets

[Sven Eckelmann]

[-- Attachment #1: Type: Text/Plain, Size: 2511 bytes --]

Antonio Quartulli wrote:
> > just looked a little bit closer at your tt_query implementation and was
> > reminded that we had a small discussion in IRC some weeks ago.
> > 
> > <ecsv> can those flags be mixed?
> > <ordex> mixed? in which way?
> > <ecsv> request | table -> still a valid package
> > <ecsv> sry, packet
> > <ordex> yes it is, because it means "I'm requesting a full table"
> > <ordex> all the combinations are valid
> > 
> > This sounds wrong when I look at the tt_query packet. There we have a
> > tt_data field which can used used exclusive by either TT_REQUEST (x)or
> > TT_RESPONSE. Therefore, it is not possible to use both flags at the same
> > time. Can you please explain how this is handled or otherwise change it
> > so that only one bit in flags is used to decide if it is an response or
> > a request.
> 
> At the beginning we had one bit only with this meaning:
> 
> 0 => TT_REQUEST
> 1 => TT_RESPONSE

Could be correct - never followed the discussion in detail. I asked Marek and 
he said something about roaming.

> But then we had a disussion about:
> 1) what will we do if we want to add more packet types in the future?

Depends on the type. For example full-table creates different packet types 
together with the TT_REQUEST and TT_RESPONSE. But using TT_REQUEST and 
TT_RESPONSE together doesn't work at all. So forget about the flag and think 
more about a currently undefined amount of bits which encode the type of 
packet. Encoding the request and reply in two different bits and call them 
flags doesn't really make sense. 

> 2) is it correct or not to use 0x0 as flag?

A not existing bit position is not really a "flag". The bit at position zero 
(lsb) could be used as flag to decide if it is a request or a response - but 
using nothing to store something is... <please insert your favourite swear 
word here>.

> Regarding 1) I thought that using a "two bits flag" could help in reserving
> two configurations more for the future (0x0 and 0x3).

Then you say that you don't use flags but 2 bits to store 2 values. Please 
don't use it in your code as flags, but use something like (x & TT_TYPE_MASK) 
to extract the corresponding bits and compare the resulting values against 
representation of TT_RESPONSE and TT_RESPONSE (TT_TYPE_MASK would be 0x3, 
TT_RESPONSE 0 and TT_REQUEST 3). Also remove the TT_RESPONSE and TT_REQUEST 
from the enum tt_query_flags and use another enum for them.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] Combination of flags in tt_query packets

[Sven Eckelmann]

[-- Attachment #1: Type: Text/Plain, Size: 557 bytes --]

Sven Eckelmann wrote:
> Then you say that you don't use flags but 2 bits to store 2 values. Please
> don't use it in your code as flags, but use something like (x &
> TT_TYPE_MASK) to extract the corresponding bits and compare the resulting
> values against representation of TT_RESPONSE and TT_RESPONSE (TT_TYPE_MASK
> would be 0x3, TT_RESPONSE 0 and TT_REQUEST 3). Also remove the TT_RESPONSE
> and TT_REQUEST from the enum tt_query_flags and use another enum for them.

s/TT_REQUEST 3/TT_REQUEST 2/

1 and 3 are currently undefined.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] Combination of flags in tt_query packets

[Antonio Quartulli]

On Mon, Jun 13, 2011 at 04:29:16PM +0200, Sven Eckelmann wrote:
> Antonio Quartulli wrote:
> > > just looked a little bit closer at your tt_query implementation and was
> > > reminded that we had a small discussion in IRC some weeks ago.
> > > 
> > > <ecsv> can those flags be mixed?
> > > <ordex> mixed? in which way?
> > > <ecsv> request | table -> still a valid package
> > > <ecsv> sry, packet
> > > <ordex> yes it is, because it means "I'm requesting a full table"
> > > <ordex> all the combinations are valid
> > > 
> > > This sounds wrong when I look at the tt_query packet. There we have a
> > > tt_data field which can used used exclusive by either TT_REQUEST (x)or
> > > TT_RESPONSE. Therefore, it is not possible to use both flags at the same
> > > time. Can you please explain how this is handled or otherwise change it
> > > so that only one bit in flags is used to decide if it is an response or
> > > a request.
> > 
> > At the beginning we had one bit only with this meaning:
> > 
> > 0 => TT_REQUEST
> > 1 => TT_RESPONSE
> 
> Could be correct - never followed the discussion in detail. I asked Marek and 
> he said something about roaming.

We did something similar for the tt_change flag field. I think I have to
give a look at it too.


> 
> > But then we had a disussion about:
> > 1) what will we do if we want to add more packet types in the future?
> 
> Depends on the type. For example full-table creates different packet types 
> together with the TT_REQUEST and TT_RESPONSE. But using TT_REQUEST and 
> TT_RESPONSE together doesn't work at all. So forget about the flag and think 
> more about a currently undefined amount of bits which encode the type of 
> packet. Encoding the request and reply in two different bits and call them 
> flags doesn't really make sense. 
> 
> > 2) is it correct or not to use 0x0 as flag?
> 
> A not existing bit position is not really a "flag". The bit at position zero 
> (lsb) could be used as flag to decide if it is a request or a response - but 
> using nothing to store something is... <please insert your favourite swear 
> word here>.
> 
> > Regarding 1) I thought that using a "two bits flag" could help in reserving
> > two configurations more for the future (0x0 and 0x3).
> 
> Then you say that you don't use flags but 2 bits to store 2 values. Please 
> don't use it in your code as flags, but use something like (x & TT_TYPE_MASK) 
> to extract the corresponding bits and compare the resulting values against 
> representation of TT_RESPONSE and TT_RESPONSE (TT_TYPE_MASK would be 0x3, 
> TT_RESPONSE 0 and TT_REQUEST 3). Also remove the TT_RESPONSE and TT_REQUEST 
> from the enum tt_query_flags and use another enum for them.

Ok, I agree. I'm going to propose a patch to clean it up. Thanks for all
the suggestions.

Regards,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara