Thread: [RTnet-developers] Maintaining UDP sockets in a hash instead of a list.
Brought to you by:
bet-frogger,
kiszka
Menu
▾
▴
rtnet-developers
|
From: Gilles C. <gil...@xe...> - 2008-06-25 13:32:34
Attachments:
rtnet-udp-hash.diff
|
Hi,
here is another patch to RTnet, which replaces the list of UDP sockets
with a hash, so makes the searching for an RTnet registered socket a bit
faster.
Regards.
--
Gilles.
|
|
From: Gilles C. <gil...@xe...> - 2008-06-25 13:41:27
|
Gilles Chanteperdrix wrote:
> Hi,
>
> here is another patch to RTnet, which replaces the list of UDP sockets
> with a hash, so makes the searching for an RTnet registered socket a bit
> faster.
Ok. Should have read the patch better before sending it. There are two
more features poured into this patch:
- we add a "receiving" member to the UDP socket structure, which is only
sets either if the program using the socket has called recv, or if the
program has bound the socket to a particular port, this is to prevent a
problem which happens when putting an RTnet box on an heterogenous
network, with an application which only uses a socket to send packets:
if the network sends some packets to the automatically assigned port,
they use rtskbs from the socket pool, until the socket pool becomes
empty, and no more packet can be sent using the socket.
- when a socket is bound to a particular IP address, we use this IP as
the source IP of the packets sent by this socket, even if they end-up
being sent through another interface, this is Linux behaviour, and was
required for one of our applications to work correctly with
off-the-shelf IP phones (they do filtering based on the source IP).
--
Gilles.
|
|
From: Jan K. <jan...@we...> - 2008-06-27 22:00:04
Attachments:
signature.asc
|
Gilles Chanteperdrix wrote: > Gilles Chanteperdrix wrote: >> Hi, >> >> here is another patch to RTnet, which replaces the list of UDP sockets >> with a hash, so makes the searching for an RTnet registered socket a bit >> faster. > > Ok. Should have read the patch better before sending it. There are two > more features poured into this patch: > - we add a "receiving" member to the UDP socket structure, which is only > sets either if the program using the socket has called recv, or if the > program has bound the socket to a particular port, this is to prevent a > problem which happens when putting an RTnet box on an heterogenous > network, with an application which only uses a socket to send packets: > if the network sends some packets to the automatically assigned port, > they use rtskbs from the socket pool, until the socket pool becomes > empty, and no more packet can be sent using the socket. > - when a socket is bound to a particular IP address, we use this IP as > the source IP of the packets sent by this socket, even if they end-up > being sent through another interface, this is Linux behaviour, and was > required for one of our applications to work correctly with > off-the-shelf IP phones (they do filtering based on the source IP). Could you break those two changes out and post them separately? Such changes are too critical to have them all-in-one. TiA, Jan |
|
From: Gilles C. <gil...@xe...> - 2008-06-30 09:58:46
|
Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Gilles Chanteperdrix wrote:
>>> Hi,
>>>
>>> here is another patch to RTnet, which replaces the list of UDP sockets
>>> with a hash, so makes the searching for an RTnet registered socket a bit
>>> faster.
>> Ok. Should have read the patch better before sending it. There are two
>> more features poured into this patch:
>> - we add a "receiving" member to the UDP socket structure, which is only
>> sets either if the program using the socket has called recv, or if the
>> program has bound the socket to a particular port, this is to prevent a
>> problem which happens when putting an RTnet box on an heterogenous
>> network, with an application which only uses a socket to send packets:
>> if the network sends some packets to the automatically assigned port,
>> they use rtskbs from the socket pool, until the socket pool becomes
>> empty, and no more packet can be sent using the socket.
>> - when a socket is bound to a particular IP address, we use this IP as
>> the source IP of the packets sent by this socket, even if they end-up
>> being sent through another interface, this is Linux behaviour, and was
>> required for one of our applications to work correctly with
>> off-the-shelf IP phones (they do filtering based on the source IP).
>
> Could you break those two changes out and post them separately? Such
> changes are too critical to have them all-in-one.
Ok. Here it comes, extracted from revision control history.
Apply in order:
- rtnet-fix-src-ip.diff
- rtnet-check-receiving.diff
- rtnet-hash-port.diff
--
Gilles.
|
|
From: Gilles C. <gil...@xe...> - 2008-07-05 17:19:33
|
Jan Kiszka wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> - rtnet-check-receiving.diff
>>> @@ -375,13 +379,23 @@ ssize_t rt_udp_recvmsg(struct rtdm_dev_c
>>> struct udphdr *uh;
>>> struct sockaddr_in *sin;
>>> nanosecs_rel_t timeout = sock->timeout;
>>> - int ret;
>>> + int ret, index;
>>> + rtdm_lockctx_t context;
>>>
>>>
>>> /* non-blocking receive? */
>>> if (testbits(msg_flags, MSG_DONTWAIT))
>>> timeout = -1;
>>>
>>> + rtdm_lock_get_irqsave(&udp_socket_base_lock, context);
>>> + if ((index = sock->prot.inet.reg_index) < 0) {
>>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>>> + /* socket is being closed */
>>> + return -EBADF;
>>> + }
>>> + port_registry[index].receiving = 1;
>>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>>> +
>> This is the only part of the patch I don't like. I don't want to add
>> another lock site to the RX path. I see the problem, but I think we need
>> some other solution. Maybe something that disables reception, and thus
>> unwanted buffer consumption.
>
> Well, I was too quick. There is another thing I don't like about this
> patch: It breaks valid use cases.
>
> Consider some application creating a socket first, then sending some
> message, next doing something else (or being preempted for a while), and
> finally calling into the recvmsg functions. With your change a potential
> reply to that sent-out message could have been dropped in the meantime
> because the socket was not yet marked as 'receiving'. Not good.
It seems I was too quick to admit your use case: would such an
application do this without binding the socket to another port than the
automacically assigned port ? Because once a socket is bound to a port
no received packet is lost.
--
Gilles.
|
|
From: Jan K. <jan...@we...> - 2008-07-05 17:32:53
Attachments:
signature.asc
|
Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> - rtnet-check-receiving.diff
>>>> @@ -375,13 +379,23 @@ ssize_t rt_udp_recvmsg(struct rtdm_dev_c
>>>> struct udphdr *uh;
>>>> struct sockaddr_in *sin;
>>>> nanosecs_rel_t timeout = sock->timeout;
>>>> - int ret;
>>>> + int ret, index;
>>>> + rtdm_lockctx_t context;
>>>>
>>>>
>>>> /* non-blocking receive? */
>>>> if (testbits(msg_flags, MSG_DONTWAIT))
>>>> timeout = -1;
>>>>
>>>> + rtdm_lock_get_irqsave(&udp_socket_base_lock, context);
>>>> + if ((index = sock->prot.inet.reg_index) < 0) {
>>>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>>>> + /* socket is being closed */
>>>> + return -EBADF;
>>>> + }
>>>> + port_registry[index].receiving = 1;
>>>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>>>> +
>>> This is the only part of the patch I don't like. I don't want to add
>>> another lock site to the RX path. I see the problem, but I think we need
>>> some other solution. Maybe something that disables reception, and thus
>>> unwanted buffer consumption.
>> Well, I was too quick. There is another thing I don't like about this
>> patch: It breaks valid use cases.
>>
>> Consider some application creating a socket first, then sending some
>> message, next doing something else (or being preempted for a while), and
>> finally calling into the recvmsg functions. With your change a potential
>> reply to that sent-out message could have been dropped in the meantime
>> because the socket was not yet marked as 'receiving'. Not good.
>
> It seems I was too quick to admit your use case: would such an
> application do this without binding the socket to another port than the
> automacically assigned port ? Because once a socket is bound to a port
> no received packet is lost.
Well, not binding to a specific port is the standard pattern for client
applications. And clients start their work with sending some packet to a
server. So this use case is surely not unusual.
Jan
|
|
From: Gilles C. <gil...@xe...> - 2008-07-05 17:39:41
|
Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> - rtnet-check-receiving.diff
>>>>> @@ -375,13 +379,23 @@ ssize_t rt_udp_recvmsg(struct rtdm_dev_c
>>>>> struct udphdr *uh;
>>>>> struct sockaddr_in *sin;
>>>>> nanosecs_rel_t timeout = sock->timeout;
>>>>> - int ret;
>>>>> + int ret, index;
>>>>> + rtdm_lockctx_t context;
>>>>>
>>>>>
>>>>> /* non-blocking receive? */
>>>>> if (testbits(msg_flags, MSG_DONTWAIT))
>>>>> timeout = -1;
>>>>>
>>>>> + rtdm_lock_get_irqsave(&udp_socket_base_lock, context);
>>>>> + if ((index = sock->prot.inet.reg_index) < 0) {
>>>>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>>>>> + /* socket is being closed */
>>>>> + return -EBADF;
>>>>> + }
>>>>> + port_registry[index].receiving = 1;
>>>>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>>>>> +
>>>> This is the only part of the patch I don't like. I don't want to add
>>>> another lock site to the RX path. I see the problem, but I think we need
>>>> some other solution. Maybe something that disables reception, and thus
>>>> unwanted buffer consumption.
>>> Well, I was too quick. There is another thing I don't like about this
>>> patch: It breaks valid use cases.
>>>
>>> Consider some application creating a socket first, then sending some
>>> message, next doing something else (or being preempted for a while), and
>>> finally calling into the recvmsg functions. With your change a potential
>>> reply to that sent-out message could have been dropped in the meantime
>>> because the socket was not yet marked as 'receiving'. Not good.
>> It seems I was too quick to admit your use case: would such an
>> application do this without binding the socket to another port than the
>> automacically assigned port ? Because once a socket is bound to a port
>> no received packet is lost.
>
> Well, not binding to a specific port is the standard pattern for client
> applications. And clients start their work with sending some packet to a
> server. So this use case is surely not unusual.
I would expect client applications to try to bind their socket to a
given port range, not to rely on the UDP stack to choose one in an
unknown range. This way, you get things working behind a firewall...
--
Gilles.
|
|
From: Jan K. <jan...@we...> - 2008-07-05 17:51:40
Attachments:
signature.asc
|
Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Jan Kiszka wrote:
>>>>> Gilles Chanteperdrix wrote:
>>>>>> - rtnet-check-receiving.diff
>>>>>> @@ -375,13 +379,23 @@ ssize_t rt_udp_recvmsg(struct rtdm_dev_c
>>>>>> struct udphdr *uh;
>>>>>> struct sockaddr_in *sin;
>>>>>> nanosecs_rel_t timeout = sock->timeout;
>>>>>> - int ret;
>>>>>> + int ret, index;
>>>>>> + rtdm_lockctx_t context;
>>>>>>
>>>>>>
>>>>>> /* non-blocking receive? */
>>>>>> if (testbits(msg_flags, MSG_DONTWAIT))
>>>>>> timeout = -1;
>>>>>>
>>>>>> + rtdm_lock_get_irqsave(&udp_socket_base_lock, context);
>>>>>> + if ((index = sock->prot.inet.reg_index) < 0) {
>>>>>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>>>>>> + /* socket is being closed */
>>>>>> + return -EBADF;
>>>>>> + }
>>>>>> + port_registry[index].receiving = 1;
>>>>>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>>>>>> +
>>>>> This is the only part of the patch I don't like. I don't want to add
>>>>> another lock site to the RX path. I see the problem, but I think we need
>>>>> some other solution. Maybe something that disables reception, and thus
>>>>> unwanted buffer consumption.
>>>> Well, I was too quick. There is another thing I don't like about this
>>>> patch: It breaks valid use cases.
>>>>
>>>> Consider some application creating a socket first, then sending some
>>>> message, next doing something else (or being preempted for a while), and
>>>> finally calling into the recvmsg functions. With your change a potential
>>>> reply to that sent-out message could have been dropped in the meantime
>>>> because the socket was not yet marked as 'receiving'. Not good.
>>> It seems I was too quick to admit your use case: would such an
>>> application do this without binding the socket to another port than the
>>> automacically assigned port ? Because once a socket is bound to a port
>>> no received packet is lost.
>> Well, not binding to a specific port is the standard pattern for client
>> applications. And clients start their work with sending some packet to a
>> server. So this use case is surely not unusual.
>
> I would expect client applications to try to bind their socket to a
> given port range, not to rely on the UDP stack to choose one in an
> unknown range. This way, you get things working behind a firewall...
First, there are /typically/ no firewalls to cross for RTnet
applications. And second even if there are, hacking applications to work
around some firewall shortcomings might be required from time to time
but that must not be considered as a common application pattern.
My point remains: We cannot block reception until the application calls
recvmsg, we must allow it from the beginning. Blocking it on request is
acceptable, though.
Jan
|
|
From: Gilles C. <gil...@xe...> - 2008-07-07 09:20:52
|
Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Gilles Chanteperdrix wrote:
>>>>>>> - rtnet-check-receiving.diff
>>>>>>> @@ -375,13 +379,23 @@ ssize_t rt_udp_recvmsg(struct rtdm_dev_c
>>>>>>> struct udphdr *uh;
>>>>>>> struct sockaddr_in *sin;
>>>>>>> nanosecs_rel_t timeout = sock->timeout;
>>>>>>> - int ret;
>>>>>>> + int ret, index;
>>>>>>> + rtdm_lockctx_t context;
>>>>>>>
>>>>>>>
>>>>>>> /* non-blocking receive? */
>>>>>>> if (testbits(msg_flags, MSG_DONTWAIT))
>>>>>>> timeout = -1;
>>>>>>>
>>>>>>> + rtdm_lock_get_irqsave(&udp_socket_base_lock, context);
>>>>>>> + if ((index = sock->prot.inet.reg_index) < 0) {
>>>>>>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>>>>>>> + /* socket is being closed */
>>>>>>> + return -EBADF;
>>>>>>> + }
>>>>>>> + port_registry[index].receiving = 1;
>>>>>>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>>>>>>> +
>>>>>> This is the only part of the patch I don't like. I don't want to add
>>>>>> another lock site to the RX path. I see the problem, but I think we need
>>>>>> some other solution. Maybe something that disables reception, and thus
>>>>>> unwanted buffer consumption.
>>>>> Well, I was too quick. There is another thing I don't like about this
>>>>> patch: It breaks valid use cases.
>>>>>
>>>>> Consider some application creating a socket first, then sending some
>>>>> message, next doing something else (or being preempted for a while), and
>>>>> finally calling into the recvmsg functions. With your change a potential
>>>>> reply to that sent-out message could have been dropped in the meantime
>>>>> because the socket was not yet marked as 'receiving'. Not good.
>>>> It seems I was too quick to admit your use case: would such an
>>>> application do this without binding the socket to another port than the
>>>> automacically assigned port ? Because once a socket is bound to a port
>>>> no received packet is lost.
>>> Well, not binding to a specific port is the standard pattern for client
>>> applications. And clients start their work with sending some packet to a
>>> server. So this use case is surely not unusual.
>> I would expect client applications to try to bind their socket to a
>> given port range, not to rely on the UDP stack to choose one in an
>> unknown range. This way, you get things working behind a firewall...
>
> First, there are /typically/ no firewalls to cross for RTnet
> applications. And second even if there are, hacking applications to work
> around some firewall shortcomings might be required from time to time
> but that must not be considered as a common application pattern.
>
> My point remains: We cannot block reception until the application calls
> recvmsg, we must allow it from the beginning. Blocking it on request is
> acceptable, though.
Ok. I had a look at Linux "shutdown" call, it seems to accept UDP
sockets, but is not used by anything in the UDP code except poll. The
only Linux call that we could use to avoid queuing received packets is
SO_RCVBUF. Of course, implementing rcvbuf for RTnet makes little sense,
because the messages pool has a finite number of messages, and every
message consumes a fixed amount of memory, so even with an rcvbuf limit,
if we received only small messages, we could exhaust the pool, the only
case that would make sense is rcvbuf set to 0.
--
Gilles.
|
|
From: Gilles C. <gil...@xe...> - 2008-08-28 15:33:22
|
Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Hi,
>>
>> here is a repost of the rtnet-hash-port.diff patch. Only, without the
>> "receiving" bit, and with the INADDR_ANY bit fixed.
>
> OK, almost ready for merge.
>
> BTW, did you also happen to measure the performance gain (and for how
> many sockets)?
I have just done a quick test: with 200 sockets, the worst case (i.e.
the last socket) takes 30us to search the sockets table. Due to the
linear search, the average case is half the worst case.
200 sockets is what happens when 50 phones trying to phone outside at
the same time, each conversation results in 4 sockets being opened by
the NAT helper for each conversation (1 RTP outbound, 1 RTCP inbound, 1
RTP inbound, 1 RTP). An RTP stream takes 50 packets by second. an RTCP
stream 0.2 packet by second. So, the total is 100 * 50 + 100 * 0.2 =
5020 packets by second, and the overhead of the socket search is 5020 *
15 us = 75300 us, i.e. 75 ms, i.e. 7.5 % CPU.
Now, when using the hash table, the worst case, which is the same as the
average case is 2 us, that is precisely 7.5 times less, so the overhead
is 1%.
Needless to say, that on a machine where just searching a socket in a
hash table consumes 1% of the CPU, we simply can not run 50 external
phone calls at the same time.
But there is a gain.
--
Gilles.
|
|
From: Gilles C. <gil...@xe...> - 2008-09-01 08:24:52
Attachments:
hlist.h
|
Jan Kiszka wrote:
> Unfortunately, hlist is not available with kernel 2.4. Please provide
> wrappers as well.
Hi Jan,
here is a hlist.h header with only what is needed. I did not know where
you want to put it, so here it comes, as an attachment.
I could not test it, since I have no working 2.4 sources at hand.
Regards.
--
Gilles.
|
|
From: Gilles C. <gil...@xe...> - 2008-09-01 08:40:51
Attachments:
hlist.h
|
Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Unfortunately, hlist is not available with kernel 2.4. Please provide
>> wrappers as well.
>
> Hi Jan,
>
> here is a hlist.h header with only what is needed. I did not know where
> you want to put it, so here it comes, as an attachment.
>
> I could not test it, since I have no working 2.4 sources at hand.
Forgot hlist_entry. Here is a fixed version.
--
Gilles.
|
|
From: Jan K. <jan...@we...> - 2008-09-01 09:16:05
Attachments:
signature.asc
|
Gilles Chanteperdrix wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Unfortunately, hlist is not available with kernel 2.4. Please provide >>> wrappers as well. >> Hi Jan, >> >> here is a hlist.h header with only what is needed. I did not know where >> you want to put it, so here it comes, as an attachment. >> >> I could not test it, since I have no working 2.4 sources at hand. > > Forgot hlist_entry. Here is a fixed version. > Thanks, but... mind to integrate it into stack/include/rtnet_port.h? We can then ask Wolfgang to do some tests on 2.4. Jan |
|
From: Gilles C. <gil...@xe...> - 2008-09-01 09:29:47
|
Jan Kiszka wrote:
> Thanks, but... mind to integrate it into stack/include/rtnet_port.h?
>
> We can then ask Wolfgang to do some tests on 2.4.
Here it comes.
--- stack/include/rtnet_port.h~ 2008-03-31 19:33:11.000000000 +0200
+++ stack/include/rtnet_port.h 2008-09-01 11:24:31.000000000 +0200
@@ -24,6 +24,7 @@
#include <linux/bitops.h>
#include <linux/moduleparam.h>
+#include <linux/list.h>
#if 1
#include <rtskb.h>
@@ -56,6 +57,56 @@
module_param_array(name, int, NULL, 0444)
#endif
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)
+# define LIST_POISON1 ((void *) 0x00100100)
+# define LIST_POISON2 ((void *) 0x00200200)
+
+struct hlist_head {
+ struct hlist_node *first;
+};
+
+struct hlist_node {
+ struct hlist_node *next, **pprev;
+};
+
+# define INIT_HLIST_HEAD(ptr) ((ptr)->first = NULL)
+
+static inline void __hlist_del(struct hlist_node *n)
+{
+ struct hlist_node *next = n->next;
+ struct hlist_node **pprev = n->pprev;
+ *pprev = next;
+ if (next)
+ next->pprev = pprev;
+}
+
+static inline void hlist_del(struct hlist_node *n)
+{
+ __hlist_del(n);
+ n->next = LIST_POISON1;
+ n->pprev = LIST_POISON2;
+}
+
+static inline void hlist_add_head(struct hlist_node *n, struct hlist_head *h)
+{
+ struct hlist_node *first = h->first;
+ n->next = first;
+ if (first)
+ first->pprev = &n->next;
+ h->first = n;
+ n->pprev = &h->first;
+}
+
+# define hlist_entry(ptr, type, member) \
+ ((type *)((char *)(ptr) - (unsigned long)(&((type *)0)->member)))
+
+# define hlist_for_each_entry(tpos, pos, head, member) \
+ for (pos = (head)->first; \
+ pos && ({ prefetch(pos->next); 1;}) && \
+ ({ tpos = hlist_entry(pos, typeof(*tpos), member); 1;}); \
+ pos = pos->next)
+#endif
+
static inline void rtnetif_start_queue(struct rtnet_device *rtdev)
{
clear_bit(__LINK_STATE_XOFF, &rtdev->state);
--
Gilles.
|
|
From: Jan K. <jan...@we...> - 2008-09-01 20:45:26
Attachments:
signature.asc
|
Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Thanks, but... mind to integrate it into stack/include/rtnet_port.h? >> >> We can then ask Wolfgang to do some tests on 2.4. > > Here it comes. Thanks. I committed both pieces, finally. Wolfgang, once you have some time, please check if latest SVN still builds for Linux 2.4. We both lack appropriate test environments, unfortunately. TiA. Jan |
|
From: Gilles C. <gil...@xe...> - 2008-08-28 15:47:04
|
Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Hi,
>>>
>>> here is a repost of the rtnet-hash-port.diff patch. Only, without the
>>> "receiving" bit, and with the INADDR_ANY bit fixed.
>> OK, almost ready for merge.
>>
>> BTW, did you also happen to measure the performance gain (and for how
>> many sockets)?
>
> I have just done a quick test: with 200 sockets, the worst case (i.e.
> the last socket) takes 30us to search the sockets table. Due to the
> linear search, the average case is half the worst case.
>
> 200 sockets is what happens when 50 phones trying to phone outside at
> the same time, each conversation results in 4 sockets being opened by
> the NAT helper for each conversation (1 RTP outbound, 1 RTCP inbound, 1
> RTP inbound, 1 RTP). An RTP stream takes 50 packets by second. an RTCP
> stream 0.2 packet by second. So, the total is 100 * 50 + 100 * 0.2 =
> 5020 packets by second, and the overhead of the socket search is 5020 *
> 15 us = 75300 us, i.e. 75 ms, i.e. 7.5 % CPU.
>
> Now, when using the hash table, the worst case, which is the same as the
> average case is 2 us, that is precisely 7.5 times less, so the overhead
> is 1%.
>
> Needless to say, that on a machine where just searching a socket in a
> hash table consumes 1% of the CPU, we simply can not run 50 external
> phone calls at the same time.
Actually, the box sustains 5000 packets by second. Ok. They eat 80% cpu.
--
Gilles.
|
|
From: Li C. <lic...@gm...> - 2008-08-29 08:21:10
|
hi,jan:
I want to do a experiment on rtnet using netem and want to know that
how rtnet works when transmission error occurs. but after I setup rtnet,
I use:
#tc qdisc add dev rteth0 root netem delay 1ms
there is error message: can't find "rteth0" device.
I use strace to find what happened, I got:
ioctl(4,SIOCGIFINDEX,{ifr_name="rteth0",..}) =-1(No such device)
I think this because rtnet is invisible to Linux, so tc can not work.
Is it possible to use netem on rtnet? if it is, what should I do?
thanks very much!!
lily
|
|
From: Jan K. <jan...@we...> - 2008-09-01 20:45:22
Attachments:
signature.asc
|
Li Chanjuan wrote:
> hi,jan:
>
> I want to do a experiment on rtnet using netem and want to know that
> how rtnet works when transmission error occurs. but after I setup rtnet,
> I use:
> #tc qdisc add dev rteth0 root netem delay 1ms
> there is error message: can't find "rteth0" device.
> I use strace to find what happened, I got:
>
> ioctl(4,SIOCGIFINDEX,{ifr_name="rteth0",..}) =-1(No such device)
>
> I think this because rtnet is invisible to Linux, so tc can not work.
Exactly.
>
> Is it possible to use netem on rtnet? if it is, what should I do?
Nope, there is no (publicly known) implementation of a netem-like
service on top of RTnet. However, it shouldn't be too tricky to write
such support on top of RTnet's socket API.
On the other hand, I wonder why you need hard real-time guarantees for a
"noise generator" like netem? Why is Linux itself unsuited?
Jan
|
|
From: Li C. <lic...@gm...> - 2008-09-03 01:28:10
|
在 2008-09-01一的 22:45 +0200，Jan Kiszka写道：
> Li Chanjuan wrote:
> > hi,jan:
> >
> > I want to do a experiment on rtnet using netem and want to know that
> > how rtnet works when transmission error occurs. but after I setup rtnet,
> > I use:
> > #tc qdisc add dev rteth0 root netem delay 1ms
> > there is error message: can't find "rteth0" device.
> > I use strace to find what happened, I got:
> >
> > ioctl(4,SIOCGIFINDEX,{ifr_name="rteth0",..}) =-1(No such device)
> >
> > I think this because rtnet is invisible to Linux, so tc can not work.
>
> Exactly.
>
> >
> > Is it possible to use netem on rtnet? if it is, what should I do?
>
> Nope, there is no (publicly known) implementation of a netem-like
> service on top of RTnet. However, it shouldn't be too tricky to write
> such support on top of RTnet's socket API.
>
> On the other hand, I wonder why you need hard real-time guarantees for a
> "noise generator" like netem? Why is Linux itself unsuited?
what we interests is how rtnet will behave when there is communication
error. Now I redesign our experiement plan like this:
______ ________ _______
| | | | | |
| rtnet |----->|Linux |------->| rtnet|
|_______| |_______| |______|
I use netem on the linux to simulate communication error between two
renet nodes. Then I will run some roundtrip test to see what happen.
Do you think such data make sence?
lily
thanks.
>
|
|
From: Wolfgang G. <wg...@gr...> - 2008-06-30 07:34:05
|
Hi Gilles, Gilles Chanteperdrix wrote: > Hi, > > here is another patch to RTnet, which replaces the list of UDP sockets > with a hash, so makes the searching for an RTnet registered socket a bit > faster. What does a "bit faster" mean. I'm curious, do you have some benchmark figures at hand? Wolfgang. |
|
From: Gilles C. <gil...@xe...> - 2008-06-30 10:01:48
|
Wolfgang Grandegger wrote:
> Hi Gilles,
>
> Gilles Chanteperdrix wrote:
>> Hi,
>>
>> here is another patch to RTnet, which replaces the list of UDP sockets
>> with a hash, so makes the searching for an RTnet registered socket a bit
>> faster.
>
> What does a "bit faster" mean. I'm curious, do you have some benchmark
> figures at hand?
No, but the current scheme is to test every packet against every socket
(until one is found). This is O(N).
With a hash, and a well dimensioned/behaved system, you get O(1).
So, no doubt there is a gain, at least on systems with many sockets
opened, and especially when you receive many non real-time UDP packets
which you must send to the proxy: non real-time packets are tested
against all opened sockets, to finally find that they are matching none
of them.
--
Gilles.
|
|
From: Jan K. <jan...@we...> - 2008-07-01 23:40:52
Attachments:
signature.asc
|
Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Hi,
>>>>
>>>> here is another patch to RTnet, which replaces the list of UDP sockets
>>>> with a hash, so makes the searching for an RTnet registered socket a
>>>> bit
>>>> faster.
>>> Ok. Should have read the patch better before sending it. There are two
>>> more features poured into this patch:
>>> - we add a "receiving" member to the UDP socket structure, which is only
>>> sets either if the program using the socket has called recv, or if the
>>> program has bound the socket to a particular port, this is to prevent a
>>> problem which happens when putting an RTnet box on an heterogenous
>>> network, with an application which only uses a socket to send packets:
>>> if the network sends some packets to the automatically assigned port,
>>> they use rtskbs from the socket pool, until the socket pool becomes
>>> empty, and no more packet can be sent using the socket.
>>> - when a socket is bound to a particular IP address, we use this IP as
>>> the source IP of the packets sent by this socket, even if they end-up
>>> being sent through another interface, this is Linux behaviour, and was
>>> required for one of our applications to work correctly with
>>> off-the-shelf IP phones (they do filtering based on the source IP).
>>
>> Could you break those two changes out and post them separately? Such
>> changes are too critical to have them all-in-one.
>
> Ok. Here it comes, extracted from revision control history.
> Apply in order:
> - rtnet-fix-src-ip.diff
> --- stack/ipv4/udp.c (révision 3250)
> +++ stack/ipv4/udp.c (révision 5093)
> @@ -576,14 +576,8 @@ ssize_t rt_udp_sendmsg(struct rtdm_dev_c
> if (err)
> return err;
>
> - /* check if specified source address fits */
> - if ((saddr != INADDR_ANY) && (saddr != rt.rtdev->local_ip)) {
> - rtdev_dereference(rt.rtdev);
> - return -EHOSTUNREACH;
> - }
> -
> /* we found a route, remember the routing dest-addr could be the netmask */
> - ufh.saddr = rt.rtdev->local_ip;
> + ufh.saddr = saddr ?: rt.rtdev->local_ip;
One minor remark: I would prefer "saddr != INADDR_ANY ? saddr : ..."
here (the compiler should produce the same output).
I guess you tested this with CONFIG_RTNET_RTIPV4_ROUTE_SRC enabled,
right? I wonder if we shouldn't turn this unconditionally on now, it
won't cost us much in the hot path, but it would make the code more
readable.
> - rtnet-check-receiving.diff
> @@ -375,13 +379,23 @@ ssize_t rt_udp_recvmsg(struct rtdm_dev_c
> struct udphdr *uh;
> struct sockaddr_in *sin;
> nanosecs_rel_t timeout = sock->timeout;
> - int ret;
> + int ret, index;
> + rtdm_lockctx_t context;
>
>
> /* non-blocking receive? */
> if (testbits(msg_flags, MSG_DONTWAIT))
> timeout = -1;
>
> + rtdm_lock_get_irqsave(&udp_socket_base_lock, context);
> + if ((index = sock->prot.inet.reg_index) < 0) {
> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
> + /* socket is being closed */
> + return -EBADF;
> + }
> + port_registry[index].receiving = 1;
> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
> +
This is the only part of the patch I don't like. I don't want to add
another lock site to the RX path. I see the problem, but I think we need
some other solution. Maybe something that disables reception, and thus
unwanted buffer consumption.
> - rtnet-hash-port.diff
This needs more thoughts than I've left for tonight (no fundamental
problems, I just want to exclude that I'm overseeing some pitfalls of it).
Jan
|
|
From: Gilles C. <gil...@xe...> - 2008-07-02 09:09:44
|
Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Hi,
>>>>>
>>>>> here is another patch to RTnet, which replaces the list of UDP sockets
>>>>> with a hash, so makes the searching for an RTnet registered socket a
>>>>> bit
>>>>> faster.
>>>> Ok. Should have read the patch better before sending it. There are two
>>>> more features poured into this patch:
>>>> - we add a "receiving" member to the UDP socket structure, which is only
>>>> sets either if the program using the socket has called recv, or if the
>>>> program has bound the socket to a particular port, this is to prevent a
>>>> problem which happens when putting an RTnet box on an heterogenous
>>>> network, with an application which only uses a socket to send packets:
>>>> if the network sends some packets to the automatically assigned port,
>>>> they use rtskbs from the socket pool, until the socket pool becomes
>>>> empty, and no more packet can be sent using the socket.
>>>> - when a socket is bound to a particular IP address, we use this IP as
>>>> the source IP of the packets sent by this socket, even if they end-up
>>>> being sent through another interface, this is Linux behaviour, and was
>>>> required for one of our applications to work correctly with
>>>> off-the-shelf IP phones (they do filtering based on the source IP).
>>> Could you break those two changes out and post them separately? Such
>>> changes are too critical to have them all-in-one.
>> Ok. Here it comes, extracted from revision control history.
>> Apply in order:
>> - rtnet-fix-src-ip.diff
>
>> --- stack/ipv4/udp.c (révision 3250)
>> +++ stack/ipv4/udp.c (révision 5093)
>> @@ -576,14 +576,8 @@ ssize_t rt_udp_sendmsg(struct rtdm_dev_c
>> if (err)
>> return err;
>>
>> - /* check if specified source address fits */
>> - if ((saddr != INADDR_ANY) && (saddr != rt.rtdev->local_ip)) {
>> - rtdev_dereference(rt.rtdev);
>> - return -EHOSTUNREACH;
>> - }
>> -
>> /* we found a route, remember the routing dest-addr could be the netmask */
>> - ufh.saddr = rt.rtdev->local_ip;
>> + ufh.saddr = saddr ?: rt.rtdev->local_ip;
>
> One minor remark: I would prefer "saddr != INADDR_ANY ? saddr : ..."
> here (the compiler should produce the same output).
>
> I guess you tested this with CONFIG_RTNET_RTIPV4_ROUTE_SRC enabled,
> right? I wonder if we shouldn't turn this unconditionally on now, it
> won't cost us much in the hot path, but it would make the code more
> readable.
No, I run rtnet without CONFIG_RTNET_RTIPV4_ROUTE_SRC. All routing,
nating, bridging is done by Linux in my configuration. Also, it looks to
me like this code is run unconditonnally, maybe is it because I am using
an older release ?
>
>> - rtnet-check-receiving.diff
>
>> @@ -375,13 +379,23 @@ ssize_t rt_udp_recvmsg(struct rtdm_dev_c
>> struct udphdr *uh;
>> struct sockaddr_in *sin;
>> nanosecs_rel_t timeout = sock->timeout;
>> - int ret;
>> + int ret, index;
>> + rtdm_lockctx_t context;
>>
>>
>> /* non-blocking receive? */
>> if (testbits(msg_flags, MSG_DONTWAIT))
>> timeout = -1;
>>
>> + rtdm_lock_get_irqsave(&udp_socket_base_lock, context);
>> + if ((index = sock->prot.inet.reg_index) < 0) {
>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>> + /* socket is being closed */
>> + return -EBADF;
>> + }
>> + port_registry[index].receiving = 1;
>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>> +
>
> This is the only part of the patch I don't like. I don't want to add
> another lock site to the RX path. I see the problem, but I think we need
> some other solution. Maybe something that disables reception, and thus
> unwanted buffer consumption.
You are lucky, because I can not remember why I did this :-) Maybe the
lock is not needed after all...
>
>> - rtnet-hash-port.diff
>
> This needs more thoughts than I've left for tonight (no fundamental
> problems, I just want to exclude that I'm overseeing some pitfalls of it).
FWIW, the patch is running for some time now, even on production
systems, without any problems. The only thing which I only tested in
unit tests and had not to test on real systems is the case where two
applications open bind the same port on two different IPs.
--
Gilles.
|
|
From: Jan K. <jan...@we...> - 2008-07-04 20:39:09
Attachments:
signature.asc
|
Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Gilles Chanteperdrix wrote:
>>>>>> Hi,
>>>>>>
>>>>>> here is another patch to RTnet, which replaces the list of UDP sockets
>>>>>> with a hash, so makes the searching for an RTnet registered socket a
>>>>>> bit
>>>>>> faster.
>>>>> Ok. Should have read the patch better before sending it. There are two
>>>>> more features poured into this patch:
>>>>> - we add a "receiving" member to the UDP socket structure, which is only
>>>>> sets either if the program using the socket has called recv, or if the
>>>>> program has bound the socket to a particular port, this is to prevent a
>>>>> problem which happens when putting an RTnet box on an heterogenous
>>>>> network, with an application which only uses a socket to send packets:
>>>>> if the network sends some packets to the automatically assigned port,
>>>>> they use rtskbs from the socket pool, until the socket pool becomes
>>>>> empty, and no more packet can be sent using the socket.
>>>>> - when a socket is bound to a particular IP address, we use this IP as
>>>>> the source IP of the packets sent by this socket, even if they end-up
>>>>> being sent through another interface, this is Linux behaviour, and was
>>>>> required for one of our applications to work correctly with
>>>>> off-the-shelf IP phones (they do filtering based on the source IP).
>>>> Could you break those two changes out and post them separately? Such
>>>> changes are too critical to have them all-in-one.
>>> Ok. Here it comes, extracted from revision control history.
>>> Apply in order:
>>> - rtnet-fix-src-ip.diff
>>> --- stack/ipv4/udp.c (révision 3250)
>>> +++ stack/ipv4/udp.c (révision 5093)
>>> @@ -576,14 +576,8 @@ ssize_t rt_udp_sendmsg(struct rtdm_dev_c
>>> if (err)
>>> return err;
>>>
>>> - /* check if specified source address fits */
>>> - if ((saddr != INADDR_ANY) && (saddr != rt.rtdev->local_ip)) {
>>> - rtdev_dereference(rt.rtdev);
>>> - return -EHOSTUNREACH;
>>> - }
>>> -
>>> /* we found a route, remember the routing dest-addr could be the netmask */
>>> - ufh.saddr = rt.rtdev->local_ip;
>>> + ufh.saddr = saddr ?: rt.rtdev->local_ip;
>> One minor remark: I would prefer "saddr != INADDR_ANY ? saddr : ..."
>> here (the compiler should produce the same output).
>>
>> I guess you tested this with CONFIG_RTNET_RTIPV4_ROUTE_SRC enabled,
>> right? I wonder if we shouldn't turn this unconditionally on now, it
>> won't cost us much in the hot path, but it would make the code more
>> readable.
>
> No, I run rtnet without CONFIG_RTNET_RTIPV4_ROUTE_SRC. All routing,
> nating, bridging is done by Linux in my configuration. Also, it looks to
> me like this code is run unconditonnally, maybe is it because I am using
> an older release ?
In older releases, this code did not even exist.
CONFIG_RTNET_RTIPV4_ROUTE_SRC used to control if the source address a
socket is bound to will be considered during output routing table
consultation. That's only relevant when you have multiple interface
which are attached to the same network.
Anyway, I just turned this into unconditional code (allowed quite some
cleanups) - and I merged the above patch of yours.
Jan
|
|
From: Gilles C. <gil...@xe...> - 2008-07-02 14:57:59
|
Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> + rtdm_lock_get_irqsave(&udp_socket_base_lock, context);
>> + if ((index = sock->prot.inet.reg_index) < 0) {
>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>> + /* socket is being closed */
>> + return -EBADF;
>> + }
>> + port_registry[index].receiving = 1;
>> + rtdm_lock_put_irqrestore(&udp_socket_base_lock, context);
>> +
>
> This is the only part of the patch I don't like. I don't want to add
> another lock site to the RX path. I see the problem, but I think we need
> some other solution. Maybe something that disables reception, and thus
> unwanted buffer consumption.
Actually, what we can do is put the "receiving" member in the udp socket
structure, so that there is no risk, when setting this member to 1, to
set to 1 the member of another socket.
--
Gilles.
|
Thanks for helping keep SourceForge clean.
X