mbox

pull request: batman-adv 2012-11-21

Message ID 1353499919-28596-1-git-send-email-ordex@autistici.org (mailing list archive)
State Not Applicable, archived
Headers

Pull-request

git://git.open-mesh.org/linux-merge.git tags/batman-adv-for-davem

Message

Antonio Quartulli Nov. 21, 2012, 12:11 p.m. UTC
  Hello David,

here is again our changeset intended for net-next/linux-3.8.
Since the last pull request I removed the patch we were discussing about (the
one exporting the batman-adv compatibility version) as we intend to do not
provide the user the feeling of being "incompatible everyday".

As we discussed some time ago, we are doing what we can to speed our
stabilisation process up and we are not going to break compatibility anymore
before we ultimate such process.

However, I see that every now and then you have the feeling we are not really
doing so. In that case, please, point out what you think is not adequate so that
we can quickly fix it.

Again: we want to stabilise the protocol/messages format and any help/suggestion
from you or the other maintainers is always well accepted (other than being
"rude" only or not answering at all. When you do so, then we have to spend the
entire day in trying to decrypt what was wrong and what we should do to address
the problem, limiting our fun and productivity).


Please pull or let me know if there is any other problem in this batch.


Thank you very much,
		Antonio


The following changes since commit de4594a51c904ddcd6c3a6cdd100f7c1d94d3239:

  sctp: send abort chunk when max_retrans exceeded (2012-11-20 15:50:37 -0500)

are available in the git repository at:

  git://git.open-mesh.org/linux-merge.git tags/batman-adv-for-davem

for you to fetch changes up to e022b956c11084f3ee1c6ece523e14ac07b7c645:

  batman-adv: Use packing of 2 for all headers before an ethernet header (2012-11-21 12:35:47 +0100)

----------------------------------------------------------------
Included changes:
- Increase batman-adv version
- Bridge Loop Avoidance: compute checksum (using crc32) on skb fragments instead
  of linearising it
- sort the sysfs documentation
- some other minor cleanups

----------------------------------------------------------------
Antonio Quartulli (1):
      batman-adv: support array of debugfs general attributes

Marek Lindner (1):
      batman-adv: sysfs documentation should keep alphabetical order

Martin Hundebøll (1):
      batman-adv: Add wrapper to look up neighbor and send skb

Simon Wunderlich (2):
      batman-adv: fix bla compare function
      batman-adv: Fix broadcast duplist for fragmentation

Sven Eckelmann (4):
      batman-adv: Mark best gateway in transtable_global debugfs
      batman-adv: Add function to calculate crc32c for the skb payload
      batman-adv: Start new development cycle
      batman-adv: Use packing of 2 for all headers before an ethernet header

 .../ABI/testing/sysfs-class-net-batman-adv         |  11 +-
 Documentation/ABI/testing/sysfs-class-net-mesh     |  40 +++---
 net/batman-adv/Kconfig                             |   1 +
 net/batman-adv/bridge_loop_avoidance.c             |  36 +++--
 net/batman-adv/bridge_loop_avoidance.h             |   6 +-
 net/batman-adv/debugfs.c                           |  34 +++--
 net/batman-adv/main.c                              |  34 +++++
 net/batman-adv/main.h                              |   3 +-
 net/batman-adv/packet.h                            |  16 ++-
 net/batman-adv/routing.c                           |  45 ++----
 net/batman-adv/send.c                              |  33 +++++
 net/batman-adv/send.h                              |   3 +
 net/batman-adv/translation-table.c                 | 155 +++++++++++----------
 net/batman-adv/types.h                             |   2 +-
 net/batman-adv/unicast.c                           |   8 +-
 net/batman-adv/vis.c                               |  35 ++---
 16 files changed, 268 insertions(+), 194 deletions(-)
  

Comments

Sven Eckelmann Nov. 21, 2012, 1:51 p.m. UTC | #1
On Wednesday 21 November 2012 13:36:54 David Laight wrote:
> > All packet headers in front of an ethernet header have to be completely
> > divisible by 2 but not by 4 to make the payload after the ethernet header
> > again 4 bytes boundary aligned.
> 
> I'm not sure that statement is correct - whether the patches are
> correct rather depends on the actual packet format(s) you are
> generating/modifying.
> 
> If you are adding data to an ethernet packet you'll need
> to add a multiple of 4 bytes in order maintain the alignment
> of the IP header (which needs to be 4 byte aligned and is
> usually 14 bytes from the start of the receive data).
> 
> This means that the data you add probably has to be 4n+2
> aligned - since I guess you are adding between the source
> MAC address and ethertype?
> If you are inserting data after the ethertype (ie after some
> protocol identifier), then your header can be 4 byte aligned
> provided it ends with the replacement ethertype (0800 or 0806
> for IP protocols).

