On Fri, May 31, 2019 at 03:35:58PM -0400, Stephen Suryaputra wrote:
> On Fri, May 31, 2019 at 07:11:01PM +0200, Pablo Neira Ayuso wrote:
> > > +/* find the offset to specified option or the header beyond the options
> > > + * if target < 0.
> > > + *
> > > + * Note that *offset is used as input/output parameter, and if it is not zero,
> > > + * then it must be a valid offset to an inner IPv4 header. This can be used
> > > + * to explore inner IPv4 header, eg. ICMP error messages.
> >
> > In other extension headers (IPv6 and TCP options) this offset is used
> > to match for a field inside the extension / option.
> >
> > So this semantics you describe here are slightly different, right?
>
> It is the same as the IPv6 one. The offset returned is the offset to the
> specific option (target) or the byte beyond the options if the target
> isn't specified (< 0).
Thanks for explaining. So you are using ipv6_find_hdr() as reference,
but not sure this offset parameter is useful for this patchset since
this is always set to zero, do you have plans to use this in a follow
up patchset?
[...]
> > > + if (tb[NFTA_EXTHDR_DREG])
> > > + return &nft_exthdr_ipv4_ops;
> > > + }
> > > + break;
> >
> > Then, from the _eval() path:
> >
> > You have to replace iph->version == 4 check abive, you could use
> > skb->protocol instead, and check for htons(ETH_P_IP) packets.
>
> A bit lost. Did you mean this?
>
> static int ipv4_find_option(struct net *net, struct sk_buff *skb,
> » » » unsigned int *offset, int target,
> » » » unsigned short *fragoff, int *flags)
> {
> ...
> » iph = skb_header_pointer(skb, *offset, sizeof(_iph), &_iph);
> » if (!iph || skb->protocol != htons(ETH_P_IP))
> » » return -EBADMSG;
I mean, you make this check upfront from the _eval() path, ie.
static void nft_exthdr_ipv4_eval(const struct nft_expr *expr,
...
{
...
if (skb->protocol != htons(ETH_P_IP))
goto err;
Thanks.
