Alexei Starovoitov wrote: > On Wed, Aug 26, 2020 at 3:06 PM John Fastabend <john.fastabend@xxxxxxxxx> wrote: > > > > It is a hold-out from when we went from having a 32-bit var-off > > and a 64-bit var-off. I'll send a patch its clumsy and not needed > > for sure. > > please follow up with such patches. Great will go ahead and do this. > > > The other subtle piece here we should clean up. Its possible > > to have a const in the subreg but a non-const in the wider > > 64-bit reg. In this case we skip marking the 32-bit subreg > > as known and rely on the 64-bit case to handle it. But, we > > may if the 64-bit reg is not const fall through and update > > the 64-bit bounds. Then later we call __update_reg32_bounds() > > and this will use the var_off, previously updated. The > > 32-bit bounds are then updated using this var_off so they > > are correct even if less precise than we might expect. I > > believe xor is correct here as well. > > makes sense. I think it's correct now, but I agree that cleaning > this up would be good as well. > > > I need to send another patch with a comment for the BTF_ID > > types. I'll add some test cases for this 64-bit non-const and > > subreg const so we don't break it later. I'm on the fence > > if we should tighten the bounds there as well. I'll see if > > it helps readability to do explicit 32-bit const handling > > there. I had it in one of the early series with the 32-bit > > bounds handling, but dropped for what we have now. > > Not following. Why is this related to btf_id ? Its not related at all. I was just looking at patches on my stack here and I have unrelated comment only patch to clarify PTR_TO_BTF_ID_OR_NULL and PTR_TO_BTF_ID. > > > LGTM, but I see a couple follow up patches with tests, comments, > > and dropping the duplicate ALU op I'll try to do those Friday, unless > > someone else does them first. > > yes. please :) will do. > > I've pushed this set to bpf-next in the meantime. > Thanks everyone!