No, it looks like this

ethernet header | batman-adv stuff | ethernet header | ip header

Kind regards,
	Sven
  
David Miller Nov. 21, 2012, 5:57 p.m. UTC | #2
From: Antonio Quartulli <ordex@autistici.org>
Date: Wed, 21 Nov 2012 13:11:59 +0100

> +#pragma pack(2)
 ...
> -} __packed;

The __packed attribute is an abstraction of the actual syntax
the compiler uses, if it is supported at all.

Therefore, you can't just unconditionally use the #pragma, and
you would need to use some kind of similar compiler abstraction
for it.

But to be honest this is really ugly and for very little, if any,
gain.
  
Sven Eckelmann Nov. 21, 2012, 6:20 p.m. UTC | #3
On Wednesday 21 November 2012 12:57:59 David Miller wrote:
> From: Antonio Quartulli <ordex@autistici.org>
> Date: Wed, 21 Nov 2012 13:11:59 +0100
> 
> > +#pragma pack(2)
> 
>  ...
> 
> > -} __packed;
> 
> The __packed attribute is an abstraction of the actual syntax
> the compiler uses, if it is supported at all.
> 
> Therefore, you can't just unconditionally use the #pragma, and
> you would need to use some kind of similar compiler abstraction
> for it.
> 
> But to be honest this is really ugly and for very little, if any,
> gain.

Agree. But we should get this message also to the other guys

$ git grep pragma -- drivers/net
drivers/net/bonding/bond_3ad.h:#pragma pack(1)
drivers/net/bonding/bond_3ad.h:#pragma pack()
drivers/net/bonding/bond_3ad.h:#pragma pack(8)
drivers/net/bonding/bond_3ad.h:#pragma pack()
drivers/net/bonding/bond_alb.c:#pragma pack(1)
drivers/net/bonding/bond_alb.c:#pragma pack()
drivers/net/ethernet/brocade/bna/bfa_defs.h:#pragma pack(1)
drivers/net/ethernet/brocade/bna/bfa_defs.h:#pragma pack()
drivers/net/ethernet/brocade/bna/bfa_defs_cna.h:#pragma pack(1)
drivers/net/ethernet/brocade/bna/bfa_defs_cna.h:#pragma pack()
drivers/net/ethernet/brocade/bna/bfa_defs_mfg_comm.h:#pragma pack(1)
drivers/net/ethernet/brocade/bna/bfa_defs_mfg_comm.h:#pragma pack()
drivers/net/ethernet/brocade/bna/bfi.h:#pragma pack(1)
drivers/net/ethernet/brocade/bna/bfi.h:#pragma pack()
drivers/net/ethernet/brocade/bna/bfi_cna.h:#pragma pack(1)
drivers/net/ethernet/brocade/bna/bfi_cna.h:#pragma pack()
drivers/net/ethernet/brocade/bna/bfi_enet.h:#pragma pack(1)
drivers/net/ethernet/brocade/bna/bfi_enet.h:#pragma pack()
drivers/net/ethernet/brocade/bna/cna.h:#pragma pack(1)
drivers/net/ethernet/brocade/bna/cna.h:#pragma pack()
drivers/net/ethernet/qlogic/qla3xxx.h:#pragma pack(1)
drivers/net/ethernet/qlogic/qla3xxx.h:#pragma pack()
drivers/net/wan/farsync.c:#pragma pack(1)
drivers/net/wan/farsync.c:#pragma pack()

Kind regards,
	Sven
  
David Miller Nov. 21, 2012, 8:31 p.m. UTC | #4
From: Sven Eckelmann <sven@narfation.org>
Date: Wed, 21 Nov 2012 19:20:21 +0100

> Agree. But we should get this message also to the other guys
> 
> $ git grep pragma -- drivers/net

Good point, I've pulled your tree, thanks!
  
