process_l3_headers_v6 in test_xdp_noinline.c

process_l3_headers_v6 in test_xdp_noinline.c

[Jose E. Marchesi]

Hello.
The following function in the BPF selftest progs/test_xdp_noinline.c:

  /* don't believe your eyes!
   * below function has 6 arguments whereas bpf and llvm allow maximum of 5
   * but since it's _static_ llvm can optimize one argument away
   */
  __attribute__ ((noinline))
  static int process_l3_headers_v6(struct packet_description *pckt,
  				 __u8 *protocol, __u64 off,
  				 __u16 *pkt_bytes, void *data,
  				 void *data_end)
  {
  	struct ipv6hdr *ip6h;
  	__u64 iph_len;
  	int action;
  
  	ip6h = data + off;
  	if (ip6h + 1 > data_end)
  		return XDP_DROP;
  	iph_len = sizeof(struct ipv6hdr);
  	*protocol = ip6h->nexthdr;
  	pckt->flow.proto = *protocol;
  	*pkt_bytes = bpf_ntohs(ip6h->payload_len);
  	off += iph_len;
  	if (*protocol == 45) {
  		return XDP_DROP;
  	} else if (*protocol == 59) {
  		action = parse_icmpv6(data, data_end, off, pckt);
  		if (action >= 0)
  			return action;
  	} else {
  		memcpy(pckt->flow.srcv6, ip6h->saddr.in6_u.u6_addr32, 16);
  		memcpy(pckt->flow.dstv6, ip6h->daddr.in6_u.u6_addr32, 16);
  	}
  	return -1;
  }

Relies, as acknowledged in the comment block, on LLVM optimizing out one
of the arguments.  As it happens GCC doesn't optimize that argument out,
and as a result it fails at compile-time when building
tst_xdp_noinline.c.

Would it be possible to rewrite this particular test to not rely on that
particular optimization?

TIA.

Re: process_l3_headers_v6 in test_xdp_noinline.c

[Alexei Starovoitov]

On Wed, Apr 24, 2024 at 5:25 AM Jose E. Marchesi
<jose.marchesi@oracle.com> wrote:
>
>
> Hello.
> The following function in the BPF selftest progs/test_xdp_noinline.c:
>
>   /* don't believe your eyes!
>    * below function has 6 arguments whereas bpf and llvm allow maximum of 5
>    * but since it's _static_ llvm can optimize one argument away
>    */
>   __attribute__ ((noinline))
>   static int process_l3_headers_v6(struct packet_description *pckt,
>                                  __u8 *protocol, __u64 off,
>                                  __u16 *pkt_bytes, void *data,
>                                  void *data_end)
>   {
>         struct ipv6hdr *ip6h;
>         __u64 iph_len;
>         int action;
>
>         ip6h = data + off;
>         if (ip6h + 1 > data_end)
>                 return XDP_DROP;
>         iph_len = sizeof(struct ipv6hdr);
>         *protocol = ip6h->nexthdr;
>         pckt->flow.proto = *protocol;
>         *pkt_bytes = bpf_ntohs(ip6h->payload_len);
>         off += iph_len;
>         if (*protocol == 45) {
>                 return XDP_DROP;
>         } else if (*protocol == 59) {
>                 action = parse_icmpv6(data, data_end, off, pckt);
>                 if (action >= 0)
>                         return action;
>         } else {
>                 memcpy(pckt->flow.srcv6, ip6h->saddr.in6_u.u6_addr32, 16);
>                 memcpy(pckt->flow.dstv6, ip6h->daddr.in6_u.u6_addr32, 16);
>         }
>         return -1;
>   }
>
> Relies, as acknowledged in the comment block, on LLVM optimizing out one
> of the arguments.  As it happens GCC doesn't optimize that argument out,
> and as a result it fails at compile-time when building
> tst_xdp_noinline.c.
>
> Would it be possible to rewrite this particular test to not rely on that
> particular optimization?

Feel free to send a patch that reduces it to 5 args.
This test was a copy paste of katran.
There was no intent to test this 6->5 llvm optimization.

Re: process_l3_headers_v6 in test_xdp_noinline.c

[Jose E. Marchesi]

> On Wed, Apr 24, 2024 at 5:25 AM Jose E. Marchesi
> <jose.marchesi@oracle.com> wrote:
>>
>>
>> Hello.
>> The following function in the BPF selftest progs/test_xdp_noinline.c:
>>
>>   /* don't believe your eyes!
>>    * below function has 6 arguments whereas bpf and llvm allow maximum of 5
>>    * but since it's _static_ llvm can optimize one argument away
>>    */
>>   __attribute__ ((noinline))
>>   static int process_l3_headers_v6(struct packet_description *pckt,
>>                                  __u8 *protocol, __u64 off,
>>                                  __u16 *pkt_bytes, void *data,
>>                                  void *data_end)
>>   {
>>         struct ipv6hdr *ip6h;
>>         __u64 iph_len;
>>         int action;
>>
>>         ip6h = data + off;
>>         if (ip6h + 1 > data_end)
>>                 return XDP_DROP;
>>         iph_len = sizeof(struct ipv6hdr);
>>         *protocol = ip6h->nexthdr;
>>         pckt->flow.proto = *protocol;
>>         *pkt_bytes = bpf_ntohs(ip6h->payload_len);
>>         off += iph_len;
>>         if (*protocol == 45) {
>>                 return XDP_DROP;
>>         } else if (*protocol == 59) {
>>                 action = parse_icmpv6(data, data_end, off, pckt);
>>                 if (action >= 0)
>>                         return action;
>>         } else {
>>                 memcpy(pckt->flow.srcv6, ip6h->saddr.in6_u.u6_addr32, 16);
>>                 memcpy(pckt->flow.dstv6, ip6h->daddr.in6_u.u6_addr32, 16);
>>         }
>>         return -1;
>>   }
>>
>> Relies, as acknowledged in the comment block, on LLVM optimizing out one
>> of the arguments.  As it happens GCC doesn't optimize that argument out,
>> and as a result it fails at compile-time when building
>> tst_xdp_noinline.c.
>>
>> Would it be possible to rewrite this particular test to not rely on that
>> particular optimization?
>
> Feel free to send a patch that reduces it to 5 args.
> This test was a copy paste of katran.
> There was no intent to test this 6->5 llvm optimization.

Ok perfect.  Will do.  Thanks!