Re: [PATCH nf-next] netfilter: bridge: introduce broute meta statement

[Florian Westphal <fw@strlen.de>]

Sriram Yagnaraman <sriram.yagnaraman@est.tech> wrote:
> > -----Original Message-----
> > From: Florian Westphal <fw@strlen.de>
> > Sent: Friday, 24 February 2023 00:02
> > To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> > Cc: netfilter-devel@vger.kernel.org; Florian Westphal <fw@strlen.de>; Pablo
> > Neira Ayuso <pablo@netfilter.org>
> > Subject: Re: [PATCH nf-next] netfilter: bridge: introduce broute meta
> > statement
> > 
> > Sriram Yagnaraman <sriram.yagnaraman@est.tech> wrote:
> > > +void nft_meta_bridge_set_eval(const struct nft_expr *expr,
> > > +			      struct nft_regs *regs,
> > > +			      const struct nft_pktinfo *pkt)
> > 
> > static?
> > 
> > > +{
> > > +		dest = eth_hdr(skb)->h_dest;
> > > +		if (skb->pkt_type == PACKET_HOST &&
> > > +		    !ether_addr_equal(skb->dev->dev_addr, dest) &&
> > > +		    ether_addr_equal(p->br->dev->dev_addr, dest))
> > > +			skb->pkt_type = PACKET_OTHERHOST;
> > 
> > We already support override of skb->pkt_type, I would prefer if users to this
> > explicitly from their ruleset if they need it.
> 
> Ok, that is better, I will remove this chunk.
> 
> > 
> > > +	priv->key = ntohl(nla_get_be32(tb[NFTA_META_KEY]));
> > 
> > I think you need to check for !tb[NFTA_META_KEY] and bail out before this
> > line.
> 
> We already validate this in nft_meta_bridge_select_ops(), isn’t that enough?

Right, thats enough.

> > > +	switch (priv->key) {
> > > +	case NFT_META_BRI_BROUTE:
> > > +		len = sizeof(u8);
> > > +		break;
> > 
> > Can you bail out if this is called from something else than PREROUTING hook?
> > 
> > You can look at nft_tproxy.c or similar on how to do this.
> 
> nft_meta_set_validate() already checks meta statements can only be used in the PREROUTING hook. Isn't that enough?

It only restricts NFT_META_PKTTYPE.