Date: Thu, 7 Jan 2016 15:31:25 +0200 From: Boris Astardzhiev <boris.astardzhiev@gmail.com> To: Luigi Rizzo <rizzo@iet.unipi.it> Cc: Mark Delany <c2h@romeo.emu.st>, "freebsd-net@freebsd.org" <freebsd-net@freebsd.org> Subject: Re: Does FreeBSD have sendmmsg or recvmmsg system calls? Message-ID: <CAP=KkTyun62m2oeRxJDsn3MbjkV%2B4hVqp6-3sGtcD5GVVFAdqg@mail.gmail.com> In-Reply-To: <CA%2BhQ2%2Bh4NNz9tgSpjJdv7fXteq5tAR7o3LvjV=u08NHjRLPwmA@mail.gmail.com> References: <alpine.BSF.2.20.1601031833130.84701@localhost.my.domain> <1451841004.10139.34.camel@me.com> <alpine.BSF.2.20.1601031744040.20884@fledge.watson.org> <CAJ-Vmomxcn%2BiYJAzNViL8WnepsCihrkTuHd8=0O6vONKsTExCA@mail.gmail.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> <CAP=KkTwfpjec2Tgnm4PRR3u8t4GEqN9Febm5HRcqapifBG-B6g@mail.gmail.com> <CA%2BhQ2%2Bh4NNz9tgSpjJdv7fXteq5tAR7o3LvjV=u08NHjRLPwmA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
>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 <rizzo@iet.unipi.it> 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
> <boris.astardzhiev@gmail.com> 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 <ngie@FreeBSD.org>
> > 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 <c2h@romeo.emu.st> 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)
> -----------------------------------------+-------------------------------
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAP=KkTyun62m2oeRxJDsn3MbjkV%2B4hVqp6-3sGtcD5GVVFAdqg>