From owner-freebsd-net@freebsd.org Thu Jan 7 13:31:27 2016 Return-Path: Delivered-To: freebsd-net@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 604F8A6672D for ; Thu, 7 Jan 2016 13:31:27 +0000 (UTC) (envelope-from boris.astardzhiev@gmail.com) Received: from mail-wm0-x230.google.com (mail-wm0-x230.google.com [IPv6:2a00:1450:400c:c09::230]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id DF8401F25 for ; Thu, 7 Jan 2016 13:31:26 +0000 (UTC) (envelope-from boris.astardzhiev@gmail.com) Received: by mail-wm0-x230.google.com with SMTP id b14so123411240wmb.1 for ; Thu, 07 Jan 2016 05:31:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=4cnwFQ5795a+J3iQGIvSwjFXuJ4KmxU2GrmC1GTP7T0=; b=CA6Ap29giQ6fonFckxI1BVgCxCBsscgk64mFy1JOGaQJT0Tw11sOW3g7ftKpxblri9 M1qNB1aIYo1D+POZbT7qemzeQTGlmH1YUfP1icKVdyIfACBGGyemUzyRhbekwg79j9tC SgX7QQ0cBUMKbXKgXD3BhLH9Y2A/nJ5nAtdVQi/gRN4pegXZrsfsKmfV/IWoP0+OTK+W Se6YRf2uEw1a9uPf3bgyoDDR/XFf1Mu6mj8b6g5U0TCVgbVSnQxdW0JMU/IAas7uao1U vKf1fXfdiXSJ1+F1G/+pDygsbLkDV7oCA3pFiD+yIadxYAFtHY817k18CMGqqNMt0j+5 quhA== MIME-Version: 1.0 X-Received: by 10.194.78.175 with SMTP id c15mr72432721wjx.16.1452173485385; Thu, 07 Jan 2016 05:31:25 -0800 (PST) Received: by 10.28.136.148 with HTTP; Thu, 7 Jan 2016 05:31:25 -0800 (PST) In-Reply-To: References: <1451841004.10139.34.camel@me.com> <20160103214720.72014.qmail@f5-external.bushwire.net> <20160104085415.GS3625@kib.kiev.ua> <20160104091108.50654.qmail@f5-external.bushwire.net> <20160104093859.GV3625@kib.kiev.ua> <20160104101747.58347.qmail@f5-external.bushwire.net> <20160104194044.GD3625@kib.kiev.ua> <20160104210741.32812.qmail@f5-external.bushwire.net> Date: Thu, 7 Jan 2016 15:31:25 +0200 Message-ID: Subject: Re: Does FreeBSD have sendmmsg or recvmmsg system calls? From: Boris Astardzhiev To: Luigi Rizzo Cc: Mark Delany , "freebsd-net@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.20 X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jan 2016 13:31:27 -0000 >Hi Boris, >thanks for working on this. > >A few comments: > >- do you have any performance data on calling *mmsg() versus > multiple invocations of the equivalent *msg() ? No, I don't for now. >- in the following chunk in recvmmsg.c , there are slight > type differences between the function and the internal cast. > Same in sendmmsg.c > > +ssize_t > +recvmmsg(int s, struct mmsghdr *msgvec, unsigned int vlen, int flags) > +{ > + > + return (((int (*)(int, struct mmsghdr *, int, int)) > + __libc_interposing[INTERPOS_recvmmsg])(s, msgvec, vlen, flags)); > +} I see. I'll fix it. >- why did you add a cap_rights_init() to the functions when > neither sendmsg or recvmsg have it (in other words, this is > a worthwhile addition but perhaps should be done later I need to grab access to so_error due to error handling. send/recvmmsg() would have to return the number of datagrams sent/rcvd. send/recvmsg() return the number of characters sent/rcvd. Thus I followed Konstantin's explanation regarding so_error. >- the initial part of the two functions sys_*mmsg() is almost > exactly the same so can you define a function with the common code ? >- you could probably avoid the repeated malloc/free of the iov > with a first loop that finds the max iov size and does the allocation > only once. copyiniov can then be replaced by a copyin I'll try to optimize the calls at the end. I like the suggestion though and I'll scrutinize over it. >- (minor) what is the point of copying the current entry into msg rather than > just using mp-> ... Just my style for making things more readable to me. >- can you add a comment explaining why you do the following > > error = copyout(&td->td_retval[0], &uap->msgvec[i].msg_len, > sizeof(td->td_retval[0])); Thank you. I'll add a comment for clarification. Best Regards, Boris Astardzhiev On Thu, Jan 7, 2016 at 12:56 PM, Luigi Rizzo wrote: > Hi Boris, > thanks for working on this. > > A few comments: > > - do you have any performance data on calling *mmsg() versus > multiple invocations of the equivalent *msg() ? > > - in the following chunk in recvmmsg.c , there are slight > type differences between the function and the internal cast. > Same in sendmmsg.c > > +ssize_t > +recvmmsg(int s, struct mmsghdr *msgvec, unsigned int vlen, int flags) > +{ > + > + return (((int (*)(int, struct mmsghdr *, int, int)) > + __libc_interposing[INTERPOS_recvmmsg])(s, msgvec, vlen, > flags)); > +} > > > - why did you add a cap_rights_init() to the functions when > neither sendmsg or recvmsg have it (in other words, this is > a worthwhile addition but perhaps should be done later > > - the initial part of the two functions sys_*mmsg() is almost > exactly the same so can you define a function with the common code ? > > - you could probably avoid the repeated malloc/free of the iov > with a first loop that finds the max iov size and does the allocation > only once. copyiniov can then be replaced by a copyin > > - (minor) what is the point of copying the current entry into msg rather > than > just using mp-> ... > > - can you add a comment explaining why you do the following > > error = copyout(&td->td_retval[0], &uap->msgvec[i].msg_len, > sizeof(td->td_retval[0])); > > > cheers > luigi > > On Thu, Jan 7, 2016 at 1:51 AM, Boris Astardzhiev > wrote: > > Hello, > > > > Here's my implementation of the two system calls. The patch is against > HEAD > > from a couple of days: > > commit ff9e83788d7ed52342dcba4dff1e62fdf3cc985c > > Author: ngie > > Date: Mon Jan 4 03:34:22 2016 +0000 > > > > Remove free'ing of an uninitialized variable > > > > Just remove it completely from the test as it's initialized but > unused > > apart > > from the free(3) call > > > > Differential Revision: https://reviews.freebsd.org/D4769 (part of > > larger diff) > > MFC after: 5 days > > Reported by: cppcheck > > Reviewed by: oshogbo > > Sponsored by: EMC / Isilon Storage Division > > > > I've made brief tests using the examples in the manpages in Linux > skipping > > the timeout stuff: > > http://man7.org/linux/man-pages/man2/sendmmsg.2.html > > http://man7.org/linux/man-pages/man2/recvmmsg.2.html > > > > I've tried to stick to the comments in the thread but any > > suggestions/testings > > are also welcomed so that I can fix the calls accordingly. > > > > Regards, > > Boris Astardzhiev > > > > > > On Mon, Jan 4, 2016 at 11:07 PM, Mark Delany wrote: > > > >> > You just repeat arguments for the text in my messages, which you > removed > >> > on reply. > >> > >> My goal is to get you to scruitinize. > >> > >> Thank you for helping. > >> > >> > >> Mark. > >> _______________________________________________ > >> freebsd-net@freebsd.org mailing list > >> https://lists.freebsd.org/mailman/listinfo/freebsd-net > >> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" > >> > > > > _______________________________________________ > > freebsd-net@freebsd.org mailing list > > https://lists.freebsd.org/mailman/listinfo/freebsd-net > > To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" > > > > -- > -----------------------------------------+------------------------------- > Prof. Luigi RIZZO, rizzo@iet.unipi.it . Dip. di Ing. dell'Informazione > http://www.iet.unipi.it/~luigi/ . Universita` di Pisa > TEL +39-050-2217533 . via Diotisalvi 2 > Mobile +39-338-6809875 . 56122 PISA (Italy) > -----------------------------------------+------------------------------- >