Pablo Neira Ayuso <pa...@netfilter.org> wrote:
> > > + ct = nf_ct_get(entry->skb, &ctinfo);
> > > + if (ct && !nf_ct_is_confirmed(ct) &&
> > > +     verdict != NF_STOLEN && verdict != NF_DROP) {
> > 
> > Why not verdict == NF_ACCEPT?
> 
> We also have to deal with NF_STOP, right?

Indeed, right.  Your call, if you prefer this rather
than ACCEPT || STOP then keep it as-is.

> > I think failopen can use nf_reinject since we didn't queue packet to
> > userspace.
> 
> Yes, that's fine. I just used nfqnl_reinject() for consistency, so all
> code uses it.

Ok, I'm fine with that.

> > > @@ -1208,7 +1233,7 @@ static int nfqnl_recv_verdict(struct net *net, 
> > > struct sock *ctnl,
> > >   if (nfqa[NFQA_MARK])
> > >           entry->skb->mark = ntohl(nla_get_be32(nfqa[NFQA_MARK]));
> > >  
> > > - nf_reinject(entry, verdict);
> > > + nfqnl_reinject(entry, verdict);
> > 
> > I wonder if we should make nfqnl_reinject dependent on
> > nfqa[NFQA_PAYLOAD] ?
> > 
> > (i.e., should we munge payload in case userspcae already did so?)
> 
> You mean, skip this codepath if nfqa[NFQA_PAYLOAD] is set, right?
> Given this may be a userspace helper doing packet mangling?

Yes, although I guess such userspace already needs to NOTRACK things
in current kernels to prevent conntrack from acting up.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to