Date: Tue, 16 Apr 2002 14:26:10 -0700 From: Maksim Yevmenkin <myevmenk@digisle.net> To: Julian Elischer <julian@elischer.org> Cc: freebsd-current@FreeBSD.ORG, freebsd-net@FreeBSD.ORG Subject: Re: Bluetooth stack for FreeBSD Message-ID: <3CBC96F2.3032077A@digisle.net> References: <Pine.BSF.4.21.0204151548160.88655-100000@InterJet.elischer.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Julian,
thanks for the comments, as always i found them very useful :)
i have combined both e-mail into one and included my answers
inline.
> ng_btsocket.c: unmodified: line 674
> sbappendrecord(&pcb->so->so_snd, m);
> m = m_dup(m, M_TRYWAIT);
> if (m == NULL) {
> error = ENOBUFS;
> goto drop1;
> }
>
> you are m_dup'ing an mbuf you have given away to the socket layer.
> on an SMP system this may already be destroyed by the time
> you do the m_dup() in fact if the sbappendrecord() fails
> due to a full queue, it may already be invalid...
fixed. thanks.
> also all functions should be 'prototype format'
> e.g.
> static int
> ng_btsocket_peeraddr(so, nam)
> struct socket *so;
> struct sockaddr **nam;
> {
> should be:
> static int
> ng_btsocket_peeraddr(struct socket *so, struct sockaddr **nam)
> {
>
> also:
> __P() is now "deprecated" and should not be used in new code.
i should read style(9) more often. i fixed my code.
> Your idea of making a special bluetooth socket type, implemented by
> a Netgraph node is interesting. Maybe we should it easier to do this
> by extending the netgraph socket type with the ability to
> make sockets of arbitrary types... i.e. register a node type that acts
> as a 'subtype' of the 'socket' type, and inherrits generic
> socket behaviour, but allow the 'child type' to specify
> other parts to replace normal behaviour.. I guess we would need to see a
> few more cases of this done before we could deduce what is
> likely to be a good candidate for abstraction.
i must admit that current socket implementation is a mess :(
my original plan was to use Netgraph sockets only and then
write several user space libraries to perform actual connection.
of course that idea had its own disadvantages. so i wrote
sockets layer. i will try to clean up it a little bit (remove
mutexes etc.) and also there probably should be support for HCI
and raw L2CAP sockets as well.
> you ask:
> /*
> * XXX FIXME/VERIFY i assume here that "hook" is a pointer
> * to the local hook we have received message from. For
> * L2CAP messages "hook" is required.
> */
>
> This is true for 5.0
> in 4.x there is no such thing as an arrival hook for a message.
> You should however not assume that it is non-NULL. test it in
> normal running code.. not in a KASSERT.
fixed.
> ok, read a bit more:
>
> [....]
>
> Any node that changes it's internal state with reception of DATA
> (as opposed to control messages) should ensure that it by doing:
> NG_NODE_FORCE_WRITER(node);
>
> This is because in the default state, multiple data messages may
> (under SMP) be transitting the node at a time, as they
> only take out READER locks. If DATA can change some state variable or
> similar then this becomes unsafe, and they should be serialised.
> If only SOME data can do this, you have the option to takew reader-locks
> and only after confirming that a message is a writer, upgrade to a writer
> lock by 'requeueing' it as such. Alternatively teh sender can mark a
> packet as 'writer'.
initially all nodes were WRITERs to "release the stack", but then i
changed my strategy and now i'm serializing data at the edge of graph.
for example both "bt3c" and "h4" nodes will NG_HOOK_FORCE_WRITER on
upstream hook. also i probably should should turn WRITER bit on "ctl"
hook for HCI node since it accepts data. L2CAP node accepts whole
L2CAP packet from upstream hook, so (i think) it should be fine unless
these packets gets re-ordered somehow. protocols that run over L2CAP
may not like it.
is it possible that SMP Netgraph can re-order data? if so then sockets
node probably should turn WRITER bit on downstream hooks too.
> NG_NODE_PRIVATE(NG_HOOK_NODE(hook))
> can be saved if you store information relavant to a hook in it's own
> private data pointer..
> In some nodes I store the same data in NG_HOOK_PRIVATE(hook)
> as is in NG_NODE_PRIVATE(node), just to save the dereference
> if it's going to be done per packet. Sometimes there are better things to
> store there however..
i'm sorry, but i'm not sure what do you mean here. i use such calls
in several places (connect, disconnect, rcvdata) to get to the node
private structure, but (i think) it just a macros that expanded to a
couple pointer accesses.
> /* Detach mbuf, discard item and process data */
> NGI_GET_M(item, m);
> NG_FREE_ITEM(item);
>
> If there is any chance that you will need a new 'item' in a function or a
> child function, then it's worth keeping them around to save teh expense of
> re-allocating them..
>
> I guess I shuld make a macro NG_FWD_DATA_HOOK() to make this easier to
> do..
you right. i should not destroy item and use NG_FWD_ITEM_HOOK where
required.
again thank you for your comments, i'm looking forward to hear more :)
thanks,
max
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3CBC96F2.3032077A>