Thiru/Sowmini,
There are a number of places in Nevada where we do something like:
if ((dst_ill->ill_group != NULL) ||
--> (ire->ire_ipif->ipif_flags & IPIF_DEPRECATED) ||
(connp != NULL && ire->ire_zoneid != zoneid &&
ire->ire_zoneid != ALL_ZONES) ||
(dst_ill->ill_usesrc_ifindex != 0)) {
ipaddr_t saddr = ire->ire_ipif->ipif_src_addr;
--> src_ipif = ipif_select_source(dst_ill, saddr,
zoneid); ^^^^^^^
In particular, I thought the rule was that the ill associated with
ire->ire_ipif was used for source address selection. Indeed, you can see
on the first arrowed line that if the ire_ipif itself is not deprecated,
we often just use it directly. However, as per the second arrowed line,
in the case where we cannot (or should not, in the case of inbound
load-spreading with IPMP) just use ire_ipif, we pass *dst_ill* rather
than ire->ire_ipif->ipif_ill.
This seems wrong. However, it doesn't cause any problems for Nevada in
the IPMP case because ipif_select_source() will check if dst_ill is in a
group and if so it will scan across all of the interfaces in the group.
However, in my new design, there is a profound difference between the two:
ire->ire_ipif->ipif_ill will refer to an IPMP interface (and thus will
return a random available data address on that IPMP interface), whereas
dst_ill will refer to an underlying interface (and thus will return a test
address).
Is there any issue with changing these callers to use
ire->ire_ipif->ipif_ill instead? (Basic testing seems to suggest not.)
As an aside, it seems like making this change would simplify VNI --
currently, since ipif_select_source() passes in dst_ill rather than
ire->ire_ipif->ipif_ill, ipif_select_source() needs to re-derive the VNI
interface:
if (ill->ill_usesrc_ifindex != 0) {
usill = ill_lookup_on_ifindex(ill->ill_usesrc_ifindex,
B_FALSE, NULL, NULL, NULL, NULL, ipst);
if (usill != NULL)
ill = usill; /* Select source from usesrc ILL */
else
return (NULL);
}
With the proposed change to the ipif_select_source() callers, I believe
this code could just be removed.
Please let me know your thoughts.
--
meem
