Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html