Hi Anders,
2016-10-20 2:41 GMT+08:00 Anders K. Pedersen | Cohaesio <akp@xxxxxxxxxxxx>:
> +static void nft_rt_inet_get_eval(const struct nft_expr *expr,
> + struct nft_regs *regs,
> + const struct nft_pktinfo *pkt)
> +{
> + const struct nft_rt *priv = nft_expr_priv(expr);
> + const struct sk_buff *skb = pkt->skb;
> + u32 *dest = ®s->data[priv->dreg];
> +
> + if (unlikely(pkt->pf != priv->family)) {
> + WARN_ONCE(1, "Address families do not match\n");
I think this WARN_ONCE seems unnecessary, in inet family, it's possible that
the family is mismatched.
> + goto err;
> + }
> +
> + switch (pkt->pf) {
> + case NFPROTO_IPV4:
> + switch (priv->key) {
> + case NFT_RT_NEXTHOP: {
> + const struct rtable *rt = skb_rtable(skb);
> +
> + if (!rt)
> + goto err;
> + *dest = rt_nexthop(rt, ip_hdr(skb)->daddr);
> + break;
> + }
> + default:
> + goto out;
> + }
> + break;
> + case NFPROTO_IPV6:
> + switch (priv->key) {
> + case NFT_RT_NEXTHOP: {
> + struct rt6_info *rt =
> + (struct rt6_info *)skb_dst(skb);
> +
> + if (!rt)
> + goto err;
> + memcpy(dest, rt6_nexthop(rt, &ipv6_hdr(skb)->daddr),
> + sizeof(struct in6_addr));
> + break;
> + }
> + default:
> + goto out;
> + }
> + break;
> + }
> +
This code looks not clean, I think it can be converted to such:
switch (priv->key) {
case NFT_RT_NEXTHOP:
switch (pkt->pf)
...
default:
return nft_rt_get_eval(expr, regs, pkt);
}
> + return;
> +out:
> + return nft_rt_get_eval(expr, regs, pkt);
> +err:
> + regs->verdict.code = NFT_BREAK;
> +}
> +
> +static int nft_rt_inet_get_init(const struct nft_ctx *ctx,
> + const struct nft_expr *expr,
> + const struct nlattr * const tb[])
> +{
> + struct nft_rt *priv = nft_expr_priv(expr);
> + unsigned int len;
> +
> + if (tb[NFTA_RT_KEY] == NULL ||
> + tb[NFTA_RT_DREG] == NULL ||
> + tb[NFTA_RT_FAMILY] == NULL)
> + return -EINVAL;
> +
> + priv->key = ntohl(nla_get_be32(tb[NFTA_RT_KEY]));
> + priv->family = ntohl(nla_get_be32(tb[NFTA_RT_FAMILY]));
> + switch (priv->key) {
> + case NFT_RT_NEXTHOP:
> + switch (priv->family) {
> + case NFPROTO_IPV4:
> + len = sizeof(u32);
> + break;
> + case NFPROTO_IPV6:
> + len = sizeof(struct in6_addr);
> + break;
> + }
> + len = sizeof(u32);
> + break;
If the family is invalid, you should report -EINVAL to the userspace.
And the code "len = sizeof(u32);" here makes no sense.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
