Re: [PATCH 6/8] udp: Unify udp_sock_handler_splice() with udp_sock_handler()

On Wed, 14 Dec 2022 12:19:14 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  On Tue, Dec 13, 2022 at 11:48:58PM +0100, Stefano
Brivio wrote:
  On Mon,  5 Dec 2022 19:14:23 +1100
 David Gibson <david(a)gibson.dropbear.id.au> wrote:
   
  These two functions now have a very similar
structure, and their first
 part (calling recvmmsg()) is functionally identical.  So, merge the two
 functions into one.
 
 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
 ---
  udp.c | 86 +++++++++++++++++++----------------------------------------
  1 file changed, 28 insertions(+), 58 deletions(-)
 
 diff --git a/udp.c b/udp.c
 index 7c601cc..6ccfe8c 100644
 --- a/udp.c
 +++ b/udp.c
 @@ -590,52 +590,6 @@ static void udp_splice_sendfrom(const struct ctx *c, unsigned start,
unsigned n,
  	sendmmsg(s, mmh_send + start, n, MSG_NOSIGNAL);
  }
  
 -/**
 - * udp_sock_handler_splice() - Handler for socket mapped to "spliced"
connection
 - * @c:		Execution context
 - * @ref:	epoll reference
 - * @events:	epoll events bitmap
 - * @now:	Current timestamp
 - */
 -static void udp_sock_handler_splice(const struct ctx *c, union epoll_ref ref,
 -				    uint32_t events, const struct timespec *now)
 -{
 -	in_port_t dst = ref.r.p.udp.udp.port;
 -	int v6 = ref.r.p.udp.udp.v6, n, i, m;
 -	struct mmsghdr *mmh_recv;
 -
 -	if (!(events & EPOLLIN))
 -		return;
 -
 -	if (v6)
 -		mmh_recv = udp6_l2_mh_sock;
 -	else
 -		mmh_recv = udp4_l2_mh_sock;
 -
 -	n = recvmmsg(ref.r.s, mmh_recv, UDP_MAX_FRAMES, 0, NULL);
 -
 -	if (n <= 0)
 -		return;
 -
 -	if (v6)
 -		udp_localname6.sin6_port = htons(dst);
 -	else
 -		udp_localname4.sin_port = htons(dst);
 -
 -	for (i = 0; i < n; i += m) {
 -		in_port_t src = sa_port(v6, mmh_recv[i].msg_hdr.msg_name);
 -
 -		for (m = 1; i + m < n; m++) {
 -			void *mname = mmh_recv[i + m].msg_hdr.msg_name;
 -			if (sa_port(v6, mname) != src)
 -				break;
 -		}
 -
 -		udp_splice_sendfrom(c, i, m, src, dst, v6, ref.r.p.udp.udp.ns,
 -				    ref.r.p.udp.udp.orig, now);
 -	}
 -}
 -
  /**
   * udp_update_hdr4() - Update headers for one IPv4 datagram
   * @c:		Execution context
 @@ -945,27 +899,43 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref,
uint32_t events,
  {
  	in_port_t dstport = ref.r.p.udp.udp.port;
  	bool v6 = ref.r.p.udp.udp.v6;
 -	struct mmsghdr *sock_mmh;
 +	struct mmsghdr *mmh_recv;
 +	unsigned int i, m;
  	ssize_t n;
  
 -	if (events == EPOLLERR)
 +	if (!(events & EPOLLIN))   
 
 Pre-existing, unrelated issue, but this reminds me: we don't handle
 socket errors here, and while udp_timer_one() will drop any sockets we
 created, eventually, it would probably be better to act right away.   
 
 Ok... I'm not sure what, if anything, you would like me to do about
 it, however. 
No no, sorry, nothing, I wanted to share in case you happen to touch
this again soon, or if you had thoughts about it. I can fix this
separately once you're done with changes for UDP.

-- 
Stefano