On 28/11/15 06:15, roopa wrote:
On 11/25/15, 8:18 AM, David Miller wrote:
From: Roopa Prabhu <ro...@cumulusnetworks.com>
Date: Tue, 24 Nov 2015 15:22:22 -0800
v4 -v5
- if kmemdup fails, modify the original route in place. This is a
corner case and only side effect is that in the remote case
of kmemdup failure, the changes will not be atomically visible
to datapath.
I really don't like this.
Either you need to make the changes appear atomic to the data path,
and you therefore must fail the operation if kmemdup() fails, or it
doesn't matter and you should just always change the route in-place.
As far as I can tell it can't possibly matter. The alive counter is
never modified by the data path, it is only tested to make a multipath
decision. Likewise it's rather harmless to send a frame or two via a
device currently going down.
But if you're convinced it matters, then is matters, and you can't
fake things when kmemdup() fails. And in that case I would recommend
that you use a two pass algorithm, one pass allocates all of the
new routes, and the second fills them in, inserts them, and frees
the old ones.
That is the only way you can unwind and fail cleanly.
And oh yeah, that's right, you can't really fail this and make the
ifdown not proceed.
So you're stuck, right?
That's why this has to be designed in a way where memory allocations
are not necessary. These notifiers aren't really designed to facilitate
situations that require resource acquisitions that can fail.
Get your point. I am not convinced that the atomic update matters much for the
transient case.
I was trying to accommodate comments i got during the review and it seemed ok
to cover both
cases by optionally doing the atomic update when we can. But, I get your
position on this.
My comment on atomic updates on v2 was referring to whether partial
updates could be seen by the forwarding code and if they did whether it
matters. I don't think it matters for nh_flags, but I think it does for
rt_nhn_alive.
Thanks,
Rob
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
