Re: [PATCH net-next 1/2] net: netfilter: export xt_policy match_policy_in() as xt_policy_match_policy_in()

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Sun, Jan 14, 2018 at 02:47:46PM +0200, Eyal Birger wrote:
> On Fri, Jan 12, 2018 at 4:00 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Fri, Jan 12, 2018 at 03:56:21PM +0200, Eyal Birger wrote:
> >> On Fri, Jan 12, 2018 at 3:41 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >> > On Fri, Jan 12, 2018 at 02:57:24PM +0200, Eyal Birger wrote:
> >> >> @@ -51,9 +52,9 @@ match_xfrm_state(const struct xfrm_state *x, const struct xt_policy_elem *e,
> >> >>              MATCH(reqid, x->props.reqid);
> >> >>  }
> >> >>
> >> >> -static int
> >> >> -match_policy_in(const struct sk_buff *skb, const struct xt_policy_info *info,
> >> >> -             unsigned short family)
> >> >> +int xt_policy_match_policy_in(const struct sk_buff *skb,
> >> >> +                           const struct xt_policy_info *info,
> >> >> +                           unsigned short family)
> >> >>  {
> >> >>       const struct xt_policy_elem *e;
> >> >>       const struct sec_path *sp = skb->sp;
> >> >> @@ -80,10 +81,11 @@ match_policy_in(const struct sk_buff *skb, const struct xt_policy_info *info,
> >> >>
> >> >>       return strict ? 1 : 0;
> >> >>  }
> >> >> +EXPORT_SYMBOL_GPL(xt_policy_match_policy_in);
> >> >
> >> > If you just want to call xt_policy_match from tc, then you could use
> >> > tc ipt infrastructure instead.
> >>
> >> Thanks for the suggestion -
> >> Are you referring to act_ipt? it looks like it allows calling targets;
> >> I couldn't find a classifier calling a netfilter matcher.
> >
> > Then, I'd suggest you extend that infrastructure to alllow to call
> > matches, so we reduce the number of interdepencies between different
> > subsystems.
> 
> This appears very versatile. though in this case the use of the xtables code and
> structures was done in order to avoid introducing new uapi structures
> and supporting
> match code, not necessarily to expose the full capabilities of extended matches,
> similar in spirit to what was done in the em_ipset ematch.
> 
> Perhaps in order to avoid the direct export of xt_policy code, I could call
> xt_request_find_match() from the em_policy module, requesting the
> xt_policy match?
> this way api exposure is minimized while not overly complicating the
> scope of this feature.
> 
> What do you think?

That would look better indeed.

But once you call xt_request_find_match() from there, how far is to
allow any arbitrary match? I think you only have to specify the match
name, family and the binary layout structure that represents
xt_policy, right?

I'm telling this, because I think it would be fair enough to me if you
add the generic infrastructure to the kernel to allow arbitrary load
of xt matches, and then from userspace you just add the code to
support this which is what you need.

Probably someone else - not you - may follow up later on to generalize
the userspace codebase to support other matches, by when that happens,
the right bits will be in the kernel already.