Re: [PATCH v2 2/3] net/sfc: fix non-constant expression inr RTE_BUILD_BUG_ON()

[Stephen Hemminger]

On Mon, 13 Nov 2023 14:28:55 -0800
Tyler Retzlaff <roret...@linux.microsoft.com> wrote:

> On Mon, Nov 13, 2023 at 02:13:26PM -0800, Stephen Hemminger wrote:
> > On Mon, 13 Nov 2023 09:06:04 -0800
> > Stephen Hemminger <step...@networkplumber.org> wrote:
> >   
> > > The macro RTE_MIN has some hidden assignments to provide type
> > > safety which means the statement can not be fully evaluted in
> > > first pass of compiler. Replace RTE_MIN() with equivalent macro.
> > > 
> > > This will cause errors from checkpatch about multiple evaluations
> > > of same expression in macro but it is ok in this case.
> > > 
> > > Fixes: 4f936666d790 ("net/sfc: support TSO for EF100 native datapath")
> > > Cc: ivan.ma...@oktetlabs.ru
> > > Signed-off-by: Stephen Hemminger <step...@networkplumber.org>  
> > 
> > Building with clang finds another issue.
> > ../drivers/net/sfc/sfc_rx.c:158:3: error: expected expression
> >                 RTE_BUILD_BUG_ON(RTE_MBUF_F_RX_IP_CKSUM_UNKNOWN != 0);
> > yet
> > lib/mbuf/rte_mbuf_core.h:#define RTE_MBUF_F_RX_IP_CKSUM_UNKNOWN 0  
> 
> curious. do you have the gcc -E / clang -E preprocessed output for the
> expansion? wonder what it looks like.

Building with clang and -E.

This code:
        switch (desc_flags & (EFX_PKT_IPV4 | EFX_CKSUM_IPV4)) {
        case (EFX_PKT_IPV4 | EFX_CKSUM_IPV4):
                mbuf_flags |= RTE_MBUF_F_RX_IP_CKSUM_GOOD;
                break;
        case EFX_PKT_IPV4:
                mbuf_flags |= RTE_MBUF_F_RX_IP_CKSUM_BAD;
                break;
        default:
                RTE_BUILD_BUG_ON(RTE_MBUF_F_RX_IP_CKSUM_UNKNOWN != 0);
                SFC_ASSERT((mbuf_flags & RTE_MBUF_F_RX_IP_CKSUM_MASK) ==
                           RTE_MBUF_F_RX_IP_CKSUM_UNKNOWN);
                break;
        }

Becomes:
 switch (desc_flags & (0x0800 | 0x0040)) {
 case (0x0800 | 0x0040):
  mbuf_flags |= (1ULL << 7);
  break;
 case 0x0800:
  mbuf_flags |= (1ULL << 4);
  break;
 default:
  _Static_assert(!(0 != 0), "RTE_MBUF_F_RX_IP_CKSUM_UNKNOWN != 0");
  do {} while (0);

  break;
 }

Doing same with Gcc:
 switch (desc_flags & (0x0800 | 0x0040)) {
 case (0x0800 | 0x0040):
  mbuf_flags |= (1ULL << 7);
  break;
 case 0x0800:
  mbuf_flags |= (1ULL << 4);
  break;
 default:
  
# 158 "./drivers/net/sfc/sfc_rx.c" 3 4
 _Static_assert
# 158 "./drivers/net/sfc/sfc_rx.c"
 (!(0 != 0), "RTE_MBUF_F_RX_IP_CKSUM_UNKNOWN != 0");
  do {} while (0)
                                     ;