Hi Florian, > -----Original Message----- > From: Florian Westphal <fw@xxxxxxxxx> > Sent: Friday, 24 February 2023 00:02 > To: Sriram Yagnaraman <sriram.yagnaraman@xxxxxxxx> > Cc: netfilter-devel@xxxxxxxxxxxxxxx; Florian Westphal <fw@xxxxxxxxx>; Pablo > Neira Ayuso <pablo@xxxxxxxxxxxxx> > Subject: Re: [PATCH nf-next] netfilter: bridge: introduce broute meta > statement > > Sriram Yagnaraman <sriram.yagnaraman@xxxxxxxx> 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? > > > + 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?