| Message ID | xlabdyc4izitfdfpuoy2p2hi3abiq4mrrgizdqz45k7xeftbsg@ee6jgncdaqg7 (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: copy routing table more efficiently in rt_dst_clone | expand |
On 2/18/24 6:35 AM, Oliver Crumrine wrote: > diff --git a/net/ipv4/route.c b/net/ipv4/route.c > index 16615d107cf0..ebb17c3a0dec 100644 > --- a/net/ipv4/route.c > +++ b/net/ipv4/route.c > @@ -1664,22 +1664,8 @@ struct rtable *rt_dst_clone(struct net_device *dev, struct rtable *rt) > rt->dst.flags); > > if (new_rt) { > + *new_rt = *rt; rtable is a container of dst_entry, so this is copying those fields as well. pw-bot: reject
On Tue, Feb 20, 2024 at 07:44:24PM -0700, David Ahern wrote: Hi David, You are correct that rtable is a container of dst_entry, and the previous code copied not only the fields of rtable but also the fields of dst_entry. However, the *new_rt = *rt line not only copies the fields of rtable, but also the fields of dst_entry. I have demonstrated this by generating a random number and putting it in the "expired" field of the dst_entry that is part of rt, and the printing the random number that was generated. After the copy, I printed the expired field of the dst_entry that is part of new_rt and the numbers matched. This proves that not only the fields that were part of rtable were copied but also the fields of dst_entry. Thanks, Oliver > On 2/18/24 6:35 AM, Oliver Crumrine wrote: > > diff --git a/net/ipv4/route.c b/net/ipv4/route.c > > index 16615d107cf0..ebb17c3a0dec 100644 > > --- a/net/ipv4/route.c > > +++ b/net/ipv4/route.c > > @@ -1664,22 +1664,8 @@ struct rtable *rt_dst_clone(struct net_device *dev, struct rtable *rt) > > rt->dst.flags); > > > > if (new_rt) { > > + *new_rt = *rt; > > rtable is a container of dst_entry, so this is copying those fields as well. > > pw-bot: reject >
diff --git a/net/ipv4/route.c b/net/ipv4/route.c index 16615d107cf0..ebb17c3a0dec 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -1664,22 +1664,8 @@ struct rtable *rt_dst_clone(struct net_device *dev, struct rtable *rt) rt->dst.flags); if (new_rt) { + *new_rt = *rt; new_rt->rt_genid = rt_genid_ipv4(dev_net(dev)); - new_rt->rt_flags = rt->rt_flags; - new_rt->rt_type = rt->rt_type; - new_rt->rt_is_input = rt->rt_is_input; - new_rt->rt_iif = rt->rt_iif; - new_rt->rt_pmtu = rt->rt_pmtu; - new_rt->rt_mtu_locked = rt->rt_mtu_locked; - new_rt->rt_gw_family = rt->rt_gw_family; - if (rt->rt_gw_family == AF_INET) - new_rt->rt_gw4 = rt->rt_gw4; - else if (rt->rt_gw_family == AF_INET6) - new_rt->rt_gw6 = rt->rt_gw6; - - new_rt->dst.input = rt->dst.input; - new_rt->dst.output = rt->dst.output; - new_rt->dst.error = rt->dst.error; new_rt->dst.lastuse = jiffies; new_rt->dst.lwtstate = lwtstate_get(rt->dst.lwtstate); }
Instead of copying field-by-field, copy the entire struct at once. This should lead to a performance improvment in rt_dst_clone. Signed-off-by: Oliver Crumrine <ozlinuxc@gmail.com> --- net/ipv4/route.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-)