Re: [Nsock] Scalable event handling

[Henri Doreau <henri.doreau () gmail com>]

2013/7/25 David Fifield <david () bamsoftware com>:
> Thanks Henri. This looks like a high-quality, worthwhile change as
> always. I tried it and the TCP flood against localhost goes faster as
> you say. gprof shows that epoll_loop stops dominating the profile after
> the change.
David,

thanks a lot for taking the time to test it. I'm glad you could
observe similar results.

> + #if !defined(container_of)
> + #define container_of(ptr, type, member) \
> +         ((type *)((char *)(ptr)-(char *)(&((type *)0)->member)))
> + #endif
>
> Perhaps this should use offsetof? offsetof appears to already be used in
> the code of a few dependencies.
Why not. I personally find this version "nicer" but I'd be fine using
an offet_of-based one.

> +   gh_lnode_t nodeq;
> +   gh_lnode_t nodeq2; /* used for pcap events */
>
> Can you elaborate on how nodeq and nodeq2 work? From what I understand,
> nodeq2 is used for pcap-related events and nodeq is used for other kinds
> of events. Why do these need to be separate? I wonder if there is a more
> orthogonal way to do it. first_ev_next taking a nodeq2 parameter to
> choose which list to work on is a little strange.
That's it nodeq is for everything but pcap and nodeq2 is for pcap
only. The thing is that nsock has this strange "BSD hack mode" where
pcap events are added to both "read" and "pcap_read" event lists. This
requires a second gh_lnode_t handle. Having it always (not only in
BSD_HACK_MODE) used for "pcap_read" events makes the code lighter.

The current gh_list implementation hides it a bit, but It's been there
for a while. I'm not fond of it either I have to say, but I'm planning
to change these linked lists to something else (probably a unique red
black tree, I'm still experimenting and benchmarking at the moment).
Making the BSD hack mode less intrusive will be one of my goals then.

One thing I could do in order to avoid having an extra gh_lnode_t (=
two pointers) per structure when not in BSD_HACK_MODE is to add inline
helpers or play with macros like:

#ifdef BSD_HACK_MODE
  gh_lnode_t nodeq2;
#else
#define nodeq2 nodeq
#endif

or even use an union or a structure, depending on whether
BSD_HACK_MODE is set or not... I find these options hackish,
especially since I want to eventually change the event lists, that's
why I kept nodeq and nodeq2.

> run_tests.sh is failing for me:
>
> started UDP listener on port 55234 (pid 5694)
> started TCP listener on port 55235 (pid 5696)
> started TCP SSL listener on port 55236 (pid 5697)
> nsock pool user data                            [OK]
> test timer operations                           [OK]
> set standard log levels                         [OK]
> check error log levels                          [OK]
> simple tcp connection                           [FAILED] (Connection refused)
hmm... I can't reproduce this. Did you try cleaning & re-building the
test suite after updating nsock? Does netstat -tlnp show the ncat
listeners? You can also see (or send me) the output of "./run_tests.sh
trace" (runs it within strace).

> > Patch generated via git, let me know if you prefer another format or
> > if you want me to open a dedicated branch on nmap-exp.
>
> It looks like a "git show" patch; "git format-patch HEAD^" is better
> because it's easier to apply with "git am".
>
> David Fifield
I see, thanks.

Regards

-- 
Henri
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/