Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxxxxx> wrote:
> > > @@ -74,11 +77,11 @@ void nft_byteorder_eval(const struct nft_expr *expr,
> > > switch (priv->op) {
> > > case NFT_BYTEORDER_NTOH:
> > > for (i = 0; i < priv->len / 2; i++)
> > > - d[i].u16 = ntohs((__force __be16)s[i].u16);
> > > + d16[i] = ntohs((__force __be16)s16[i]);
> >
> > This on the other hand... I'd say this should mimic what the 64bit
> > case is doing and use nft_reg_store16() nft_reg_load16() helpers for
> > the register accesses.
> >
> > something like:
> >
> > for (i = 0; i < priv->len / 2; i++) {
> > v16 = nft_reg_load16(&src[i]);
> > nft_reg_store16(&dst[i], + ntohs((__force __be16)v16));
> > }
> >
>
> The problem here is that we cannot index the 32-bit dst and src pointers as if
> they were 16-bit pointers. We will end up with the exact same problem we are
> trying to fix here.
>
> I can change the code to use the accessors, but they use u32 pointers, so it
> would end up looking like:
>
> case NFT_BYTEORDER_NTOH:
> for (i = 0; i < priv->len / 4; i++)
> - d[i].u32 = ntohl((__force __be32)s[i].u32);
> + dst[i] = ntohl((__force __be32)src[i]);
> break;
> case NFT_BYTEORDER_HTON:
> for (i = 0; i < priv->len / 4; i++)
> - d[i].u32 = (__force __u32)htonl(s[i].u32);
> + dst[i] = (__force __u32)htonl(src[i]);
Ack, thanks.
> case NFT_BYTEORDER_NTOH:
> - for (i = 0; i < priv->len / 2; i++)
> - d[i].u16 = ntohs((__force __be16)s[i].u16);
> + for (i = 0; i < priv->len / 2; i++) {
> + __be16 src16;
> + src16 = nft_reg_load_be16((u32 *)&s16[i]);
> + nft_reg_store_be16((u32 *)&d16[i], ntohs(src16));
> + }
These accessors take a registers' address, not something in-between.
I think your original was better after all and we need to rely on whatever
expression filled the register to have done the right thing.
