types: add gws field to Route for ECMP support - #1190
Conversation
yushoyamaguchi
changed the title
|
coverage: 63.352% (+0.03%) from 63.325% — yushoyamaguchi:ecmp1 into containernetworking:main |
cf01bf6 to
f241c80
Compare
yushoyamaguchi
changed the title
| - `routes`: Routes created by this attachment: | ||
| - `dst`: The destination of the route, in CIDR notation | ||
| - `gw`: The next hop address. If unset, a value in `gateway` in the `ips` array may be used. | ||
| - `gws` (list of strings, optional): A list of next hop addresses for ECMP (Equal-Cost Multi-Path) routing. Each entry is an IP address string. If both `gw` and `gws` are set, `gw` is treated as an additional next hop appended after the entries in `gws`. If only `gw` is set, behavior is unchanged from routes without `gws`. |
There was a problem hiding this comment.
In case of gw and gws case, usually plugin author tend to contain all ECMP routes into gws, not split into gw + gws. In addition, the consumer of Result object need to add gw and gws for all route inforation and it could be (a bit) burden. Hence I would like to know the background of this design. Could you please let me know why you design it?
In such gw and gws case, at least to me, I would like to
- use
gwsfor ECMP supported Result consumer - use
gwfor non-ECMP supported Result consumer (i.e. plugin just returns one of them)
There was a problem hiding this comment.
@s1061123
As you said, I think that it is NOT desirable to write nexthops in both gw and gws fields, too.
(I think we should write this to SPEC, so I'll add the description after this discussion.)
In the community MTG, we talk about what we should do when user define both gw and gws.
However, we talk about only two option.
- Ignore
gw. - Merge
gwtogws
and we choose merging way.
Do you think plugins should return ERROR in such case ?
Otherwise, do you think we don't need to mention such case in the SPEC?
There was a problem hiding this comment.
@s1061123
I've fixed as you described in last MTG.
- Add an explanation to
SPEC.mdthatgwsshould be used instead ofgwwhen configuring ECMP routes. - Remove the description of how to handle cases where both
gwandgwsare defined, since this behavior should not be defined in the spec (It will be decided when implementing inplugins).
| - `routes`: Routes created by this attachment: | ||
| - `dst`: The destination of the route, in CIDR notation | ||
| - `gw`: The next hop address. If unset, a value in `gateway` in the `ips` array may be used. | ||
| - `gws` (list of strings, optional): A list of next hop addresses for ECMP (Equal-Cost Multi-Path) routing. Each entry is an IP address string. If both `gw` and `gws` are set, `gw` is treated as an additional next hop appended after the entries in `gws`. If only `gw` is set, behavior is unchanged from routes without `gws`. |
There was a problem hiding this comment.
Specification document needs to specify what is required and what should NOT. In this case, how about to contain same IP address in gws? Is that still valid?
Add a GWs []net.IP field to the Route struct to enable ECMP (Equal-Cost Multi-Path) routing via multiple next hops. When both GW and GWs are set, GW is treated as an additional next hop appended after GWs. The NextHops() method returns the combined list for use by plugin implementations. Also document the gws field and its merge behavior in SPEC.md. Signed-off-by: Yusho Yamaguchi <ys-yamaguchi@kddi.com>
4 participants
Add a GWs []net.IP field to the Route struct to enable ECMP (Equal-Cost Multi-Path) routing via multiple next hops.
When both GW and GWs are set, GW is treated as an additional next hop appended after GWs. The NextHops() method returns the combined list for use by plugin implementations.
Also document the gws field and its merge behavior in SPEC.md.