On Tue, Jul 08, 2014 at 11:39:12PM -0400, Lawrence Teo wrote:
> The current divert(4) implementation allocates an mbuf tag in pf_test()
> to store the divert port specified by a divert-packet PF rule.
>
> The divert_packet() function then looks up that mbuf tag to retrieve the
> divert port number before sending the packet to userspace.
>
> As far as I can tell, this approach of using an mbuf tag was borrowed
> from divert-to's implementation. However, in the case of divert(4) I
> think it's overkill because once the packet has reached userspace,
> its mbuf and mbuf tag are no longer needed.
>
> I would like to simplify divert(4)'s implementation by passing the
> divert port to divert_packet() directly as an argument, which avoids the
> allocation of an mbuf tag completely.
>
> ok?
>
Nice one.
Does anyone have an idea why the mbuf tag was added in the first
place? Maybe henning's PF shuffling removed the need for it.
ok reyk@
>
> Index: net/pf.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.878
> diff -u -p -u -p -r1.878 pf.c
> --- net/pf.c 20 May 2014 11:03:13 -0000 1.878
> +++ net/pf.c 14 Jun 2014 18:12:06 -0000
> @@ -6617,14 +6617,8 @@ done:
> }
> }
>
> - if (action == PF_PASS && r->divert_packet.port) {
> - struct pf_divert *divert;
> -
> - if ((divert = pf_get_divert(pd.m)))
> - divert->port = r->divert_packet.port;
> -
> + if (action == PF_PASS && r->divert_packet.port)
> action = PF_DIVERT;
> - }
>
> if (pd.pflog) {
> struct pf_rule_item *ri;
> @@ -6651,12 +6645,12 @@ done:
> case PF_DIVERT:
> switch (pd.af) {
> case AF_INET:
> - if (divert_packet(pd.m, pd.dir) == 0)
> + if (!divert_packet(pd.m, pd.dir, r->divert_packet.port))
> *m0 = NULL;
> break;
> #ifdef INET6
> case AF_INET6:
> - if (divert6_packet(pd.m, pd.dir) == 0)
> + if (!divert6_packet(pd.m, pd.dir,
> r->divert_packet.port))
> *m0 = NULL;
> break;
> #endif /* INET6 */
> Index: netinet/ip_divert.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_divert.c,v
> retrieving revision 1.22
> diff -u -p -u -p -r1.22 ip_divert.c
> --- netinet/ip_divert.c 23 Apr 2014 14:43:14 -0000 1.22
> +++ netinet/ip_divert.c 14 Jun 2014 17:45:12 -0000
> @@ -189,12 +189,11 @@ fail:
> }
>
> int
> -divert_packet(struct mbuf *m, int dir)
> +divert_packet(struct mbuf *m, int dir, u_int16_t divert_port)
> {
> struct inpcb *inp;
> struct socket *sa = NULL;
> struct sockaddr_in addr;
> - struct pf_divert *divert;
>
> inp = NULL;
> divstat.divs_ipackets++;
> @@ -205,15 +204,8 @@ divert_packet(struct mbuf *m, int dir)
> return (0);
> }
>
> - divert = pf_find_divert(m);
> - if (divert == NULL) {
> - divstat.divs_errors++;
> - m_freem(m);
> - return (0);
> - }
> -
> TAILQ_FOREACH(inp, &divbtable.inpt_queue, inp_queue) {
> - if (inp->inp_lport != divert->port)
> + if (inp->inp_lport != divert_port)
> continue;
> if (inp->inp_divertfl == 0)
> break;
> Index: netinet/ip_divert.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_divert.h,v
> retrieving revision 1.5
> diff -u -p -u -p -r1.5 ip_divert.h
> --- netinet/ip_divert.h 23 Apr 2014 14:43:14 -0000 1.5
> +++ netinet/ip_divert.h 14 Jun 2014 17:52:05 -0000
> @@ -55,7 +55,7 @@ extern struct divstat divstat;
>
> void divert_init(void);
> void divert_input(struct mbuf *, ...);
> -int divert_packet(struct mbuf *, int);
> +int divert_packet(struct mbuf *, int, u_int16_t);
> int divert_sysctl(int *, u_int, void *, size_t *, void *, size_t);
> int divert_usrreq(struct socket *,
> int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *);
> Index: netinet6/ip6_divert.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v
> retrieving revision 1.23
> diff -u -p -u -p -r1.23 ip6_divert.c
> --- netinet6/ip6_divert.c 28 Apr 2014 15:43:04 -0000 1.23
> +++ netinet6/ip6_divert.c 14 Jun 2014 18:13:03 -0000
> @@ -188,12 +188,11 @@ fail:
> }
>
> int
> -divert6_packet(struct mbuf *m, int dir)
> +divert6_packet(struct mbuf *m, int dir, u_int16_t divert_port)
> {
> struct inpcb *inp;
> struct socket *sa = NULL;
> struct sockaddr_in6 addr;
> - struct pf_divert *divert;
>
> inp = NULL;
> div6stat.divs_ipackets++;
> @@ -204,15 +203,8 @@ divert6_packet(struct mbuf *m, int dir)
> return (0);
> }
>
> - divert = pf_find_divert(m);
> - if (divert == NULL) {
> - div6stat.divs_errors++;
> - m_freem(m);
> - return (0);
> - }
> -
> TAILQ_FOREACH(inp, &divb6table.inpt_queue, inp_queue) {
> - if (inp->inp_lport != divert->port)
> + if (inp->inp_lport != divert_port)
> continue;
> if (inp->inp_divertfl == 0)
> break;
> Index: netinet6/ip6_divert.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_divert.h,v
> retrieving revision 1.4
> diff -u -p -u -p -r1.4 ip6_divert.h
> --- netinet6/ip6_divert.h 23 Apr 2014 14:43:14 -0000 1.4
> +++ netinet6/ip6_divert.h 14 Jun 2014 18:12:21 -0000
> @@ -55,7 +55,7 @@ extern struct div6stat div6stat;
>
> void divert6_init(void);
> int divert6_input(struct mbuf **, int *, int);
> -int divert6_packet(struct mbuf *, int);
> +int divert6_packet(struct mbuf *, int, u_int16_t);
> int divert6_sysctl(int *, u_int, void *, size_t *, void *, size_t);
> int divert6_usrreq(struct socket *,
> int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *);
>
--
