Hi Liping,
On tor, 2016-10-20 at 21:27 +0800, Liping Zhang wrote:
> Hi Anders,
>
> 2016-10-20 20:36 GMT+08:00 Anders K. Pedersen | Cohaesio <akp@cohaesi
> o.com>:
> [...]
> >
> > Well, the suggested userspace implementation doesn't allow a rule
> > that
> > would hit this warning. It is required to use 'ether type ip ... rt
> > ip
> > nexthop' or 'ether type ip6 ... rt ip6 nexthop', when nexthop is
> > used
> > from the inet table.
> >
> > So I found it reasonable to generate a warning if such a rule
> > somehow
> > got configured anyway, but if you still feel that it should be
> > removed,
> > I can do that.
>
> I think WARN_ON was used to hint that maybe there's a bug in kernel.
> It is not appropriate to warn the user's wrong configuration.
OK, so would it be okay to replace it with
printk_once(KERN_WARNING KBUILD_MODNAME " Address families do not match\n");
?
> >
> > >
> > > switch (priv->key) {
> > > case NFT_RT_NEXTHOP:
> > > switch (pkt->pf)
> > > ...
> > > default:
> > > return nft_rt_get_eval(expr, regs, pkt);
> > > }
> >
> > My thinking was that for future rt sub-expressions, it will
> > probably
> > make sense to move
> >
> > const struct rtable *rt = skb_rtable(skb);
> >
> > if (!rt)
> > goto err;
> >
> > outside the inner switch statement, so it doesn't need to be
> > duplicated
> > for each sub-expression. And ditto for the IPV6 part.
> >
>
> If so, for future rt sub-expressions extension, I think the following
> codes
> maybe better?
>
> switch (pkt->pf)
> case IPV4:
> nft_rt_ipv4_get_eval();
> break;
> case IPV6:
> nft_rt_ipv6_get_eval();
> break;
>
> This can reduce the duplicate codes. Just a suggestion:)
Then NFT_RT_INET would have to select NFT_RT_IPV4 and NFT_RT_IPV6, but
I guess that is okay, since NFT_RT_INET depends on NF_TABLES_INET,
which selects NF_TABLES_IPV4 and NF_TABLES_IPV6, so I'll use this
solution.
Regards,
Anders K. Pedersen
��.n��������+%������w��{.n����z�����n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
