| Message ID | 1387109444-1104-4-git-send-email-christoph.paasch@uclouvain.be |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Christoph Paasch <christoph.paasch@uclouvain.be> Date: Sun, 15 Dec 2013 13:10:43 +0100 > As we now can have multiple entries per destination-IP, the "ip > tcp_metrics delete address ADDRESS" command should delete all of them. > > Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be> You have to adjust how you're doing patches #3 and #4. Create the new attribute for the source address first, then allow the source address to be specified in the deletion command. Make the source address optional in delete commands, to be compatible with existing tools, and have it mean "ANY". -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17/12/13 - 14:57:51, David Miller wrote: > From: Christoph Paasch <christoph.paasch@uclouvain.be> > Date: Sun, 15 Dec 2013 13:10:43 +0100 > > > As we now can have multiple entries per destination-IP, the "ip > > tcp_metrics delete address ADDRESS" command should delete all of them. > > > > Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be> > > You have to adjust how you're doing patches #3 and #4. > > Create the new attribute for the source address first, then > allow the source address to be specified in the deletion > command. > > Make the source address optional in delete commands, to be > compatible with existing tools, and have it mean "ANY". Thanks for the feedback, David. I will resubmit when I have the time to do the changes. Christoph -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On 17/12/13 - 14:57:51, David Miller wrote: > From: Christoph Paasch <christoph.paasch@uclouvain.be> > Date: Sun, 15 Dec 2013 13:10:43 +0100 > > As we now can have multiple entries per destination-IP, the "ip > > tcp_metrics delete address ADDRESS" command should delete all of them. > > > > Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be> > > You have to adjust how you're doing patches #3 and #4. > > Create the new attribute for the source address first, then > allow the source address to be specified in the deletion > command. > > Make the source address optional in delete commands, to be > compatible with existing tools, and have it mean "ANY". I have to come back on this one. Can you explain what you mean by "ANY"? Do you mean that if no source-IP is given in the netlink command that all entries matching the dst should be deleted or rather only one of them (and that it would be non-deterministic which one)? Because, if I delete all of them, then "ip tcp_metrics flush PREFIX" of today's iproute2 will complain, because iproute2 expects that for each entry of "ip tcp_metrics show" a delete-call must be done. But, the non-deterministic case also feels a bit odd to me. Thanks, Christoph -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Christoph Paasch <christoph.paasch@uclouvain.be> Date: Thu, 2 Jan 2014 10:18:23 +0100 > Do you mean that if no source-IP is given in the netlink command that all > entries matching the dst should be deleted or rather only one of them (and that it > would be non-deterministic which one)? You would delete all of them, it's the only way to stay compatible with the current code. > Because, if I delete all of them, then "ip tcp_metrics flush PREFIX" of > today's iproute2 will complain, because iproute2 expects that for each entry of > "ip tcp_metrics show" a delete-call must be done. > > But, the non-deterministic case also feels a bit odd to me. iproute2 should not expect that if it doesn't specify a specific source address, in fact it doesn't even know how to currently. I can't think of any other reasonable behavior. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c index de32aa41a846..8d544bb475dc 100644 --- a/net/ipv4/tcp_metrics.c +++ b/net/ipv4/tcp_metrics.c @@ -976,7 +976,7 @@ static int tcp_metrics_flush_all(struct net *net) static int tcp_metrics_nl_cmd_del(struct sk_buff *skb, struct genl_info *info) { struct tcpm_hash_bucket *hb; - struct tcp_metrics_block *tm; + struct tcp_metrics_block *tm, *tmlist = NULL; struct tcp_metrics_block __rcu **pp; struct inetpeer_addr daddr; unsigned int hash; @@ -993,17 +993,22 @@ static int tcp_metrics_nl_cmd_del(struct sk_buff *skb, struct genl_info *info) hb = net->ipv4.tcp_metrics_hash + hash; pp = &hb->chain; spin_lock_bh(&tcp_metrics_lock); - for (tm = deref_locked_genl(*pp); tm; - pp = &tm->tcpm_next, tm = deref_locked_genl(*pp)) { + for (tm = deref_locked_genl(*pp); tm; tm = deref_locked_genl(*pp)) { if (addr_same(&tm->tcpm_daddr, &daddr)) { *pp = tm->tcpm_next; - break; + tm->tcpm_next = tmlist; + tmlist = tm; + } else { + pp = &tm->tcpm_next; } } spin_unlock_bh(&tcp_metrics_lock); - if (!tm) + if (!tmlist) return -ESRCH; - kfree_rcu(tm, rcu_head); + for (tm = tmlist; tm; tm = tmlist) { + tmlist = tm->tcpm_next; + kfree_rcu(tm, rcu_head); + } return 0; }
As we now can have multiple entries per destination-IP, the "ip tcp_metrics delete address ADDRESS" command should delete all of them. Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be> --- net/ipv4/tcp_metrics.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)