Hi Alex, On ma, 2014-12-01 at 16:01 +0100, Alexander Aring wrote: > Hi Jukka, > > On Mon, Dec 01, 2014 at 02:33:03PM +0200, Jukka Rissanen wrote: > > Hi Alex, > > > > On la, 2014-11-29 at 22:14 +0100, Alexander Aring wrote: > > > 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. > > > > > > Note: > > > > > > If building 6lowpan_udp as module please make sure it's loaded before running > > > rfc6282 udp connection. If it's not loaded we send 6lowpan with raw udp header > > > and next header compression bit isn't set. Nevertheless if you use rfc6282 > > > connection and 6lowpan_udp isn't loaded following will be printed: > > > > > > "ieee802154 wpan-phy0 wpan0: received nhc which is not supported. Dropping." > > > > > > 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. > > > > > > > > > Tested it between two BT 6lowpan connections. If both ends load the > > 6lowpan_udp module, then no issues. If only the other end has loaded the > > module, the other end (receiving end) crashed. > > > > > > [ 132.417733] (NULL net_device): received nhc which is not supported. > > Dropping. > > [ 132.722432] skbuff: skb_under_panic: text:d19cb60c len:55 put:40 > > head:cd923000 data:cd922fec tail:0xcd923023 end:0xcd923740 dev:<NULL> > > [ 133.536256] ------------[ cut here ]------------ > > [ 133.537233] kernel BUG at 3.16+gitAUTOINC > > +f6af675ef5-r215/linux/net/core/skbuff.c:100! > > [ 133.537233] invalid opcode: 0000 [#1] PREEMPT SMP > > [ 133.537233] Modules linked in: bluetooth_6lowpan 6lowpan nfc ecb > > btusb bluetooth rfkill snd_intel8x0 ohci_pci snd_ac97_codec ac97_bus > > parport_pc parport uvesafb > > [ 133.537233] CPU: 0 PID: 327 Comm: kworker/u3:0 Not tainted > > 3.17.0-bt6lowpan #1 > > [ 133.537233] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS > > VirtualBox 12/01/2006 > > [ 133.537233] Workqueue: hci0 hci_rx_work [bluetooth] > > [ 133.537233] task: c0170000 ti: cc7d0000 task.ti: cc7d0000 > > [ 133.537233] EIP: 0060:[<c1779838>] EFLAGS: 00010292 CPU: 0 > > [ 133.537233] EIP is at skb_panic+0x68/0x70 > > [ 133.537233] EAX: 0000007a EBX: cd923000 ECX: 00000000 EDX: 00000001 > > [ 133.537233] ESI: c1b2a11b EDI: 00000004 EBP: cc7d1c18 ESP: cc7d1be8 > > [ 133.537233] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 > > [ 133.537233] CR0: 8005003b CR2: b76d9000 CR3: 0e1e5000 CR4: 000006d0 > > [ 133.537233] Stack: > > [ 133.537233] c1ba8748 c1982e68 d19cb60c 00000037 00000028 cd923000 > > cd922fec cd923023 > > [ 133.537233] cd923740 c1b2a11b cd914300 00000003 cc7d1c24 c1779881 > > c1982e68 cc7d1c94 > > [ 133.537233] d19cb60c c0304c48 00000003 00000008 cc7d1c40 00000003 > > 017d1c64 00000003 > > [ 133.537233] Call Trace: > > [ 133.537233] [<d19cb60c>] ? lowpan_header_decompress+0x26c/0x980 > > [6lowpan] > > [ 133.537233] [<c1779881>] skb_push+0x41/0x50 > > [ 133.537233] [<d19cb60c>] lowpan_header_decompress+0x26c/0x980 > > [6lowpan] > > [ 133.537233] [<d19daeb4>] chan_recv_cb+0x244/0x3f0 > > [bluetooth_6lowpan] > > [ 133.537233] [<d1738711>] l2cap_data_channel+0x8e1/0x980 [bluetooth] > > [ 133.537233] [<c10772d9>] ? sched_clock_local+0x49/0x180 > > [ 133.537233] [<d173aa8f>] l2cap_recv_frame+0xbf/0x2ee0 [bluetooth] > > [ 133.537233] [<c100a7a8>] ? sched_clock+0x8/0x10 > > [ 133.537233] [<c10772d9>] ? sched_clock_local+0x49/0x180 > > [ 133.537233] [<c100a7a8>] ? sched_clock+0x8/0x10 > > [ 133.537233] [<c10772d9>] ? sched_clock_local+0x49/0x180 > > [ 133.537233] [<c107770f>] ? sched_clock_cpu+0x10f/0x160 > > [ 133.537233] [<c10729cb>] ? get_parent_ip+0xb/0x40 > > [ 133.537233] [<c1072a4b>] ? preempt_count_add+0x4b/0xa0 > > [ 133.537233] [<c13e0112>] ? debug_smp_processor_id+0x12/0x20 > > [ 133.537233] [<c108e584>] ? get_lock_stats+0x24/0x40 > > [ 133.537233] [<c1090a34>] ? mark_held_locks+0x64/0x90 > > [ 133.537233] [<c189dbfd>] ? __mutex_unlock_slowpath+0xcd/0x1b0 > > [ 133.537233] [<c13e012f>] ? __this_cpu_preempt_check+0xf/0x20 > > [ 133.537233] [<c1090b54>] ? trace_hardirqs_on_caller+0xf4/0x240 > > [ 133.537233] [<c108e326>] ? trace_hardirqs_off_caller+0xb6/0x160 > > [ 133.537233] [<d173ee19>] l2cap_recv_acldata+0x2f9/0x340 [bluetooth] > > [ 133.537233] [<d1709a93>] ? hci_rx_work+0x113/0x4a0 [bluetooth] > > [ 133.537233] [<d1709ce9>] hci_rx_work+0x369/0x4a0 [bluetooth] > > [ 133.537233] [<d1709a93>] ? hci_rx_work+0x113/0x4a0 [bluetooth] > > [ 133.537233] [<c1063203>] process_one_work+0x1b3/0x4e0 > > [ 133.537233] [<c1063163>] ? process_one_work+0x113/0x4e0 > > [ 133.537233] [<c10638c9>] worker_thread+0x39/0x440 > > [ 133.537233] [<c1063890>] ? init_pwq+0xc0/0xc0 > > [ 133.537233] [<c1067ce8>] kthread+0xa8/0xc0 > > [ 133.537233] [<c1090cab>] ? trace_hardirqs_on+0xb/0x10 > > [ 133.537233] [<c18a1741>] ret_from_kernel_thread+0x21/0x30 > > [ 133.537233] [<c1067c40>] ? kthread_create_on_node+0x160/0x160 > > [ 133.537233] Code: 98 a8 00 00 00 89 54 24 10 89 5c 24 14 8b 40 5c 89 > > 4c 24 08 c7 04 24 48 87 ba c1 89 44 24 0c 8b 45 08 89 44 24 04 e8 b0 db > > 11 00 <0f> 0b 8d b6 00 00 00 00 55 89 e5 83 ec 04 3e 8d 74 26 00 89 c1 > > [ 133.537233] EIP: [<c1779838>] skb_panic+0x68/0x70 SS:ESP > > 0068:cc7d1be8 > > > > Ok I send now a correction for patch PATCH 1/2 for use "-ENOENT" as ret > initialization. Can you test that, please? :-) Sure, I will do that, will send status about that later. > > > > > Just wondering about how it works now. If the device does not have > > 6lowpan_nhc module loaded, then UDP compressed packets are dropped. This > > feels wrong and is a kind of regression to the previous way of working, > > at least we need to auto-load the compression module. > > It works now in this way (example UDP, RAW UDP is a normal 8 byte UDP header): > > If no 6lowpan_nhc_udp is loaded: > > sending side: > > We don't set the NHC bit in 6LoWPAN header, we putting the RAW UDP header. > > receiving side: > > If we get a 6LoWPAN header which set the NHC bit: > We print a warning and drop the packet, because we don't know the NHC ID. > "ieee802154 wpan-phy0 wpan0: received nhc which is not supported. Dropping." > Maybe this can look a little bit better. :-) > > If we get a 6LoWPAN header which does not set the NHC bit and contains > the raw UDP header then this will go into IPv6 layer. > > > > > If 6lowpan_nhc_udp is loaded: > > sending side: > If we get nexthdr == UDP then automatically we do a udp compression > algorithm for RFC6282. > > receiving side: > > We can now accept boths methods, NHC bit is set and UDP compression > (rfc6282) then magic 6lowpan_nhc_udp module function do an > uncompression. > > If NHC bit isn't set we accept also a RAW udp without compression. > > > > > How the system works if there would be another way to compress UDP > > packets. User would need to know somehow what kind of packets it is > > suppose to send/receive and load corresponding module. I wonder if this > > kind of knowledge is really possible in practice. Should we always > > support the NHC defined in https://tools.ietf.org/html/rfc6282 and then > > have a way to load correct compression "plugin" if we get a packet with > > different compression scheme. > > > > If the system does not support https://tools.ietf.org/html/rfc6775 (like > > at the moment) there is no way to figure out what compression system the > > other end is using/supporting. In this respect, it would be nice if the > > current way of working would be automatically supported and user would > > not need to remember to select 6lowpan_nhc option when building the > > kernel. > > > > Yes, we should support automatically and not by user if he loads the > corresponding module. > > For configuration the compression methods I would expect some kind of > netlink interface. On the userspace side we need some kind of tool to > make it configurable. What do you think here? Yes, that sounds good. > > Supporting a new userspace tool will make a high working load and we > need some kind of new 6lowpan repo for releasing such tool. I _could_ try > to implement such netlink interface for nhc into net/6lowpan and greating > a userspace tool. Don't know if we do this effort and making a 6lowpan_udp > userspace tool for configuration, but please make first such framework > mainline and the netlink interface can we add later. :-) > > > btw: which part of rfc6775 describes how to detect the compression > methods on the other end (using/supporting)? My bad, I remembered incorrectly, there was nothing like that mentioned in rfc6775. > > > I like the way to support different compression algorithms, it just > > needs some fine tuning. > > > > Yea, this should mainly _first_ bring any framework for next header > compression mainline. I didn't program such framework in my life and > don't know what 100% we need. For adding new compression methods I think > this framework will grow automatically by history of hacking new > compression methods and I am sure lot of things can be do better than > right now. At the moment it's better than the current behaviour that we > don't have such framework for adding new compression methods. > > - Alex I forgot to mention earlier that I think we need to support the case where some clients are using different compression method than the other (at the same time). What do you think? Cheers, Jukka -- 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
