Anders Persson wrote:
The ill_ip_mib is allocated/freed where it was previously done for
ill_ip6_mib. I am not sure I follow the comment about naming, could you
elaborate?
Sorry, I answered the naming part as I read more of the diffs.
ip.c (and elsewhere)
6103 BUMP_MIB(&ip_mib,
ipIfStatsInUnknownProtos);
6104 BUMP_MIB(ill->ill_ip_mib,
6105 ipIfStatsInUnknownProtos);
I think you must switch to only incrementing one MIB in each place; in
the vast majority of cases this will be the ill mib. Thus there should
never be a need to update the ill mib and the global mb in the same
place; use the IPv6 code paths as the example.
Then when handling a mib_get (and kstat update if there are kstats for
these) you can add the per-ill mib and the global mib together into a
separate sum which is what is returned as the global stats.
If there is code that sometimes have an ill and others not, instead of
doing this:
7536 BUMP_MIB(&ip_mib, ipIfStatsOutDiscards);
7537 if (ill != NULL)
7538 BUMP_MIB(ill->ill_ip_mib,
ipIfStatsOutDiscards);
you should do
if (ill != NULL)
BUMP_MIB(ill->ill_ip_mib, ...);
else
BUMP_MIB(&ip_mib, ...);
if there are functions that do lots of those, it might make sense to
have a local 'mibptr' variable set up front based on ill != NULL.
I believe that Rao have some concerns on taking this path.
Rao, can you explain your concerns?
Doing an add loop in ip_snmp_get is in the noise compared to the total
cost of an application doing a mib_get.
ip.c:
Doesn't the caller of ip_proto_not_sup() know the ill so it can pass it
in as an additional argument?
6095 /* Get ill from index in ipsec_in_t. */
6096 ill = ill_lookup_on_ifindex(ii->ipsec_in_ill_index,
6097 (IPH_HDR_VERSION(ipha) == IPV6_VERSION), NULL, NULL,
NULL, NULL);
I am not sure if the ILL that the caller knows of will always be the
same as the one obtain by the lookup (although it might not matter). I
will look into it more and make the change if possible.
OK
ip.c: I don't see why you need these lines; you've already added
pkt_len to the MIB earlier and the fact that the packet was padded by
Ethernet isn't important.
14705 UPDATE_MIB(&ip_mib,
ipIfStatsHCInOctets, len);
14706 UPDATE_MIB(ill->ill_ip_mib,
ipIfStatsHCInOctets,
14707 len);
and
14719 UPDATE_MIB(&ip_mib,
ipIfStatsHCInOctets, len);
14720 UPDATE_MIB(ill->ill_ip_mib,
ipIfStatsHCInOctets,
14721 len);
I was thinking about the case when the length reported in the header is
wrong for some reason.
Yes, but in that case you should still use the length in the header.
If the packet is shorter than the length in the header, then the packet
will be discarded (and counted as such); no need to adjust the length in
the MIB.
And if the packet is longer than the length in the header (e.g. for an
Ethernet packet that is padded to 60 bytes), then the MIB should count
the length without the padding.
Erik
_______________________________________________
networking-discuss mailing list
[email protected]
