On Fri, Oct 30, 2015 at 07:05:09PM +0100, Alexander Bluhm wrote:
> On Fri, Oct 30, 2015 at 06:48:16PM +0100, Mike Belopuhov wrote:
> > On Fri, Oct 30, 2015 at 18:27 +0100, Alexander Bluhm wrote:
> > > On Fri, Oct 30, 2015 at 02:04:52PM +0100, Reyk Floeter wrote:
> > > > > Socket splicing somove() does the same thing. I will change it to
> > > > > use m_resethdr() after that got commited.
> > >
> > > I just compared code in somove() with m_resethdr(). Socket splicing
> > > has to clear the whole packet header, not only the pf part. I think
> > > this is also useful for pair(4) as it should behave like a cable.
> > >
> > > Resetting the other m_flags seems wrong. I have put the M_PKTHDR
> > > into an assert. If it is not an packet header, the memset() could
> > > overwrite mbuf data.
> > >
> > > ok?
> > >
> >
> > Resetting flags that are set on input (e.g. M_CONF, M_AUTH, etc.) is
> > not wrong since input on the other pair has not happened yet and it
> > might provide more problems than shortcuts. I'm not certain which
> > flags may still be of use before the other pair figures it on its
> > own. Do you have any examples of such flags?
>
> I think flags that must be preserverd are
> - M_ZEROIZE
> - M_EXT
>
> In sys/mbuf.h Flags are split into mbuf flags and mbuf pkthdr flags.
> As we clear the packet header but keep the data, we should probably
> only clear the mbuf pkthdr flags.
>
> Something like this flag combination?
>
> bluhm
>
Yes, OK reyk@
tested with pair(4) ... ipsec on pair(4) ... routed ipsec on pair(4) ...
(pair0 -> ipsec -> pair1 -> $ext_if) ... bridge/pair stp ...
> Index: kern/uipc_mbuf.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v
> retrieving revision 1.209
> diff -u -p -r1.209 uipc_mbuf.c
> --- kern/uipc_mbuf.c 30 Oct 2015 12:54:36 -0000 1.209
> +++ kern/uipc_mbuf.c 30 Oct 2015 18:00:59 -0000
> @@ -253,13 +253,18 @@ m_inithdr(struct mbuf *m)
> void
> m_resethdr(struct mbuf *m)
> {
> - /* like the previous, but keep any associated data and mbufs */
> - m->m_flags = M_PKTHDR;
> - memset(&m->m_pkthdr.pf, 0, sizeof(m->m_pkthdr.pf));
> - m->m_pkthdr.pf.prio = IFQ_DEFPRIO;
> + int len = m->m_pkthdr.len;
> +
> + KASSERT(m->m_flags & M_PKTHDR);
> + m->m_flags &= (M_EXT|M_PKTHDR|M_EOR|M_EXTWR|M_ZEROIZE);
>
> - /* also delete all mbuf tags to reset the state */
> + /* delete all mbuf tags to reset the state */
> m_tag_delete_chain(m);
> +
> + /* like m_inithdr(), but keep any associated data and mbufs */
> + memset(&m->m_pkthdr, 0, sizeof(m->m_pkthdr));
> + m->m_pkthdr.pf.prio = IFQ_DEFPRIO;
> + m->m_pkthdr.len = len;
> }
>
> struct mbuf *
> Index: kern/uipc_socket.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.142
> diff -u -p -r1.142 uipc_socket.c
> --- kern/uipc_socket.c 24 Aug 2015 14:28:25 -0000 1.142
> +++ kern/uipc_socket.c 30 Oct 2015 17:06:57 -0000
> @@ -1325,10 +1325,8 @@ somove(struct socket *so, int wait)
> goto release;
> m->m_nextpkt = NULL;
> if (m->m_flags & M_PKTHDR) {
> - m_tag_delete_chain(m);
> - memset(&m->m_pkthdr, 0, sizeof(m->m_pkthdr));
> + m_resethdr(m);
> m->m_pkthdr.len = len;
> - m->m_pkthdr.pf.prio = IFQ_DEFPRIO;
> }
>
> /* Send window update to source peer as receive buffer has changed. */
--
