The fallout is a bit worse than first anticipated.

I converted my code to use the vl_api_*_t_endian functions rather than doing 
the hton*/ntoh* by hand (since a bunch of them were now doing the wrong thing), 
and discovered another bigger problem. Unions are not handled correctly by the 
generated endian functions e.g.,:

static inline void vl_api_punt_t_endian (vl_api_punt_t *a)
{
    int i __attribute__((unused));
    vl_api_punt_type_t_endian(&a->type);
    vl_api_punt_union_t_endian(&a->punt);
}

static inline void vl_api_punt_union_t_endian (vl_api_punt_union_t *a)
{
    int i __attribute__((unused));
    vl_api_punt_exception_t_endian(&a->exception);
    vl_api_punt_l4_t_endian(&a->l4);
    vl_api_punt_ip_proto_t_endian(&a->ip_proto);
}

The second function there is flipping bits in the same memory space (union) 
multiple times in different ways according to each structure in the union.

Anything that has a union in the API will suffer this same bug; the recent enum 
size changes to very common types (address family, ip proto, etc) merely cause 
this bug to be hit more ofetn.

Thanks,
Chris.

> On May 2, 2020, at 8:20 AM, Christian Hopps <cho...@chopps.org> wrote:
> 
> Some recent changes related to packed enums in the API have broken already 
> written code for low level API use. I think the main commit that did this was 
> the one that added __packed__ to the generated enum declaration, but there 
> are the related changes that add the enum size declarations I guess. Those 
> appear to be percolating slowly (e.g., in ip_types.api), although version 
> numbers are not being changed inline (e.g., "enum address_family" went from 
> default u32 to u8 but the ip_types.api version didn't change)
> 
> I understand that the API needs to be able to change, but I'm using like 
> maybe 7 or 8 API calls and I now have conditional code for seemingly every 
> release since 19.04. It's frustrating.
> 
> The project encourages running the latest code by moving support away from 
> old releases quickly, but then is making it hard to follow with API churn.
> 
> That said I would not be being bitten by this particular change I think if I 
> were using the higher level API; however, I have resisted this b/c that API 
> with synchronous calls requiring a callback for results is ... inelegant, to 
> put it nicely. :)
> 
> Thanks,
> Chris.

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#16222): https://lists.fd.io/g/vpp-dev/message/16222
Mute This Topic: https://lists.fd.io/mt/73940014/21656
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to