Kevin Curtis Nov. 22, 2012, 8:12 a.m. UTC | #5
>> Agree. But we should get this message also to the other guys

What message would this be?
Are you advocating a different use of #pragma pack()?


Kevin Curtis
Linux Development
FarSite Communications Ltd http://www.farsite.com
Winner of The Queen's Award for Enterprise 2009
tel:  +44 1256 330461
fax:  +44 1256 854931



-----Original Message-----
From: Sven Eckelmann [mailto:sven@narfation.org] 
Sent: 21 November 2012 18:20
To: David Miller
Cc: ordex@autistici.org; netdev@vger.kernel.org; b.a.t.m.a.n@lists.open-mesh.org; lindner_marek@yahoo.de; Kevin Curtis; linux-driver@qlogic.com; ron.mercer@qlogic.com; jitendra.kalsaria@qlogic.com; rmody@brocade.com; andy@greyhouse.net; fubar@us.ibm.com
Subject: Re: Re: [PATCH 9/9] batman-adv: Use packing of 2 for all headers before an ethernet header

On Wednesday 21 November 2012 12:57:59 David Miller wrote:
> From: Antonio Quartulli <ordex@autistici.org>
> Date: Wed, 21 Nov 2012 13:11:59 +0100
> 
> > +#pragma pack(2)
> 
>  ...
> 
> > -} __packed;
> 
> The __packed attribute is an abstraction of the actual syntax the 
> compiler uses, if it is supported at all.
> 
> Therefore, you can't just unconditionally use the #pragma, and you 
> would need to use some kind of similar compiler abstraction for it.
> 
> But to be honest this is really ugly and for very little, if any, 
> gain.

Agree. But we should get this message also to the other guys

$ git grep pragma -- drivers/net
drivers/net/bonding/bond_3ad.h:#pragma pack(1) drivers/net/bonding/bond_3ad.h:#pragma pack() drivers/net/bonding/bond_3ad.h:#pragma pack(8) drivers/net/bonding/bond_3ad.h:#pragma pack() drivers/net/bonding/bond_alb.c:#pragma pack(1) drivers/net/bonding/bond_alb.c:#pragma pack() drivers/net/ethernet/brocade/bna/bfa_defs.h:#pragma pack(1) drivers/net/ethernet/brocade/bna/bfa_defs.h:#pragma pack() drivers/net/ethernet/brocade/bna/bfa_defs_cna.h:#pragma pack(1) drivers/net/ethernet/brocade/bna/bfa_defs_cna.h:#pragma pack() drivers/net/ethernet/brocade/bna/bfa_defs_mfg_comm.h:#pragma pack(1) drivers/net/ethernet/brocade/bna/bfa_defs_mfg_comm.h:#pragma pack() drivers/net/ethernet/brocade/bna/bfi.h:#pragma pack(1) drivers/net/ethernet/brocade/bna/bfi.h:#pragma pack() drivers/net/ethernet/brocade/bna/bfi_cna.h:#pragma pack(1) drivers/net/ethernet/brocade/bna/bfi_cna.h:#pragma pack() drivers/net/ethernet/brocade/bna/bfi_enet.h:#pragma pack(1) drivers/net/ethernet/brocade/bna/bfi_enet.h:#pragma pack() drivers/net/ethernet/brocade/bna/cna.h:#pragma pack(1) drivers/net/ethernet/brocade/bna/cna.h:#pragma pack() drivers/net/ethernet/qlogic/qla3xxx.h:#pragma pack(1) drivers/net/ethernet/qlogic/qla3xxx.h:#pragma pack() drivers/net/wan/farsync.c:#pragma pack(1) drivers/net/wan/farsync.c:#pragma pack()

Kind regards,
	Sven
  
Sven Eckelmann Nov. 22, 2012, 8:28 a.m. UTC | #6
On Thursday 22 November 2012 08:12:38 Kevin Curtis wrote:
> >> Agree. But we should get this message also to the other guys
> 
> What message would this be?
> Are you advocating a different use of #pragma pack()?

David started this discussion and his statement was:

> The __packed attribute is an abstraction of the actual syntax the 
> compiler uses, if it is supported at all.
> 
> Therefore, you can't just unconditionally use the #pragma, and you 
> would need to use some kind of similar compiler abstraction for it.

So yes, it sounds to me like "don't use #pragma but think about some 
abstraction".

Kind regards,
	Sven