On Mon, Jun 12, 2023 at 4:39 AM 张祖建 <[email protected]> wrote:
>
> Reported by Valgrind:
>
> ==00:00:00:46.715 951138== 56 bytes in 1 blocks are definitely lost in loss
> record 363 of 408
> ==00:00:00:46.715 951138== at 0x4848899: malloc (in
> /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==00:00:00:46.715 951138== by 0x278E98: xmalloc__ (util.c:140)
> ==00:00:00:46.715 951138== by 0x2998C2: netem_install__ (netdev-linux.c:4365)
> ==00:00:00:46.715 951138== by 0x2998C2: netem_tc_install (netdev-linux.c:4454)
> ==00:00:00:46.715 951138== by 0x29A5E8: netdev_linux_set_qos
> (netdev-linux.c:2994)
> ==00:00:00:46.715 951138== by 0x1390AC: iface_configure_qos (bridge.c:4822)
> ==00:00:00:46.715 951138== by 0x1390AC: bridge_reconfigure (bridge.c:929)
> ==00:00:00:46.715 951138== by 0x13C3B8: bridge_run (bridge.c:3298)
> ==00:00:00:46.715 951138== by 0x131314: main (ovs-vswitchd.c:129)
> ==00:00:00:46.715 951138==
> ==00:00:00:46.715 951138== 56 bytes in 1 blocks are definitely lost in loss
> record 364 of 408
> ==00:00:00:46.715 951138== at 0x4848899: malloc (in
> /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==00:00:00:46.715 951138== by 0x278E98: xmalloc__ (util.c:140)
> ==00:00:00:46.715 951138== by 0x293688: netem_install__ (netdev-linux.c:4365)
> ==00:00:00:46.715 951138== by 0x293688: netem_qdisc_set (netdev-linux.c:4504)
> ==00:00:00:46.715 951138== by 0x29A696: netdev_linux_set_qos
> (netdev-linux.c:2982)
> ==00:00:00:46.715 951138== by 0x1390AC: iface_configure_qos (bridge.c:4822)
> ==00:00:00:46.715 951138== by 0x1390AC: bridge_reconfigure (bridge.c:929)
> ==00:00:00:46.715 951138== by 0x13C3B8: bridge_run (bridge.c:3298)
> ==00:00:00:46.715 951138== by 0x131314: main (ovs-vswitchd.c:129)
> ==00:00:00:46.715 951138==
>
> Signed-off-by: 张祖建 <[email protected]>
Good find, it looks like some but not all of the QoS .set_qos()
functions call install as well. htb_qdisc_set() doesn't, but many of
the others do.
Could we just delete those extra calls to the install functions in
netem_qdisc_set() and the other qdisc_set functions?
Thanks,
Mike
> ---
> lib/netdev-linux.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 49c74346a..43fd9ccfe 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -3764,6 +3764,13 @@ codel_install__(struct netdev *netdev_, uint32_t
> target, uint32_t limit,
> struct netdev_linux *netdev = netdev_linux_cast(netdev_);
> struct codel *codel;
>
> + if (netdev->tc) {
> + if (netdev->tc->ops->tc_destroy) {
> + netdev->tc->ops->tc_destroy(netdev->tc);
> + }
> + netdev->tc = NULL;
> + }
> +
> codel = xmalloc(sizeof *codel);
> tc_init(&codel->tc, &tc_ops_codel);
> codel->target = target;
> @@ -3975,6 +3982,13 @@ fqcodel_install__(struct netdev *netdev_, uint32_t
> target, uint32_t limit,
> struct netdev_linux *netdev = netdev_linux_cast(netdev_);
> struct fqcodel *fqcodel;
>
> + if (netdev->tc) {
> + if (netdev->tc->ops->tc_destroy) {
> + netdev->tc->ops->tc_destroy(netdev->tc);
> + }
> + netdev->tc = NULL;
> + }
> +
> fqcodel = xmalloc(sizeof *fqcodel);
> tc_init(&fqcodel->tc, &tc_ops_fqcodel);
> fqcodel->target = target;
> @@ -4197,6 +4211,13 @@ sfq_install__(struct netdev *netdev_, uint32_t
> quantum, uint32_t perturb)
> struct netdev_linux *netdev = netdev_linux_cast(netdev_);
> struct sfq *sfq;
>
> + if (netdev->tc) {
> + if (netdev->tc->ops->tc_destroy) {
> + netdev->tc->ops->tc_destroy(netdev->tc);
> + }
> + netdev->tc = NULL;
> + }
> +
> sfq = xmalloc(sizeof *sfq);
> tc_init(&sfq->tc, &tc_ops_sfq);
> sfq->perturb = perturb;
> @@ -4372,6 +4393,13 @@ netem_install__(struct netdev *netdev_, uint32_t
> latency,
> struct netdev_linux *netdev = netdev_linux_cast(netdev_);
> struct netem *netem;
>
> + if (netdev->tc) {
> + if (netdev->tc->ops->tc_destroy) {
> + netdev->tc->ops->tc_destroy(netdev->tc);
> + }
> + netdev->tc = NULL;
> + }
> +
> netem = xmalloc(sizeof *netem);
> tc_init(&netem->tc, &tc_ops_netem);
> netem->latency = latency;
> @@ -4562,6 +4590,13 @@ htb_install__(struct netdev *netdev_, uint64_t
> max_rate)
> struct netdev_linux *netdev = netdev_linux_cast(netdev_);
> struct htb *htb;
>
> + if (netdev->tc) {
> + if (netdev->tc->ops->tc_destroy) {
> + netdev->tc->ops->tc_destroy(netdev->tc);
> + }
> + netdev->tc = NULL;
> + }
> +
> htb = xmalloc(sizeof *htb);
> tc_init(&htb->tc, &tc_ops_htb);
> htb->max_rate = max_rate;
> @@ -5047,6 +5082,13 @@ hfsc_install__(struct netdev *netdev_, uint32_t
> max_rate)
> struct netdev_linux *netdev = netdev_linux_cast(netdev_);
> struct hfsc *hfsc;
>
> + if (netdev->tc) {
> + if (netdev->tc->ops->tc_destroy) {
> + netdev->tc->ops->tc_destroy(netdev->tc);
> + }
> + netdev->tc = NULL;
> + }
> +
> hfsc = xmalloc(sizeof *hfsc);
> tc_init(&hfsc->tc, &tc_ops_hfsc);
> hfsc->max_rate = max_rate;
> @@ -5518,6 +5560,13 @@ noop_install__(struct netdev *netdev_)
> struct netdev_linux *netdev = netdev_linux_cast(netdev_);
> static const struct tc tc = TC_INITIALIZER(&tc, &tc_ops_default);
>
> + if (netdev->tc) {
> + if (netdev->tc->ops->tc_destroy) {
> + netdev->tc->ops->tc_destroy(netdev->tc);
> + }
> + netdev->tc = NULL;
> + }
> +
> netdev->tc = CONST_CAST(struct tc *, &tc);
> }
>
> @@ -5553,6 +5602,13 @@ default_install__(struct netdev *netdev_)
> struct netdev_linux *netdev = netdev_linux_cast(netdev_);
> static const struct tc tc = TC_INITIALIZER(&tc, &tc_ops_default);
>
> + if (netdev->tc) {
> + if (netdev->tc->ops->tc_destroy) {
> + netdev->tc->ops->tc_destroy(netdev->tc);
> + }
> + netdev->tc = NULL;
> + }
> +
> /* Nothing but a tc class implementation is allowed to write to a tc.
> This
> * class never does that, so we can legitimately use a const tc object.
> */
> netdev->tc = CONST_CAST(struct tc *, &tc);
> --
> 2.41.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
