-
Notifications
You must be signed in to change notification settings - Fork 23
infra: export interface statistics through DPDK telemetry socket #234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ac6fb8d to
b2b133a
Compare
|
I'm thinking about this a bit more and the interface statistics should be attached to interfaces themselves. Otherwise statistics will be displayed in multiple locations. What do you think about splitting this in multiple steps:
What do you think? |
f4e7732 to
e12d289
Compare
09bc9e2 to
7178082
Compare
|
@rjarry I agree with your suggestion, please have a look at what I have now. |
rjarry
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you split that in multiple commits to follow the specs I gave earlier? Also, you forgot to update the grout API to expose the per-iface statistics.
|
Thanks for the review @rjarry, I have incorporated the changes. I'm working getting another commit for grout API. Will mark the PR as WIP. |
85dd5e1 to
1082673
Compare
|
@rjarry this is now ready for review. Please have a look. Thanks |
rjarry
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to reset all interface statistics when the user calls clear stats. Also, Make sure to reset the stats for a given interface when creating a new one.
rjarry
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Spoorthi, thanks for the update. We're almost there :)
Could you squash the last commit (infra: clear interface stats) with the one that adds interface stats (infra: implement per interface statistics)? No need to have them separate.
Thanks!
| while ((iface = iface_next(GR_IFACE_TYPE_PORT, iface)) != NULL) { | ||
| struct iface_info_port *port = (struct iface_info_port *)iface->info; | ||
| if ((ret = rte_eth_stats_reset(port->port_id)) < 0) | ||
| return api_out(-ret, 0); | ||
| if ((ret = rte_eth_xstats_reset(port->port_id)) < 0) | ||
| return api_out(-ret, 0); | ||
|
|
||
| while ((iface = iface_next(GR_IFACE_TYPE_UNDEF, iface)) != NULL) { | ||
| struct iface_stats *sw_stats = iface_get_stats(iface->id); | ||
| // Reset software stats for all interface types. | ||
| if (sw_stats != NULL) { | ||
| memset(sw_stats, 0, sizeof(*sw_stats)); | ||
| } | ||
|
|
||
| if (iface->type == GR_IFACE_TYPE_PORT) { | ||
| struct iface_info_port *port = (struct iface_info_port *)iface->info; | ||
| if ((ret = rte_eth_stats_reset(port->port_id)) < 0) | ||
| return api_out(-ret, 0); | ||
| if ((ret = rte_eth_xstats_reset(port->port_id)) < 0) | ||
| return api_out(-ret, 0); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was squashed into the wrong commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic to clear the stats is part of "infra: add API and CLI command to obtain and clear iface stats" commit - d5f812f, I think we are good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really. The statistics already exist in the first commit infra: implement per interface statistics even if they are not exposed in the API yet.
You should implement the reset of these statistics in this commit. Not the one that exposes the stats in the API.
rjarry
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, could you reset stats when a new interface is created?
| s.rx_bytes = 0; | ||
| s.tx_packets = 0; | ||
| s.tx_bytes = 0; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rjarry I'm already resetting the stats per iface_id before obtaining the stats. Isn't it enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean resetting the internal structure.
grout/modules/infra/control/iface.c
Line 175 in 9f716b3
| static struct iface_stats stats[MAX_IFACES]; |
So after this line:
grout/modules/infra/control/iface.c
Line 91 in 9f716b3
| ifaces[ifid] = iface; |
You need to add:
memset(&stats[ifid], 0, sizeof(stats[ifid]));So that new interfaces are created with all their statistics reset to 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh thanks, added this part in latest change.
3ab7143#diff-0b2d0fe2e3aef9dc99b65af53ee4d4ac8a618dc8b3f57fdd8b9a450a223fed2cR95
| while ((iface = iface_next(GR_IFACE_TYPE_PORT, iface)) != NULL) { | ||
| struct iface_info_port *port = (struct iface_info_port *)iface->info; | ||
| if ((ret = rte_eth_stats_reset(port->port_id)) < 0) | ||
| return api_out(-ret, 0); | ||
| if ((ret = rte_eth_xstats_reset(port->port_id)) < 0) | ||
| return api_out(-ret, 0); | ||
|
|
||
| while ((iface = iface_next(GR_IFACE_TYPE_UNDEF, iface)) != NULL) { | ||
| struct iface_stats *sw_stats = iface_get_stats(iface->id); | ||
| // Reset software stats for all interface types. | ||
| if (sw_stats != NULL) { | ||
| memset(sw_stats, 0, sizeof(*sw_stats)); | ||
| } | ||
|
|
||
| if (iface->type == GR_IFACE_TYPE_PORT) { | ||
| struct iface_info_port *port = (struct iface_info_port *)iface->info; | ||
| if ((ret = rte_eth_stats_reset(port->port_id)) < 0) | ||
| return api_out(-ret, 0); | ||
| if ((ret = rte_eth_xstats_reset(port->port_id)) < 0) | ||
| return api_out(-ret, 0); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic to clear the stats is part of "infra: add API and CLI command to obtain and clear iface stats" commit - d5f812f, I think we are good?
Collect statistics per interface and later expose it via DPDK telemetry and grout API. Signed-off-by: Spoorthi K <[email protected]>
Output snippet:
--> /grout/iface
{
"/grout/iface": {
"p0": {
"name": "p0",
"id": 1,
"type": "port",
"mtu": 1500,
"flags": [
"up",
"running"
],
"mode": "l3",
"vrf_id": 0,
"statistics": {
"rx_packets": 3,
"rx_bytes": 1169,
"tx_packets": 3,
"tx_bytes": 150
"rx_missed": 0,
"tx_errors": 0,
"rx_good_packets": 3,
"tx_good_packets": 3,
"rx_good_bytes": 1169,
"tx_good_bytes": 180,
"rx_multicast_packets": 3,
"rx_broadcast_packets": 1,
"rx_unknown_protocol_packets": 4,
"tx_unicast_packets": 3,
"mac_remote_errors": 1,
"rx_size_256_to_511_packets": 4,
"tx_size_64_packets": 3
}
}
}
}
Signed-off-by: Spoorthi K <[email protected]>
Display interface statistics (rx_packets, rx_bytes, tx_packets and tx_bytes) per interface using 'show interface stats' grcli command. With clear stats, reset the these interface statistics along with software and hardware stats. Sample output: grout# show interface <return> Validate command. type Show interface details. stats Show interface statistics. name Show interface details. grout# show interface stats INTERFACE RX_PACKETS RX_BYTES TX_PACKETS TX_BYTES p0 7 2757 2 100 gr-loop0 0 0 0 0 grout# clear stats grout# show interface stats INTERFACE RX_PACKETS RX_BYTES TX_PACKETS TX_BYTES p0 0 0 0 0 gr-loop0 0 0 0 0 Signed-off-by: Spoorthi K <[email protected]>
show interface statsclear stats