From: Sven Eckelmann <sven@narfation.org>
To: b.a.t.m.a.n@lists.open-mesh.org
Cc: "Linus Lüssing" <linus.luessing@c0d3.blue>,
"David Bauer" <mail@david-bauer.net>
Subject: Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Add multicast-to-unicast support for multiple targets
Date: Tue, 29 Jan 2019 16:42:56 +0100 [thread overview]
Message-ID: <2115997.7JKGhZffWe@bentobox> (raw)
In-Reply-To: <20190119061422.32335-1-linus.luessing@c0d3.blue>
[-- Attachment #1: Type: text/plain, Size: 1608 bytes --]
On Saturday, 19 January 2019 07.14.22 CET Linus Lüssing wrote:
[...]
> + ret = batadv_send_skb_unicast(bat_priv, newskb,
> + BATADV_UNICAST, 0,
> + orig_entry->orig_node, vid);
> +
> + if (ret != NET_XMIT_SUCCESS) {
> + /* use kfree_skb() to signalize losses here, but keep
> + * trying other destinations
> + */
> + kfree_skb(newskb);
> + ret = NET_XMIT_SUCCESS;
> + }
[...]
> + ret = batadv_send_skb_unicast(bat_priv, newskb,
> + BATADV_UNICAST, 0,
> + orig_node, vid);
> +
> + if (ret != NET_XMIT_SUCCESS) {
> + /* use kfree_skb() to signalize losses here, but keep
> + * trying other destinations
> + */
> + kfree_skb(newskb);
> + ret = NET_XMIT_SUCCESS;
> + }
As you already wrote in your other messages - the send skb send functions
(like also in other parts of the kernel) consume the skb. The caller must
therefore not free the skb again.
Which also means that you should rewrite the patch to behave the same and not
leak skbs. You most likely already partially do this this in [1]. But we have
to check this in detail in the new version.
One point where it looks "wrong" (I've seen the comment) is
batadv_mcast_forw_want_all_send -> default case. Not sure what to do about
batadv_mcast_forw_tt_send + batadv_mcast_forw_want_all_send which follow after
each other in batadv_mcast_forw_send. Even when the function should have
consumed the actual skb.
Kind regards,
Sven
[1] https://github.com/freifunk-gluon/gluon/pull/1357/commits/a973442fc9efd28a8b2ccc4bef65bcab981d0b20
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
prev parent reply other threads:[~2019-01-29 15:42 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-19 6:14 [B.A.T.M.A.N.] [PATCH] batman-adv: Add multicast-to-unicast support for multiple targets Linus Lüssing
2019-01-19 9:07 ` Sven Eckelmann
2019-01-19 13:49 ` Martin Weinelt
2019-01-29 15:42 ` Sven Eckelmann [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2115997.7JKGhZffWe@bentobox \
--to=sven@narfation.org \
--cc=b.a.t.m.a.n@lists.open-mesh.org \
--cc=linus.luessing@c0d3.blue \
--cc=mail@david-bauer.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).