> > > Also, I'm curious there are still a few places under net/ using: > > > > fl4.flowi4_tos = tos & INET_DSCP_MASK; > > > > Will you consider changing all of them with > > inet_dsfield_to_dscp() && inet_dsfield_to_dscp() as well? > > Yes, I have a few more cases to convert. But some of them will have to > stay. For example, in net/ipv4/ip_output.c, __ip_queue_xmit() has > "fl4->flowi4_tos = tos & INET_DSCP_MASK;", but we can't just convert > that "tos" variable to dscp_t because it carries both DSCP and ECN > values. Although ->flowi4_tos isn't concerned with ECN, these ECN bits > are used later to set the IP header. > > There are other cases that I'm not planning to convert, for example > because the value is read from a UAPI structure that can't be updated. > For example the "fl4.flowi4_tos = params->tos & INET_DSCP_MASK;" case > in bpf_ipv4_fib_lookup(), where "params" is a struct bpf_fib_lookup, > exported in UAPI. > > To summarise, the plan is to incrementally convert most ->flowi4_tos > assignments, so that we have a dscp_t variable at hand. Then I'll send > a patch converting all ->flowi4_tos users at once. Most of it should > consist of trivial inet_dscp_to_dsfield() removals, thanks to the > previous dscp_t conversions. The cases that won't follow that pattern > will be explained in the commit message, but the idea is to have as few > of them as possible. > > BTW, the reason for this work is to avoid having ECN bits interfering > with route lookups. We had several such issues and regressions in the > past because of ->flowi4_tos having ECN bits set in specific scenarios. > Got it, thanks for the detailed explanation. Acked-by: Xin Long <lucien.xin@xxxxxxxxx>
