Hi Pablo,
On Mon, 15 Jan 2018 13:48:41 +0200
Eyal Birger <eyal.bir...@gmail.com> wrote:
> On Mon, Jan 15, 2018 at 12:57 PM, Pablo Neira Ayuso
> <pa...@netfilter.org> wrote:
> > 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
> >> <pa...@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
> >> >> <pa...@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 don't think that should be a problem. I'd need to pass the protocol
> onto the ematches .change() callbacks and get the appropriate match
> from there.
>
> > 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.
>
> I'm fine with submitting the more generic infrastructure.
> Will follow up with a new series.
Following up on this thread, I think this feature would better be
implemented utilizing xt_policy from tc instead of supporting arbitrary
xt matches.
Feedback on the generic framework ([1], [2]) revolved around the ability
to create the skb environment for running matches accessing the
skb->data.
My concern is that it would be difficult to maintain the correct
environment for any xt match, whereas it is simple to create a
designated ematch for a specific xt match - as done for ipset - which
can validate the necessary prerequisites for that xt match.
It is also simple to dynamically fetch the xt_policy match function
using xt_request_find_match() as suggested in the em_ipt submittion.
I'd very much appreciate your feedback.
Thanks,
Eyal.
[1] https://patchwork.ozlabs.org/patch/864683/
[2] https://patchwork.ozlabs.org/patch/866490/
