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
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------- Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW! Studies have shown that voting for your favorite open source project, along with a healthy diet, reduces your potential for chronic lameness and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________ RTnet-developers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/rtnet-developers
