i'm a little bit confused here. can you please explain why reallink_type is needed here?

since bdaddr, addrtype, page scan and clock offset do not seem to change, perhaps move those inside the block for new entry creation?

since link_type is known here (as it comes from the message) would it make more sense to convert if-else into switch(link_type) { } here?

perhaps better name, i.e. sockaddr_l2cap_le?

is adding idtype filed here backward compatible with older Bluetooth versions?

no break; here? is this intended? as it is right now, the code will panic

remove printf() ?

'send_data_packets' function is invoked when either ACL link or SCO link is available and the argument is determined by the available link.
Here, I have to compare link value with real direction.

No objection.

No objection.

Should I keep this compatibility shim permanently?

I don't consider in-kernel compatibility.

For now, I don't support device side. And this is device side request.
"
4.20 CONNECTION PARAMETER UPDATE REQUEST (CODE
0X12)
This command shall only be sent from the LE slave device to the LE master
device."

Yes.... I forget to remove it.

i don't think so.

i wonder if you can use l2cap_len here. basically, cast everything to sockaddr_l2cap (old version) to begin with. then check l2cap_len and compare it to sizeof(sockaddr_l2cap_new). if size matches, re-cast to sockaddr_l2cap_new.

i think it should be possible to rename sockaddr_l2cap to sockaddr_l2cap_old and have new fields in sockaddr_l2cap by default.

this way, applications complied with old sockaddr_l2cap will use sockaddr_l2cap_old and still work.

so, i think, it would be better to add new fields to sockaddr_l2cap and have the rest of the code use it. compatibility shim probably should only exists in l2cap socket layer.

struct name and type name are the same?

not supporting features is ok. what i was trying to confirm is that panic() in this case is intentional. if its not, then perhaps adding a break; here (with an optional error message) might be a good idea. again, this is to protect against the case where, somehow, this command ends up on the l2cap connection queue.

can you please explain why this is needed? is there no indication for received responses here?

can you please explain why ng_l2cap_node_hook_info_ep is needed? as far as i can see, ng_l2cap_node_hook_info_ep has only one member, i.e. bdaddr_t

i'm confused here. this is request path, but _ng_l2cap_con_rsp (response) is used. can you please explain if this something from the spec ?

since ng_l2cap_node_hook_info_ep was added, would it not be more correct to check for sizeof(ng_l2cap_node_hook_info_ep) instead of sizeof(bdaddr_t)?

It is intended, for now. At least it may have to be warned, I think.

It simulates L2CAP connection complete when HCI connection established.
When it established, this routine is called and start processing packet.
Then, the flagged Connection *Response* packet is reflected to receive queue and L2CAP layer tranfer state machine to another state.

Need to add Address type later, Especially required to support random address.

See above.

Yes.

Renaming our new structure to sockaddr_l2cap would be good.
But we should note it breaks source compatibility, because uninitialized member may be passed to kernel.

i'm sorry, but i do not follow. there is l2cap_len field. any binary application would (ot at least should) have it set to size of the old sockaddr_l2cap. what i'm suggesting is to check l2cap_len first and cast pointer appropriately. we can rename structure safely then.

agreed. warning is absolutely fine. panic() is a bit too drastic for me.

i'm sorry, but i'm completely lost here. ng_l2ap_lp_deliver() sends data to the HCI layer. ng_l2cap_lp_receive() receives data from the HCI layer. so, if i'm reading this right, the code tries to loopback some special packet marked with M_PROTO2 flag. instead of doing that, would it be possible to adjust state in upper layer and not send effectively dummy command down to l2cap layer?

yes. i'm still confused, sorry. is there no other way? is it no possible to adjust state here?

If a programmer use new structure as old structure,
struct size is changed as new structure(because he use sizeof(struct sockaddr_l2cap) to set the value), but s/he may still not initialize some member of new sockaddr_l2cap structure. So source compatibility may be broken.

I want to know when the packet processing start here, so state transition at caller is not desirable. And I used mbuf flag because I don't want to peek packet content here.

as we have discussed, please move those two inside block where new entry is created

please add some dummy return here with a comment that says it should never get here

please either make new structure default, or, please move new fields into l2cap socket options. i think using l2cap_len works for binary backwards compatibility. source compatibility, where application fails to bzero(sockadd_l2cap) is a problem, but it will have to be addressed in the application.

i personally think that having socket option for Bluetooth LE is acceptable, and, in some ways cleaner. however, for sake of better linux compatibility i'm fine with adding new fields to sockaddr_l2cap.

please just remove it

please do not use ng_l2cap_node_hook_info_ep as both struct name and type name.

can you please explain why loopback it here? is it not possible to loopback command at the socket layer?

please remove #if 0 ... #endif block

i think this should read sizeof(*ep) instead of sizeof(bdaddr_t)

this looks good. if you choose to keep new fields in sockaddr_l2cap please rename sockaddr_l2cap to sockaddr_l2cap_old, and have new fields added to sockaddr_l2cap by default. if you choose to go with socket option, then, none of this is needed.

Address and address type should be in the block. OK.
other than this, which is now no effect, should be here, because rssi
will be updated properly

ok.

Ok, I will name old structure sockaddr_l2cap_compat and new one sockaddr_l2cap_compat.

Also warn structure change by preprossor directives.

Socket option was used in Linux. but it has been changed in some reason.

Ok.

Ok. Just define typedef name.

To wait HCI connection event. Otherwise, it may possible to add outstanding ATT channel pointer to connection structure and change channel states if the channel pointer is valid.

Ok

Ok

Ok. I'll rename sockaddr_l2cap and change sockname, peername bind call structure also.

can you please remove #if 0 block if this code is not needed?

can you please make sure that this is not going to break the buildworld?