Re: [PATCHv5 bluetooth-next 0/3] 6lowpan: introduce nhc framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Alex,

> This patch series introduce the next header compression framework. Currently
> we support udp compression/uncompression only. This framework allow to add new
> next header compression formats easily.
> 
> If somebody wants to add a new header compression format and some information
> are missing while calling compression and uncompression callbacks. Please
> feel free to make framework changes according these callbacks.
> 
> changes since v2:
> - make udp nhc as module as suggested by Marcel Holtmann
> - fix comment header in nhc_udp.c
> 
> I didn't make the lowpan_nhc declaration "const" because this will occur
> issues with rb_node, id and idmask array. Which will manipulated during
> runtime.
> 
> changes since v3:
> - add patch 3/3 for other known rfc6282 ipv6 extension headers compression
>   formats
> - add request_modules for loading nhc default compression format modules.
>   Which was suggested by Jukka Rissanen. Thanks, this is really working.
> - Add rtnl_lock for lowpan_nhc_add and del since we have no synced
>   request_modules call this could make trouble.
> - Move some handling out of nhc_do_compression and uncompression function.
>   The complete handling is now inside of iphc.c and nhc_do_compression and
>   uncompression functions is only a wrapper call for the callback.
> - rework some menuentries for Kconfig and compression format, they are
>   grouped by rfc now.
> - move some generic handling like "skb_pull(skb, nhc->nexthdrlen);" into
>   iphc.c. It would be great if we have something also for uncompression
>   for the skb_cow. But this isn't possible right now.
> - change warning if nhc was not found to "was not found" instead isn't
>   implemented. It isn't implemented if callbacks are NULL now.
> - small cleanups.
> 
> changes since v4:
> - add spinlock for to prevent nhc from deletion while using. This can occur
>   if nhc module is unloading while compression/uncompression.
> - move nhc handling for nhc compression/uncompression completely into
>   nhc_do_compression/nhc_do_uncompression.
> 
> Note about locking:
> 
> We have now a spinlock nhc_lowpan_lock which prevents manipulating the nhc
> datastructures while using it. On receiving side it's easy to handle, at the
> end we check if 6lowpan nh compressed flag is set and run uncompression.
> On the other hand the transmit side is complicated, we check if next_hdr field
> is registrated and run some other compression routines, at least we do the
> finally nhc compression which depends on the first check if next_hdr was
> registrated. The kernel will crash if we remove the using module between
> "next_hdr check" and "do nhc compression", the lock will prevent that now.
> 
> Now in the transmit path exist a little window to remove a lowpan_nhc while
> compression. This is exactly the part between "check if we support" and
> "doing compression". This is a very unlikely case, if this occurs we drop
> the packet while compression. Don't know how to better deal with that right
> now. I will try to search a better solution later.
> 
> changes since v5:
> - s/rfc6282// on filenames and kconfig entries, suggested by Marcel Holtmann
>   and Stefan Schmidt
> - s/FRAG/FRAGMENT/ - suggested by Stefan Schmidt
> - s/MOBIL/MOBILITY/ - suggested by Stefan Schmidt
> - s/ROUTE/ROUTING/ - suggested by Stefan Schmidt
> - replace "depends on 6LOWPAN_NHC" with a global "if 6LOWPAN_NHC
>   $LOT_OF_ENTRIES endif" inside net/6lowpan/Kconfig
> - fix small typo "nhc" -> "nhcs" in net/6lowpan/Makefile
> - add MODULE_DESCRIPTION to each NHC module
> 
> Cc: Jukka Rissanen <jukka.rissanen@xxxxxxxxxxxxxxx>
> Cc: Martin Townsend <mtownsend1973@xxxxxxxxx>
> Cc: Marcel Holtmann <marcel@xxxxxxxxxxxx>
> Cc: Stefan Schmidt <s.schmidt@xxxxxxxxxxx>
> 
> Alexander Aring (3):
>  6lowpan: add generic nhc layer interface
>  6lowpan: add udp compression via nhc layer
>  6lowpan: nhc: add other known rfc6282 compressions
> 
> net/6lowpan/Kconfig        |  57 ++++++++++-
> net/6lowpan/Makefile       |  13 ++-
> net/6lowpan/iphc.c         | 200 ++++++-------------------------------
> net/6lowpan/nhc.c          | 241 +++++++++++++++++++++++++++++++++++++++++++++
> net/6lowpan/nhc.h          | 146 +++++++++++++++++++++++++++
> net/6lowpan/nhc_dest.c     |  28 ++++++
> net/6lowpan/nhc_fragment.c |  27 +++++
> net/6lowpan/nhc_hop.c      |  27 +++++
> net/6lowpan/nhc_ipv6.c     |  27 +++++
> net/6lowpan/nhc_mobility.c |  27 +++++
> net/6lowpan/nhc_routing.c  |  27 +++++
> net/6lowpan/nhc_udp.c      | 157 +++++++++++++++++++++++++++++
> 12 files changed, 806 insertions(+), 171 deletions(-)
> create mode 100644 net/6lowpan/nhc.c
> create mode 100644 net/6lowpan/nhc.h
> create mode 100644 net/6lowpan/nhc_dest.c
> create mode 100644 net/6lowpan/nhc_fragment.c
> create mode 100644 net/6lowpan/nhc_hop.c
> create mode 100644 net/6lowpan/nhc_ipv6.c
> create mode 100644 net/6lowpan/nhc_mobility.c
> create mode 100644 net/6lowpan/nhc_routing.c
> create mode 100644 net/6lowpan/nhc_udp.c

so I collected the reviewed-by and acked-by statements, but when I did a simple compile test it failed badly.

  CC      net/6lowpan/nhc.o
net/6lowpan/nhc.c: In function ‘lowpan_nhc_add’:
net/6lowpan/nhc.c:206:2: warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘size_t’ [-Wformat=]
  WARN_ONCE(nhc->idlen > LOWPAN_NHC_MAX_ID_LEN,
  ^

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux