Subject: Re: Take #3 - final proposed patch for ipsec/bpf/ipfilter integration
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Darren Reed <avalon@caligula.anu.edu.au>
List: tech-net
Date: 05/14/2003 11:37:20
In some mail from YAMAMOTO Takashi, sie said:
> 
> hi,
> 
> > Hi All,
> > 
> > At this pass on the ipsec/tcpdump/ipfilter problem, I think I'm
> > approaching something very close to 100% working.  I'd really
> > like some feedback from someone at KAME on the changes below as
> > well as from people who can test it or just review the patches.
> > I've also not yet tested IPv6, so if someone could give that a
> > go, it'd be most welcome.
> 
> sorry for a dumb question;
> i can't understand why you want to introduce such a 'virtual interface' hack.
> what you need for ipf is just additional filtering points(pfil_head), isn't
> it?

It is not just for ipf, but that is one of the two beneficaries.
The other is BPF (tcpdump.)

There are a couple of problems with just adding more filtering points
with pfil.  The first is that for input traffic, the packets are processed
twice by the same code, already, so it wouldn't be an extra pfil callout
but rather some mechanism to do it with.  The second major problem is you
now have to invent a way to do filteirng on those packets in a
distinguishable manner.  By creating a virtual interface, this is
achieved without needing any changes to ipfilter or anything else
that wants to get these packets - they are simply associated with
a different interface.

> > ***************
> > *** 608,614 ****
> >   				if (pr->pr_usrreq != NULL) {
> >   					(void) (*pr->pr_usrreq)(&so,
> >   					    PRU_PURGEIF, NULL, NULL,
> > ! 					    (struct mbuf *) ifp, curproc);
> >   					purged = 1;
> >   				}
> >   			}
> > --- 664,670 ----
> >   				if (pr->pr_usrreq != NULL) {
> >   					(void) (*pr->pr_usrreq)(&so,
> >   					    PRU_PURGEIF, NULL, NULL,
> > ! 					    (struct mbuf *) ifp, curlwp);
> >   					purged = 1;
> >   				}
> >   			}
> 
> why?

That patch is an "oops!" and is from me hacking on ktrace/lwp stuff.

> > Index: sys/net/if.h
> > ===================================================================
> > RCS file: /cvsroot/src/sys/net/if.h,v
> > retrieving revision 1.88
> > diff -c -r1.88 if.h
> > *** sys/net/if.h	2003/04/30 18:50:26	1.88
> > --- sys/net/if.h	2003/05/11 07:17:26
> > ***************
> > *** 292,297 ****
> > --- 292,298 ----
> >   
> >   	void	*if_afdata[AF_MAX];
> >   	struct	mowner *if_mowner;	/* who owns mbufs for this interface */
> > + 	struct ifnet	*if_ipsec;
> >   };
> >   #define	if_mtu		if_data.ifi_mtu
> >   #define	if_type		if_data.ifi_type
> 
> adding more protocol-specific member into struct ifnet seems a bad idea.

Well, I'll ask you the same question I asked Itojun - do you have a better
solution to "this problem" ?  I'm willing to listen to ideas others have
but so far, after much frustration to many people, there would appear to
have been no better idea ?  Consider that there are two targets here that
we want to fix the problem for - tcpdump et al & ipfilter.  Solving the
problem for ipfilter with fancy pfil stuff is not going to work for
tcpdump.

Cheers,
Darren