On Wed, Jan 27, 2021 at 11:14:51AM +1000, David Gwynne wrote:
> On Wed, Jan 27, 2021 at 11:13:12AM +1000, David Gwynne wrote:
> > when pf_route (and pf_route6) are supposed to handle forwarding the
> > packet (ie, for route-to or reply-to rules), they take the mbuf
> > away from the calling code path. this is done by clearing the mbuf
> > pointer in the pf_pdesc struct. it doesn't do this for dup-to rules
> > though.
> >
> > at the moment pf_route clears that pointer on the way out, but it could
> > take the mbuf away up front in the same place that it already checks if
> > it's a dup-to rule or not.
> >
> > it's a small change. i've bumped up the number of lines of context so
> > it's easier to read too.
> >
> > ok?
>
> sigh. here's the diff with the extra context.
Usually you want all information including the mbuf in pd.
Here it does not matter, pd->m is not accessed.
I see no advantage in changing, so I would leave it as it is.
If others think new code is better, I have no objections.
bluhm
> Index: pf.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.1101
> diff -u -p -U8 -r1.1101 pf.c
> --- pf.c 19 Jan 2021 22:22:23 -0000 1.1101
> +++ pf.c 27 Jan 2021 01:10:52 -0000
> @@ -5983,16 +5983,17 @@ pf_route(struct pf_pdesc *pd, struct pf_
>
> if (r->rt == PF_DUPTO) {
> if ((m0 = m_dup_pkt(pd->m, max_linkhdr, M_NOWAIT)) == NULL)
> return;
> } else {
> if ((r->rt == PF_REPLYTO) == (r->direction == pd->dir))
> return;
> m0 = pd->m;
> + pd->m = NULL;
> }
>
> if (m0->m_len < sizeof(struct ip)) {
> DPFPRINTF(LOG_ERR,
> "%s: m0->m_len < sizeof(struct ip)", __func__);
> goto bad;
> }
>
> @@ -6103,18 +6104,16 @@ pf_route(struct pf_pdesc *pd, struct pf_
> else
> m_freem(m0);
> }
>
> if (error == 0)
> ipstat_inc(ips_fragmented);
>
> done:
> - if (r->rt != PF_DUPTO)
> - pd->m = NULL;
> rtfree(rt);
> return;
>
> bad:
> m_freem(m0);
> goto done;
> }
>
> @@ -6141,16 +6140,17 @@ pf_route6(struct pf_pdesc *pd, struct pf
>
> if (r->rt == PF_DUPTO) {
> if ((m0 = m_dup_pkt(pd->m, max_linkhdr, M_NOWAIT)) == NULL)
> return;
> } else {
> if ((r->rt == PF_REPLYTO) == (r->direction == pd->dir))
> return;
> m0 = pd->m;
> + pd->m = NULL;
> }
>
> if (m0->m_len < sizeof(struct ip6_hdr)) {
> DPFPRINTF(LOG_ERR,
> "%s: m0->m_len < sizeof(struct ip6_hdr)", __func__);
> goto bad;
> }
> ip6 = mtod(m0, struct ip6_hdr *);
> @@ -6232,18 +6232,16 @@ pf_route6(struct pf_pdesc *pd, struct pf
> ip6stat_inc(ip6s_cantfrag);
> if (r->rt != PF_DUPTO)
> pf_send_icmp(m0, ICMP6_PACKET_TOO_BIG, 0,
> ifp->if_mtu, pd->af, r, pd->rdomain);
> goto bad;
> }
>
> done:
> - if (r->rt != PF_DUPTO)
> - pd->m = NULL;
> rtfree(rt);
> return;
>
> bad:
> m_freem(m0);
> goto done;
> }
> #endif /* INET6 */
