RE: [PATCH nf-next] netfilter: xt_socket: add XT_SOCKET_MATCHSOCKMARK flag and mark fields

["Harout Hedeshian" <harouth@codeaurora.org>]

> -----Original Message-----
> From: Pablo Neira Ayuso [mailto:pablo@netfilter.org]
> Sent: Monday, June 15, 2015 2:58 PM
> To: Harout Hedeshian
> Cc: netfilter-devel@vger.kernel.org; 'Lorenzo Colitti'
> Subject: Re: [PATCH nf-next] netfilter: xt_socket: add
> XT_SOCKET_MATCHSOCKMARK flag and mark fields
> 
> >
> > Actually, on second thought, I'm not so sure. This socket lookup is
> > happening as part of a match operation in xt_socket.c.
> > From x_tables.h, I can see that match functions are not supposed to
> > modify the skb:
> >
> > struct xt_match{
> > ...
> > bool (*match)(const struct sk_buff *skb, struct xt_action_param *);
> >
> > In that case, would this even be valid?
> > -m socket --transparent --restore-skmark
> >
> > Keeping in mind --restore-skmark is happening as part of -m socket
> >
> > I would think we would need a whole new target to handle something
> > like
> > this:
> > iptables -t mangle -I PREROUTING -m socket --transparent --no-wildcard
> > -j SOCKET --restore-skmark
> >
> > Since the target invocation would be separate, we would need a second
> > socket lookup? Seems perhaps a little overkill...
> 
> Yes, it's simply overkill to add a new target to add this.
> 
> So just cast it:
Yuck...

>         struct sk_buff *pskb = (struct sk_buff *)skb;
> 
> iptables matches were originally design not to modify anything, but it
> can lead us to situations like this. That's one of the reasons why we
> have no distinction between matches and targets in nftables anymore.

That makes sense.
> Thanks.

These 2 MATCHSOCKMARK patches should be replaced with:
[PATCH nf-next] netfilter: xt_socket: add XT_SOCKET_RESTORESKMARK flag
[PATCH iptables] xt_socket: add --restore-skmark option

Thanks,
Harout