| Message ID | 1479688779-1328-1-git-send-email-fgao@ikuai8.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Montag, 21. November 2016 08:39:39 CET fgao@ikuai8.com wrote: > From: Gao Feng <fgao@ikuai8.com> > > The tc could return NET_XMIT_CN as one congestion notification, but > it does not mean the packe is lost. Other modules like ipvlan, s/packe/packet/ > macvlan, and others treat NET_XMIT_CN as success too. > > So batman-adv should add the NET_XMIT_CN check. > > Signed-off-by: Gao Feng <fgao@ikuai8.com> > --- > net/batman-adv/distributed-arp-table.c | 2 +- > net/batman-adv/fragmentation.c | 2 +- > net/batman-adv/routing.c | 2 +- > net/batman-adv/tp_meter.c | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) [...] Not sure how the batman-adv maintainers see this - but this patch needs an additional patch for net-next to also add it to the parts which were rewritten. Kind regards, Sven
Hi Sven, On Mon, Nov 21, 2016 at 4:16 PM, Sven Eckelmann <sven@narfation.org> wrote: > On Montag, 21. November 2016 08:39:39 CET fgao@ikuai8.com wrote: >> From: Gao Feng <fgao@ikuai8.com> >> >> The tc could return NET_XMIT_CN as one congestion notification, but >> it does not mean the packe is lost. Other modules like ipvlan, > > s/packe/packet/ What's this mean? > >> macvlan, and others treat NET_XMIT_CN as success too. >> >> So batman-adv should add the NET_XMIT_CN check. >> >> Signed-off-by: Gao Feng <fgao@ikuai8.com> >> --- >> net/batman-adv/distributed-arp-table.c | 2 +- >> net/batman-adv/fragmentation.c | 2 +- >> net/batman-adv/routing.c | 2 +- >> net/batman-adv/tp_meter.c | 2 +- >> 4 files changed, 4 insertions(+), 4 deletions(-) > [...] > > Not sure how the batman-adv maintainers see this - but this patch needs > an additional patch for net-next to also add it to the parts which were > rewritten. > > Kind regards, > Sven Ok. I would commit another patch to net-next. Best Regards Feng
On Montag, 21. November 2016 16:21:52 CET Feng Gao wrote: > Hi Sven, > > On Mon, Nov 21, 2016 at 4:16 PM, Sven Eckelmann <sven@narfation.org> wrote: > > On Montag, 21. November 2016 08:39:39 CET fgao@ikuai8.com wrote: > >> From: Gao Feng <fgao@ikuai8.com> > >> > >> The tc could return NET_XMIT_CN as one congestion notification, but > >> it does not mean the packe is lost. Other modules like ipvlan, > > > > s/packe/packet/ > > What's this mean? That there is a minor typo (*t* is missing) and this sed statement (when applied only to the commit message) would fix it. David already marked this patch as "Under Review" in his patchwork. So I would guess that he will accept this patch and not the batman-adv maintainers. And maybe he will fix this small typo - or maybe not. Kind regards, Sven
Hi Sven, On Mon, Nov 21, 2016 at 4:31 PM, Sven Eckelmann <sven@narfation.org> wrote: > On Montag, 21. November 2016 16:21:52 CET Feng Gao wrote: >> Hi Sven, >> >> On Mon, Nov 21, 2016 at 4:16 PM, Sven Eckelmann <sven@narfation.org> wrote: >> > On Montag, 21. November 2016 08:39:39 CET fgao@ikuai8.com wrote: >> >> From: Gao Feng <fgao@ikuai8.com> >> >> >> >> The tc could return NET_XMIT_CN as one congestion notification, but >> >> it does not mean the packe is lost. Other modules like ipvlan, >> > >> > s/packe/packet/ >> >> What's this mean? > > That there is a minor typo (*t* is missing) and this sed statement (when > applied only to the commit message) would fix it. Thanks. I didn't thought it was sed statement. > > David already marked this patch as "Under Review" in his patchwork. So I would > guess that he will accept this patch and not the batman-adv maintainers. And > maybe he will fix this small typo - or maybe not. > > Kind regards, > Sven I would correct the typo in the patch for net-next. Best Regards Feng
Hello. On 11/21/2016 3:39 AM, fgao@ikuai8.com wrote: > From: Gao Feng <fgao@ikuai8.com> > > The tc could return NET_XMIT_CN as one congestion notification, but > it does not mean the packe is lost. Other modules like ipvlan, Packet. > macvlan, and others treat NET_XMIT_CN as success too. > > So batman-adv should add the NET_XMIT_CN check. > > Signed-off-by: Gao Feng <fgao@ikuai8.com> [...] > diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c > index 7e8dc64..8edd324 100644 > --- a/net/batman-adv/routing.c > +++ b/net/batman-adv/routing.c > @@ -706,7 +706,7 @@ static int batadv_route_unicast_packet(struct sk_buff *skb, > goto out; > > /* translate transmit result into receive result */ > - if (res == NET_XMIT_SUCCESS) { > + if (res == NET_XMIT_SUCCESS || ret == NET_XMIT_CN) { Not 'res == NET_XMIT_CN'? > /* skb was transmitted and consumed */ > batadv_inc_counter(bat_priv, BATADV_CNT_FORWARD); > batadv_add_counter(bat_priv, BATADV_CNT_FORWARD_BYTES, [...] MBR, Sergei
Hi Sergei, On Mon, Nov 21, 2016 at 6:44 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > Hello. > > On 11/21/2016 3:39 AM, fgao@ikuai8.com wrote: > >> From: Gao Feng <fgao@ikuai8.com> >> >> The tc could return NET_XMIT_CN as one congestion notification, but >> it does not mean the packe is lost. Other modules like ipvlan, > > > Packet. Thanks, it was typo. > >> macvlan, and others treat NET_XMIT_CN as success too. >> >> So batman-adv should add the NET_XMIT_CN check. >> >> Signed-off-by: Gao Feng <fgao@ikuai8.com> > > > [...] > >> diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c >> index 7e8dc64..8edd324 100644 >> --- a/net/batman-adv/routing.c >> +++ b/net/batman-adv/routing.c >> @@ -706,7 +706,7 @@ static int batadv_route_unicast_packet(struct sk_buff >> *skb, >> goto out; >> >> /* translate transmit result into receive result */ >> - if (res == NET_XMIT_SUCCESS) { >> + if (res == NET_XMIT_SUCCESS || ret == NET_XMIT_CN) { Thanks again. I didn't find it during myself's review and compile process. > > > Not 'res == NET_XMIT_CN'? > >> /* skb was transmitted and consumed */ >> batadv_inc_counter(bat_priv, BATADV_CNT_FORWARD); >> batadv_add_counter(bat_priv, BATADV_CNT_FORWARD_BYTES, > > [...] > > MBR, Sergei > I have sent the v2 patch which corrects these two typos. Best Regards Feng
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c index e257efd..4bf0622 100644 --- a/net/batman-adv/distributed-arp-table.c +++ b/net/batman-adv/distributed-arp-table.c @@ -660,7 +660,7 @@ static bool batadv_dat_send_data(struct batadv_priv *bat_priv, } send_status = batadv_send_unicast_skb(tmp_skb, neigh_node); - if (send_status == NET_XMIT_SUCCESS) { + if (send_status == NET_XMIT_SUCCESS || send_status == NET_XMIT_CN) { /* count the sent packet */ switch (packet_subtype) { case BATADV_P_DAT_DHT_GET: diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c index 0934730..4714b8f 100644 --- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -495,7 +495,7 @@ int batadv_frag_send_packet(struct sk_buff *skb, batadv_add_counter(bat_priv, BATADV_CNT_FRAG_TX_BYTES, skb_fragment->len + ETH_HLEN); ret = batadv_send_unicast_skb(skb_fragment, neigh_node); - if (ret != NET_XMIT_SUCCESS) { + if (ret != NET_XMIT_SUCCESS && ret != NET_XMIT_CN) { /* return -1 so that the caller can free the original * skb */ diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index 7e8dc64..8edd324 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -706,7 +706,7 @@ static int batadv_route_unicast_packet(struct sk_buff *skb, goto out; /* translate transmit result into receive result */ - if (res == NET_XMIT_SUCCESS) { + if (res == NET_XMIT_SUCCESS || ret == NET_XMIT_CN) { /* skb was transmitted and consumed */ batadv_inc_counter(bat_priv, BATADV_CNT_FORWARD); batadv_add_counter(bat_priv, BATADV_CNT_FORWARD_BYTES, diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c index 8af1611..461dbad 100644 --- a/net/batman-adv/tp_meter.c +++ b/net/batman-adv/tp_meter.c @@ -618,7 +618,7 @@ static int batadv_tp_send_msg(struct batadv_tp_vars *tp_vars, const u8 *src, if (r == -1) kfree_skb(skb); - if (r == NET_XMIT_SUCCESS) + if (r == NET_XMIT_SUCCESS || r == NET_XMIT_CN) return 0; return BATADV_TP_REASON_CANT_SEND;