new mbuf flag proposal

[Tom Judge]

Oleg Bulyzhin wrote:
> Hi all.
> 
> Recently, i discovered following problem (though it was already discussed, see
> http://freebsd.rambler.ru/bsdmail/freebsd-ipfw_2006/msg00491.html):
> pfil handlers (like ipfw or pf) sometime need to create packets (like tcp rst
> or icmp errors). In order to avoid loops M_SKIP_FIREWALL flag is used.
> Unfortunately, this behaviour is not always correct.
> There are configurations when you need to reinject such packets into pfil(4)
> handlers (in order to translate them using NAT or apply routing policy 
> or divert them somewhere, etc). In my case i had to modify kernel
> in order to translate tcp keepalive packets(generated by ipfw) using pfnat.
> 
> I have a proposal how to solve this:
> 1) Introduce new mbuf flag, something like M_PFIL_CREATED, which should be
>    used to mark packets created by pfil handler. If packet is not supposed
>    to reenter pfil handlers M_SKIP_FIREWALL can be used instead.
> 2) When pfil handler generate packet it should be marked either with
>    M_SKIP_FIREWALL or M_PFIL_CREATED. In latter case, pfil handler should add
>    mbuf_tag for distinguishing source of M_PFIL_CREATED flag.
> 

I only really have one comment,  surely all packets created in pfil 
handlers should have M_PFIL_CREATED set, and those that should not pass 
through the firewall should have M_SKIP_FIREWALL set in addition?

Just my 2p

Tom

> So, for packet creation code should be like this:
> 
> 	m->m_flags |= M_PFIL_CREATED;
> 	mtag = m_tag_alloc(MTAG_PFIL_CREATED, PFIL_IPFW, 0, M_NOWAIT);
> 	if (mtag) {
> 		m_tag_prepend(m, mtag);
> 	} else {
> 		goto drop_pkt;
> 	}
> 
> at the beginning of pfil handler we should have something like this:
> 
> 	int dont_emit_pkt = 0;
> 
> 	if (m->m_flags & M_PFIL_CREATED) {
> 		dont_emit_pkt = 1;
> 		mtag = m_tag_locate(m, MTAG_PFIL_CREATED, PFIL_IPFW, NULL);
> 		if (mtag) {	/* pkt was created by myself */
> 			/* my own packet, handle it with care. */
> 			goto specal_handler;
> 		} else {	/* pkt was created by other pfil(4) handler */
> 
> 			/* do normal processing but do not emit new packets. */
> 			goto normal_handler;
> 		}
> 	}
> 
> This functionality can be archived with mbuf_tag only (without new mbuf flag),
> but it would be ineffective:
> calling m_tag_locate() (unsuccessful most of the time!) for every packet is
> rather expensive.
> 
> What do you think about this?
>