On 5 Jan 2023, at 2:52, wangchuanlei wrote:
> Add support to count upall packets, when kmod of openvswitch upcall to > count the number of packets for upcall succeed and failed, which is a > better way to see how many packets upcalled on every interfaces. > > Signed-off-by: wangchuanlei <[email protected]> The code works, and I see no other problems, so I’m ok to ACK it. However, before I do, I do think we should get some feedback on how the stats are displayed. This is what it looks like now: port 0: my (internal) RX packets:10 errors:0 dropped:0 overruns:0 frame:0 upcall success:1 upcall fail:0 TX packets:0 errors:0 dropped:0 aborted:0 carrier:0 collisions:0 RX bytes:1230 TX bytes:0 It’s a bit confusing with the space in the name. Should we maybe separate upcall stats completely? Something like: port 0: my (internal) RX packets:10 errors:0 dropped:0 overruns:0 frame:0 TX packets:0 errors:0 dropped:0 aborted:0 carrier:0 collisions:0 RX bytes:1230 TX bytes:0 UPCALL packets:1 failed:0 Also, note I used ‘packets’ and ‘failed’ to be more in line with existing stats. And maybe not even display it when the feature is not supported? Anyone any thoughts!? > --- > > Since v1: > - Patch of add support > Since v2: > - Modify format of code > > include/linux/openvswitch.h | 14 ++++++++++++++ > include/openvswitch/netdev.h | 4 ++++ > lib/dpctl.c | 4 ++++ > lib/dpif-netlink.c | 17 +++++++++++++++++ > lib/dpif-netlink.h | 2 ++ > lib/netdev-linux.c | 24 +++++++++++++----------- > 6 files changed, 54 insertions(+), 11 deletions(-) > > diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h > index 8bb5abdc8..26a6af9a3 100644 > --- a/include/linux/openvswitch.h > +++ b/include/linux/openvswitch.h > @@ -301,11 +301,25 @@ enum ovs_vport_attr { > OVS_VPORT_ATTR_PAD, > OVS_VPORT_ATTR_IFINDEX, > OVS_VPORT_ATTR_NETNSID, > + OVS_VPORT_ATTR_UPCALL_STATS, > __OVS_VPORT_ATTR_MAX > }; > > #define OVS_VPORT_ATTR_MAX (__OVS_VPORT_ATTR_MAX - 1) > > +/** > +* enum ovs_vport_upcall_attr -- attributes for %OVS_VPORT_UPCALL* commands > +* @OVS_VPORT_UPCALL_ATTR_SUCCESS: 64-bit upcall success packets. > +* @OVS_VPORT_UPCALL_ATTR_FAIL: 64-bit upcall fail packets. > +*/ > +enum ovs_vport_upcall_attr { > + OVS_VPORT_UPCALL_ATTR_SUCCESS, > + OVS_VPORT_UPCALL_ATTR_FAIL, > + __OVS_VPORT_UPCALL_ATTR_MAX, > +}; > + > +#define OVS_VPORT_UPCALL_ATTR_MAX (__OVS_VPORT_UPCALL_ATTR_MAX - 1) > + > enum { > OVS_VXLAN_EXT_UNSPEC, > OVS_VXLAN_EXT_GBP, > diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h > index 0c10f7b48..585ff069c 100644 > --- a/include/openvswitch/netdev.h > +++ b/include/openvswitch/netdev.h > @@ -87,6 +87,10 @@ struct netdev_stats { > uint64_t rx_oversize_errors; > uint64_t rx_fragmented_errors; > uint64_t rx_jabber_errors; > + > + /* Datapath upcall statistics */ > + uint64_t upcall_success; /* Rx packets forwarded to userspace. */ > + uint64_t upcall_fail; /* Rx packets failed forwarding to userspace. */ > }; > > /* Structure representation of custom statistics counter */ > diff --git a/lib/dpctl.c b/lib/dpctl.c > index 29041fa3e..7ce235dc4 100644 > --- a/lib/dpctl.c > +++ b/lib/dpctl.c > @@ -734,6 +734,10 @@ show_dpif(struct dpif *dpif, struct dpctl_params > *dpctl_p) > print_stat(dpctl_p, " frame:", s.rx_frame_errors); > dpctl_print(dpctl_p, "\n"); > > + print_stat(dpctl_p, " upcall success:", s.upcall_success); > + print_stat(dpctl_p, " upcall fail:", s.upcall_fail); > + dpctl_print(dpctl_p, "\n"); > + > print_stat(dpctl_p, " TX packets:", s.tx_packets); > print_stat(dpctl_p, " errors:", s.tx_errors); > print_stat(dpctl_p, " dropped:", s.tx_dropped); > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c > index 026b0daa8..586fb8893 100644 > --- a/lib/dpif-netlink.c > +++ b/lib/dpif-netlink.c > @@ -4685,6 +4685,8 @@ dpif_netlink_vport_from_ofpbuf(struct > dpif_netlink_vport *vport, > .optional = true }, > [OVS_VPORT_ATTR_OPTIONS] = { .type = NL_A_NESTED, .optional = true }, > [OVS_VPORT_ATTR_NETNSID] = { .type = NL_A_U32, .optional = true }, > + [OVS_VPORT_ATTR_UPCALL_STATS] = { .type = NL_A_NESTED, > + .optional = true }, > }; > > dpif_netlink_vport_init(vport); > @@ -4716,6 +4718,21 @@ dpif_netlink_vport_from_ofpbuf(struct > dpif_netlink_vport *vport, > if (a[OVS_VPORT_ATTR_STATS]) { > vport->stats = nl_attr_get(a[OVS_VPORT_ATTR_STATS]); > } > + if (a[OVS_VPORT_ATTR_UPCALL_STATS]) { > + const struct nlattr *nla; > + size_t left; > + > + NL_NESTED_FOR_EACH (nla, left, a[OVS_VPORT_ATTR_UPCALL_STATS]) { > + if (nl_attr_type(nla) == OVS_VPORT_UPCALL_ATTR_SUCCESS) { > + vport->upcall_success = nl_attr_get_u64(nla); > + } else if (nl_attr_type(nla) == OVS_VPORT_UPCALL_ATTR_FAIL) { > + vport->upcall_fail = nl_attr_get_u64(nla); > + } > + } > + } else { > + vport->upcall_success = UINT64_MAX; > + vport->upcall_fail = UINT64_MAX; > + } > if (a[OVS_VPORT_ATTR_OPTIONS]) { > vport->options = nl_attr_get(a[OVS_VPORT_ATTR_OPTIONS]); > vport->options_len = nl_attr_get_size(a[OVS_VPORT_ATTR_OPTIONS]); > diff --git a/lib/dpif-netlink.h b/lib/dpif-netlink.h > index 24294bc42..4909fe160 100644 > --- a/lib/dpif-netlink.h > +++ b/lib/dpif-netlink.h > @@ -44,6 +44,8 @@ struct dpif_netlink_vport { > uint32_t n_upcall_pids; > const uint32_t *upcall_pids; /* OVS_VPORT_ATTR_UPCALL_PID. */ > const struct ovs_vport_stats *stats; /* OVS_VPORT_ATTR_STATS. */ > + uint64_t upcall_success; /* OVS_VPORT_UPCALL_ATTR_SUCCESS. > */ > + uint64_t upcall_fail; /* OVS_VPORT_UPCALL_ATTR_FAIL. */ > const struct nlattr *options; /* OVS_VPORT_ATTR_OPTIONS. */ > size_t options_len; > }; > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index 59e8dc0ae..6f86b0c01 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -2156,16 +2156,16 @@ swap_uint64(uint64_t *a, uint64_t *b) > * 'src' is allowed to be misaligned. */ > static void > netdev_stats_from_ovs_vport_stats(struct netdev_stats *dst, > - const struct ovs_vport_stats *src) > -{ > - dst->rx_packets = get_32aligned_u64(&src->rx_packets); > - dst->tx_packets = get_32aligned_u64(&src->tx_packets); > - dst->rx_bytes = get_32aligned_u64(&src->rx_bytes); > - dst->tx_bytes = get_32aligned_u64(&src->tx_bytes); > - dst->rx_errors = get_32aligned_u64(&src->rx_errors); > - dst->tx_errors = get_32aligned_u64(&src->tx_errors); > - dst->rx_dropped = get_32aligned_u64(&src->rx_dropped); > - dst->tx_dropped = get_32aligned_u64(&src->tx_dropped); > + struct dpif_netlink_vport *vport) > +{ > + dst->rx_packets = get_32aligned_u64(&vport->stats->rx_packets); > + dst->tx_packets = get_32aligned_u64(&vport->stats->tx_packets); > + dst->rx_bytes = get_32aligned_u64(&vport->stats->rx_bytes); > + dst->tx_bytes = get_32aligned_u64(&vport->stats->tx_bytes); > + dst->rx_errors = get_32aligned_u64(&vport->stats->rx_errors); > + dst->tx_errors = get_32aligned_u64(&vport->stats->tx_errors); > + dst->rx_dropped = get_32aligned_u64(&vport->stats->rx_dropped); > + dst->tx_dropped = get_32aligned_u64(&vport->stats->tx_dropped); > dst->multicast = 0; > dst->collisions = 0; > dst->rx_length_errors = 0; > @@ -2179,6 +2179,8 @@ netdev_stats_from_ovs_vport_stats(struct netdev_stats > *dst, > dst->tx_fifo_errors = 0; > dst->tx_heartbeat_errors = 0; > dst->tx_window_errors = 0; > + dst->upcall_success = vport->upcall_success; > + dst->upcall_fail = vport->upcall_fail; > } > > static int > @@ -2196,7 +2198,7 @@ get_stats_via_vport__(const struct netdev *netdev, > struct netdev_stats *stats) > return EOPNOTSUPP; > } > > - netdev_stats_from_ovs_vport_stats(stats, reply.stats); > + netdev_stats_from_ovs_vport_stats(stats, &reply); > > ofpbuf_delete(buf); > > -- > 2.27.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
