Hi Daniel,
On Sat, Sep 10, 2011 at 05:14:58PM +0200, Daniel Willmann wrote:
Hello Pablo,
sorry, but this is going to take me longer than I previously thought.
No problem, we're not in the rush :-).
Just some short remarks on what I noticed so far.
After Tuesday I'll
have more time on my hands to help with porting.
On Fri, 9 Sep 2011 03:05:26 +0200
pablo(a)gnumonks.org wrote:
From: Pablo Neira Ayuso
<pablo(a)gnumonks.org>
These patchset contains the port of libctrl to use the IPA
infrastructure available in libosmo-abis.
I've also made one small cleanup to bail out
in case that
we cannot bind to the telnet port. Another patch follows up
to avoid disabling nagle (to avoid problems with TCP
segmentation, we still have to support this appropriately),
I noticed this while doing the port work.
First of all thanks for starting the port. I had a brief look into that
a week ago, but ran into different problems.
Pablo Neira Ayuso (3):
ctrl: use generic IPA socket infrastructure available in
libosmo-abis
The control commands need to be sent both over the dedicated control
connection as well as between osmo-bsc and nat. It seems that the
bsc_msc_connection is still using osmo_wqueues which complicates
matters as I originally just used ctrl_cmd_send to send the response to
whatever osmo_wqueue the command came in from or forward commands from
the nat to the appropriate osmo-bsc.
Can you come with some idea to overcome this situation? Probably we
can use osmo_wqueues in the generic IPA infrastructure, let me give it
some spins.
bsc_nat also needs to know if a a control connection
is closed so it
can remove any commands pending response from the pending list. The
list is needed to generate errors if the msc_bsc connection is reset or
a timeout occurs while a command is in transit. Could you add a
callback to ipa_server_conn that gets called if the connection is
closed?
Sure, we can add that. The idea is to make the IPA infrastructure
generic enough so we can use in as many places as we can.
src: check
for error returned by controlif_setup()
That's certainly useful, thanks.
ctrl: do not disable nagle algorithm until we
fully support
segmentation
I had a look at commit 4462f8 and I believe I just tried to mimic the
behavior of msc_connection_connected() from ./osmo-bsc/osmo_bsc_msc.c
at that time.
Anyway, I'd be interested in the problems you encountered. I know that
ipaccess_read_msg mishandled the case where the data read returned only
a partial packet, but shouldn't that only occur if the data to be sent
is larger than the current MSS? In any case I would have expected
disabling the nagle algorithm to help in that regard...
I think disabling nagle in that code is fine. Basically, if we
disable nagle we obtain the same behaviour that in UDP sockets, which
means that one send() call means one packet, and this is what we have
to do by now to avoid TCP segmentation issues (that we don't handle
appropriately). Drop the patch.
So I guess there's still some work to do. I'll
reserve some time on
Thursday to look at the issues.
Fine, count to me if I can be of any help. I'll have a look at the
issues that you mentions along this week.