Hi Pablo, On Wed, 14 Feb 2018 11:19:40 +0100 Pablo Neira Ayuso <pa...@netfilter.org> wrote:
> On Wed, Feb 14, 2018 at 10:14:24AM +0200, Eyal Birger wrote: > > 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. > > I think conclusion was that we're all fine. At ingress this turns into > noop and at egress there's no skb sharing at all. Anyway, see below. > > > 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. > > Then, artificially restrict this to work for xt_policy only. But > please, no new exported symbols to achieve this given you can do this > with the existing exported symbols. I mean no direct symbol > dependencies with xt_policy. > > I'm fine if you just want to expose the policy match via tc, instead > of a generic ipt match infrastructure as long as you use the existing > exported symbols. New submitted version does not expose new netfilter symbols. Thanks for your help! Eyal.