* Re: [PATCH V2] mlx5: Fix formats with line continuation whitespace
From: Joe Perches @ 2019-08-02 18:09 UTC (permalink / raw)
To: Doug Ledford, Leon Romanovsky
Cc: Saeed Mahameed, David S. Miller, netdev, linux-rdma, linux-kernel
In-Reply-To: <ac8361beee5dd80ad6546328dd7457bb6ee1ca5a.camel@redhat.com>
On Tue, 2018-11-06 at 16:34 -0500, Doug Ledford wrote:
> On Thu, 2018-11-01 at 09:34 +0200, Leon Romanovsky wrote:
> > On Thu, Nov 01, 2018 at 12:24:08AM -0700, Joe Perches wrote:
> > > The line continuations unintentionally add whitespace so
> > > instead use coalesced formats to remove the whitespace.
> > >
> > > Signed-off-by: Joe Perches <joe@perches.com>
> > > ---
> > >
> > > v2: Remove excess space after %u
> > >
> > > drivers/net/ethernet/mellanox/mlx5/core/rl.c | 6 ++----
> > > 1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> >
> > Thanks,
> > Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
>
> Applied, thanks.
Still not upstream. How long does it take?
^ permalink raw reply
* Re: [PATCH net-next v5 5/6] flow_offload: support get flow_block immediately
From: Jakub Kicinski @ 2019-08-02 18:02 UTC (permalink / raw)
To: wenxu; +Cc: jiri, pablo, fw, netfilter-devel, netdev, John Hurley
In-Reply-To: <55850b13-991f-97bd-b452-efacd0f39aa4@ucloud.cn>
On Fri, 2 Aug 2019 21:09:03 +0800, wenxu wrote:
> >> We'd have something like the loop in flow_get_default_block():
> >>
> >> for each (subsystem)
> >> subsystem->handle_new_indir_cb(indr_dev, cb);
> >>
> >> And then per-subsystem logic would actually call the cb. Or:
> >>
> >> for each (subsystem)
> >> block = get_default_block(indir_dev)
> >> indr_dev->ing_cmd_cb(...)
> > nft dev chian is also based on register_netdevice_notifier, So for unregister case,
> >
> > the basechian(block) of nft maybe delete before the __tc_indr_block_cb_unregister. is right?
> >
> > So maybe we can cache the block as a list of all the subsystem in indr_dev ?
>
>
> when the device is unregister the nft netdev chain related to this device will also be delete through netdevice_notifier
>
> . So for unregister case,the basechian(block) of nft maybe delete before the __tc_indr_block_cb_unregister.
Hm, but I don't think that should be an issue. The ordering should be
like one of the following two:
device unregister:
- driver notifier callback
- unregister flow cb
- UNBIND cb
- free driver's block state
- free driver's device state
- nft block destroy
# doesn't see driver's CB any more
Or:
device unregister:
- nft block destroy
- UNBIND cb
- free driver's block state
- driver notifier callback
- free driver's state
No?
> cache for the block is not work because the chain already be delete and free. Maybe it improve the prio of
>
> rep_netdev_event can help this?
In theory the cache should work in a similar way as drivers, because
once the indr_dev is created and the initial block is found, the cached
value is just recorded in BIND/UNBIND calls. So if BIND/UNBIND works for
drivers it will also put the right info in the cache.
^ permalink raw reply
* Re: [PATCH v4 2/4] net: phy: Add function to retrieve LED configuration from the DT
From: Matthias Kaehlcke @ 2019-08-02 17:59 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190802163810.GL2099@lunn.ch>
On Fri, Aug 02, 2019 at 06:38:10PM +0200, Andrew Lunn wrote:
> On Thu, Aug 01, 2019 at 12:07:57PM -0700, Matthias Kaehlcke wrote:
> > Add a phylib function for retrieving PHY LED configuration that
> > is specified in the device tree using the generic binding. LEDs
> > can be configured to be 'on' for a certain link speed or to blink
> > when there is TX/RX activity.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > Changes in v4:
> > - patch added to the series
> > ---
> > drivers/net/phy/phy_device.c | 50 ++++++++++++++++++++++++++++++++++++
> > include/linux/phy.h | 15 +++++++++++
> > 2 files changed, 65 insertions(+)
> >
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index 6b5cb87f3866..b4b48de45712 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -2188,6 +2188,56 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv)
> > return phydrv->config_intr && phydrv->ack_interrupt;
> > }
> >
> > +int of_get_phy_led_cfg(struct phy_device *phydev, int led,
> > + struct phy_led_config *cfg)
> > +{
> > + struct device_node *np, *child;
> > + const char *trigger;
> > + int ret;
> > +
> > + if (!IS_ENABLED(CONFIG_OF_MDIO))
> > + return -ENOENT;
> > +
> > + np = of_find_node_by_name(phydev->mdio.dev.of_node, "leds");
> > + if (!np)
> > + return -ENOENT;
> > +
> > + for_each_child_of_node(np, child) {
> > + u32 val;
> > +
> > + if (!of_property_read_u32(child, "reg", &val)) {
> > + if (val == (u32)led)
> > + break;
> > + }
> > + }
>
> Hi Matthias
>
> This is leaking references to np and child. In the past we have not
> cared about this too much, but we are now getting patches adding the
> missing releases. So it would be good to fix this.
Good point, I'll fix it in the next revision.
Thanks
Matthias
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: Print error when ovs_execute_actions() fails
From: Gregory Rose @ 2019-08-02 17:51 UTC (permalink / raw)
To: Yifeng Sun, netdev, pshelar
In-Reply-To: <1564694047-4859-1-git-send-email-pkusunyifeng@gmail.com>
On 8/1/2019 2:14 PM, Yifeng Sun wrote:
> Currently in function ovs_dp_process_packet(), return values of
> ovs_execute_actions() are silently discarded. This patch prints out
> an error message when error happens so as to provide helpful hints
> for debugging.
>
> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
> ---
> net/openvswitch/datapath.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 892287d..603c533 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -222,6 +222,7 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct sw_flow_key *key)
> struct dp_stats_percpu *stats;
> u64 *stats_counter;
> u32 n_mask_hit;
> + int error;
>
> stats = this_cpu_ptr(dp->stats_percpu);
>
> @@ -229,7 +230,6 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct sw_flow_key *key)
> flow = ovs_flow_tbl_lookup_stats(&dp->table, key, &n_mask_hit);
> if (unlikely(!flow)) {
> struct dp_upcall_info upcall;
> - int error;
>
> memset(&upcall, 0, sizeof(upcall));
> upcall.cmd = OVS_PACKET_CMD_MISS;
> @@ -246,7 +246,10 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct sw_flow_key *key)
>
> ovs_flow_stats_update(flow, key->tp.flags, skb);
> sf_acts = rcu_dereference(flow->sf_acts);
> - ovs_execute_actions(dp, skb, sf_acts, key);
> + error = ovs_execute_actions(dp, skb, sf_acts, key);
> + if (unlikely(error))
> + net_err_ratelimited("ovs: action execution error on datapath %s: %d\n",
> + ovs_dp_name(dp), error);
>
> stats_counter = &stats->n_hit;
>
Thanks Yifeng.
Reviewed-by: Greg Rose <gvrose8192@gmail.com>
^ permalink raw reply
* Re: [PATCH] net: sctp: Rename fallthrough label to unhandled
From: Neil Horman @ 2019-08-02 17:50 UTC (permalink / raw)
To: Joe Perches
Cc: Vlad Yasevich, Marcelo Ricardo Leitner, David S. Miller,
linux-sctp, netdev, linux-kernel
In-Reply-To: <e0dd3af448e38e342c1ac6e7c0c802696eb77fd6.1564549413.git.joe@perches.com>
On Tue, Jul 30, 2019 at 10:04:37PM -0700, Joe Perches wrote:
> fallthrough may become a pseudo reserved keyword so this only use of
> fallthrough is better renamed to allow it.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> net/sctp/sm_make_chunk.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 36bd8a6e82df..3fdcaa2fbf12 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -2152,7 +2152,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
> case SCTP_PARAM_SET_PRIMARY:
> if (net->sctp.addip_enable)
> break;
> - goto fallthrough;
> + goto unhandled;
>
> case SCTP_PARAM_HOST_NAME_ADDRESS:
> /* Tell the peer, we won't support this param. */
> @@ -2163,11 +2163,11 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
> case SCTP_PARAM_FWD_TSN_SUPPORT:
> if (ep->prsctp_enable)
> break;
> - goto fallthrough;
> + goto unhandled;
>
> case SCTP_PARAM_RANDOM:
> if (!ep->auth_enable)
> - goto fallthrough;
> + goto unhandled;
>
> /* SCTP-AUTH: Secion 6.1
> * If the random number is not 32 byte long the association
> @@ -2184,7 +2184,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>
> case SCTP_PARAM_CHUNKS:
> if (!ep->auth_enable)
> - goto fallthrough;
> + goto unhandled;
>
> /* SCTP-AUTH: Section 3.2
> * The CHUNKS parameter MUST be included once in the INIT or
> @@ -2200,7 +2200,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>
> case SCTP_PARAM_HMAC_ALGO:
> if (!ep->auth_enable)
> - goto fallthrough;
> + goto unhandled;
>
> hmacs = (struct sctp_hmac_algo_param *)param.p;
> n_elt = (ntohs(param.p->length) -
> @@ -2223,7 +2223,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
> retval = SCTP_IERROR_ABORT;
> }
> break;
> -fallthrough:
> +unhandled:
> default:
> pr_debug("%s: unrecognized param:%d for chunk:%d\n",
> __func__, ntohs(param.p->type), cid);
> --
> 2.15.0
>
>
Yeah, it seems reasonable to me, though I'm still not comfortable with defining
fallthrough as a macrotized keyword, but thats a debate for another thread
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [PATCH] net: sctp: Rename fallthrough label to unhandled
From: Joe Perches @ 2019-08-02 17:47 UTC (permalink / raw)
To: Neil Horman
Cc: Vlad Yasevich, Marcelo Ricardo Leitner, David S. Miller,
linux-sctp, netdev, linux-kernel
In-Reply-To: <20190731121646.GD9823@hmswarspite.think-freely.org>
On Wed, 2019-07-31 at 08:16 -0400, Neil Horman wrote:
> On Wed, Jul 31, 2019 at 04:32:43AM -0700, Joe Perches wrote:
> > On Wed, 2019-07-31 at 07:19 -0400, Neil Horman wrote:
> > > On Tue, Jul 30, 2019 at 10:04:37PM -0700, Joe Perches wrote:
> > > > fallthrough may become a pseudo reserved keyword so this only use of
> > > > fallthrough is better renamed to allow it.
Can you or any other maintainer apply this patch
or ack it so David Miller can apply it?
Thanks.
^ permalink raw reply
* Re: [net-next 01/12] net/mlx5: E-Switch, add ingress rate support
From: Alexei Starovoitov @ 2019-08-02 17:37 UTC (permalink / raw)
To: Saeed Mahameed
Cc: David S. Miller, netdev@vger.kernel.org, Eli Cohen, Paul Blakey
In-Reply-To: <20190801195620.26180-2-saeedm@mellanox.com>
On Thu, Aug 1, 2019 at 6:30 PM Saeed Mahameed <saeedm@mellanox.com> wrote:
>
> From: Eli Cohen <eli@mellanox.com>
>
> Use the scheduling elements to implement ingress rate limiter on an
> eswitch ports ingress traffic. Since the ingress of eswitch port is the
> egress of VF port, we control eswitch ingress by controlling VF egress.
Looks like the patch is only passing args to firmware which is doing the magic.
Can you please describe what is the algorithm there?
Is it configurable?
^ permalink raw reply
* [PATCH iproute2-next v2] ip tunnel: add json output
From: Andrea Claudi @ 2019-08-02 17:38 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern
Add json support on iptunnel and ip6tunnel.
The plain text output format should remain the same.
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
Changes since v1:
* Use print_color_* for ifname and ip addresses;
* Use print_null() instead of print_bool() where appropriate;
* Reduce indentation level on tnl_print_gre_flags()
---
ip/ip6tunnel.c | 66 ++++++++++++++++++++++++---------------
ip/iptunnel.c | 84 ++++++++++++++++++++++++++++++++------------------
ip/tunnel.c | 37 +++++++++++++++++-----
3 files changed, 125 insertions(+), 62 deletions(-)
diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index d7684a673fdc4..80f16c35f0618 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -71,57 +71,76 @@ static void usage(void)
static void print_tunnel(const void *t)
{
const struct ip6_tnl_parm2 *p = t;
- char s1[1024];
- char s2[1024];
+ SPRINT_BUF(b1);
/* Do not use format_host() for local addr,
* symbolic name will not be useful.
*/
- printf("%s: %s/ipv6 remote %s local %s",
- p->name,
- tnl_strproto(p->proto),
- format_host_r(AF_INET6, 16, &p->raddr, s1, sizeof(s1)),
- rt_addr_n2a_r(AF_INET6, 16, &p->laddr, s2, sizeof(s2)));
+ open_json_object(NULL);
+ print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname", "%s: ", p->name);
+ snprintf(b1, sizeof(b1), "%s/ipv6", tnl_strproto(p->proto));
+ print_string(PRINT_ANY, "mode", "%s ", b1);
+ print_string(PRINT_FP, NULL, "%s", "remote ");
+ print_color_string(PRINT_ANY, COLOR_INET6, "remote", "%s ",
+ format_host_r(AF_INET6, 16, &p->raddr, b1, sizeof(b1)));
+ print_string(PRINT_FP, NULL, "%s", "local ");
+ print_color_string(PRINT_ANY, COLOR_INET6, "local", "%s",
+ rt_addr_n2a_r(AF_INET6, 16, &p->laddr, b1, sizeof(b1)));
+
if (p->link) {
const char *n = ll_index_to_name(p->link);
if (n)
- printf(" dev %s", n);
+ print_string(PRINT_ANY, "link", " dev %s", n);
}
if (p->flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
- printf(" encaplimit none");
+ print_null(PRINT_ANY, "ip6_tnl_f_ign_encap_limit",
+ " encaplimit none", NULL);
else
- printf(" encaplimit %u", p->encap_limit);
+ print_uint(PRINT_ANY, "encap_limit", " encaplimit %u",
+ p->encap_limit);
if (p->hop_limit)
- printf(" hoplimit %u", p->hop_limit);
+ print_uint(PRINT_ANY, "hoplimit", " hoplimit %u", p->hop_limit);
else
- printf(" hoplimit inherit");
+ print_string(PRINT_FP, "hoplimit", " hoplimit %s", "inherit");
- if (p->flags & IP6_TNL_F_USE_ORIG_TCLASS)
- printf(" tclass inherit");
- else {
+ if (p->flags & IP6_TNL_F_USE_ORIG_TCLASS) {
+ print_null(PRINT_ANY, "ip6_tnl_f_use_orig_tclass",
+ " tclass inherit", NULL);
+ } else {
__u32 val = ntohl(p->flowinfo & IP6_FLOWINFO_TCLASS);
- printf(" tclass 0x%02x", (__u8)(val >> 20));
+ snprintf(b1, sizeof(b1), "0x%02x", (__u8)(val >> 20));
+ print_string(PRINT_ANY, "tclass", " tclass %s", b1);
}
- if (p->flags & IP6_TNL_F_USE_ORIG_FLOWLABEL)
- printf(" flowlabel inherit");
- else
- printf(" flowlabel 0x%05x", ntohl(p->flowinfo & IP6_FLOWINFO_FLOWLABEL));
+ if (p->flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
+ print_null(PRINT_ANY, "ip6_tnl_f_use_orig_flowlabel",
+ " flowlabel inherit", NULL);
+ } else {
+ __u32 val = ntohl(p->flowinfo & IP6_FLOWINFO_FLOWLABEL);
- printf(" (flowinfo 0x%08x)", ntohl(p->flowinfo));
+ snprintf(b1, sizeof(b1), "0x%05x", val);
+ print_string(PRINT_ANY, "flowlabel", " flowlabel %s", b1);
+ }
+
+ snprintf(b1, sizeof(b1), "0x%08x", ntohl(p->flowinfo));
+ print_string(PRINT_ANY, "flowinfo", " (flowinfo %s)", b1);
if (p->flags & IP6_TNL_F_RCV_DSCP_COPY)
- printf(" dscp inherit");
+ print_null(PRINT_ANY, "ip6_tnl_f_rcv_dscp_copy",
+ " dscp inherit", NULL);
if (p->flags & IP6_TNL_F_ALLOW_LOCAL_REMOTE)
- printf(" allow-localremote");
+ print_null(PRINT_ANY, "ip6_tnl_f_allow_local_remote",
+ " allow-localremote", NULL);
tnl_print_gre_flags(p->proto, p->i_flags, p->o_flags,
p->i_key, p->o_key);
+
+ close_json_object();
}
static int parse_args(int argc, char **argv, int cmd, struct ip6_tnl_parm2 *p)
@@ -357,7 +376,6 @@ static int do_show(int argc, char **argv)
return -1;
print_tunnel(&p);
- fputc('\n', stdout);
return 0;
}
diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index 66929e75c7448..696f3b92bff55 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -289,17 +289,25 @@ static void print_tunnel(const void *t)
{
const struct ip_tunnel_parm *p = t;
struct ip_tunnel_6rd ip6rd = {};
- char s1[1024];
- char s2[1024];
+ SPRINT_BUF(b1);
/* Do not use format_host() for local addr,
* symbolic name will not be useful.
*/
- printf("%s: %s/ip remote %s local %s",
- p->name,
- tnl_strproto(p->iph.protocol),
- p->iph.daddr ? format_host_r(AF_INET, 4, &p->iph.daddr, s1, sizeof(s1)) : "any",
- p->iph.saddr ? rt_addr_n2a_r(AF_INET, 4, &p->iph.saddr, s2, sizeof(s2)) : "any");
+ open_json_object(NULL);
+ print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname", "%s: ", p->name);
+ snprintf(b1, sizeof(b1), "%s/ip", tnl_strproto(p->iph.protocol));
+ print_string(PRINT_ANY, "mode", "%s ", b1);
+ print_null(PRINT_FP, NULL, "remote ", NULL);
+ print_color_string(PRINT_ANY, COLOR_INET, "remote", "%s ",
+ p->iph.daddr || is_json_context()
+ ? format_host_r(AF_INET, 4, &p->iph.daddr, b1, sizeof(b1))
+ : "any");
+ print_null(PRINT_FP, NULL, "local ", NULL);
+ print_color_string(PRINT_ANY, COLOR_INET, "local", "%s",
+ p->iph.saddr || is_json_context()
+ ? rt_addr_n2a_r(AF_INET, 4, &p->iph.saddr, b1, sizeof(b1))
+ : "any");
if (p->iph.protocol == IPPROTO_IPV6 && (p->i_flags & SIT_ISATAP)) {
struct ip_tunnel_prl prl[16] = {};
@@ -308,54 +316,70 @@ static void print_tunnel(const void *t)
prl[0].datalen = sizeof(prl) - sizeof(prl[0]);
prl[0].addr = htonl(INADDR_ANY);
- if (!tnl_prl_ioctl(SIOCGETPRL, p->name, prl))
+ if (!tnl_prl_ioctl(SIOCGETPRL, p->name, prl)) {
for (i = 1; i < ARRAY_SIZE(prl); i++) {
- if (prl[i].addr != htonl(INADDR_ANY)) {
- printf(" %s %s ",
- (prl[i].flags & PRL_DEFAULT) ? "pdr" : "pr",
- format_host(AF_INET, 4, &prl[i].addr));
- }
+ if (prl[i].addr == htonl(INADDR_ANY))
+ continue;
+ if (prl[i].flags & PRL_DEFAULT)
+ print_string(PRINT_ANY, "pdr",
+ " pdr %s",
+ format_host(AF_INET, 4, &prl[i].addr));
+ else
+ print_string(PRINT_ANY, "pr", " pr %s",
+ format_host(AF_INET, 4, &prl[i].addr));
}
+ }
}
if (p->link) {
const char *n = ll_index_to_name(p->link);
if (n)
- printf(" dev %s", n);
+ print_string(PRINT_ANY, "dev", " dev %s", n);
}
if (p->iph.ttl)
- printf(" ttl %u", p->iph.ttl);
+ print_uint(PRINT_ANY, "ttl", " ttl %u", p->iph.ttl);
else
- printf(" ttl inherit");
+ print_string(PRINT_FP, "ttl", " ttl %s", "inherit");
if (p->iph.tos) {
- SPRINT_BUF(b1);
- printf(" tos");
- if (p->iph.tos & 1)
- printf(" inherit");
- if (p->iph.tos & ~1)
- printf("%c%s ", p->iph.tos & 1 ? '/' : ' ',
- rtnl_dsfield_n2a(p->iph.tos & ~1, b1, sizeof(b1)));
+ SPRINT_BUF(b2);
+
+ if (p->iph.tos != 1) {
+ if (!is_json_context() && p->iph.tos & 1)
+ snprintf(b2, sizeof(b2), "%s%s",
+ p->iph.tos & 1 ? "inherit/" : "",
+ rtnl_dsfield_n2a(p->iph.tos & ~1, b1, sizeof(b1)));
+ else
+ snprintf(b2, sizeof(b2), "%s",
+ rtnl_dsfield_n2a(p->iph.tos, b1, sizeof(b1)));
+ print_string(PRINT_ANY, "tos", " tos %s", b2);
+ } else {
+ print_string(PRINT_FP, NULL, " tos %s", "inherit");
+ }
}
if (!(p->iph.frag_off & htons(IP_DF)))
- printf(" nopmtudisc");
+ print_null(PRINT_ANY, "nopmtudisc", " nopmtudisc", NULL);
if (p->iph.protocol == IPPROTO_IPV6 && !tnl_ioctl_get_6rd(p->name, &ip6rd) && ip6rd.prefixlen) {
- printf(" 6rd-prefix %s/%u",
- inet_ntop(AF_INET6, &ip6rd.prefix, s1, sizeof(s1)),
- ip6rd.prefixlen);
+ print_string(PRINT_ANY, "6rd-prefix", " 6rd-prefix %s",
+ inet_ntop(AF_INET6, &ip6rd.prefix, b1, sizeof(b1)));
+ print_uint(PRINT_ANY, "6rd-prefixlen", "/%u", ip6rd.prefixlen);
if (ip6rd.relay_prefix) {
- printf(" 6rd-relay_prefix %s/%u",
- format_host(AF_INET, 4, &ip6rd.relay_prefix),
- ip6rd.relay_prefixlen);
+ print_string(PRINT_ANY, "6rd-relay_prefix",
+ " 6rd-relay_prefix %s",
+ format_host(AF_INET, 4, &ip6rd.relay_prefix));
+ print_uint(PRINT_ANY, "6rd-relay_prefixlen", "/%u",
+ ip6rd.relay_prefixlen);
}
}
tnl_print_gre_flags(p->iph.protocol, p->i_flags, p->o_flags,
p->i_key, p->o_key);
+
+ close_json_object();
}
diff --git a/ip/tunnel.c b/ip/tunnel.c
index 41b0ef3165fdc..88585cf3177b5 100644
--- a/ip/tunnel.c
+++ b/ip/tunnel.c
@@ -314,24 +314,43 @@ void tnl_print_gre_flags(__u8 proto,
{
if ((i_flags & GRE_KEY) && (o_flags & GRE_KEY) &&
o_key == i_key) {
- printf(" key %u", ntohl(i_key));
+ print_uint(PRINT_ANY, "key", " key %u", ntohl(i_key));
} else {
if (i_flags & GRE_KEY)
- printf(" ikey %u", ntohl(i_key));
+ print_uint(PRINT_ANY, "ikey", " ikey %u", ntohl(i_key));
if (o_flags & GRE_KEY)
- printf(" okey %u", ntohl(o_key));
+ print_uint(PRINT_ANY, "okey", " okey %u", ntohl(o_key));
}
- if (proto == IPPROTO_GRE) {
- if (i_flags & GRE_SEQ)
+ if (proto != IPPROTO_GRE)
+ return;
+
+ open_json_array(PRINT_JSON, "flags");
+ if (i_flags & GRE_SEQ) {
+ if (is_json_context())
+ print_string(PRINT_JSON, NULL, "%s", "rx_drop_ooseq");
+ else
printf("%s Drop packets out of sequence.", _SL_);
- if (i_flags & GRE_CSUM)
+ }
+ if (i_flags & GRE_CSUM) {
+ if (is_json_context())
+ print_string(PRINT_JSON, NULL, "%s", "rx_csum");
+ else
printf("%s Checksum in received packet is required.", _SL_);
- if (o_flags & GRE_SEQ)
+ }
+ if (o_flags & GRE_SEQ) {
+ if (is_json_context())
+ print_string(PRINT_JSON, NULL, "%s", "tx_seq");
+ else
printf("%s Sequence packets on output.", _SL_);
- if (o_flags & GRE_CSUM)
+ }
+ if (o_flags & GRE_CSUM) {
+ if (is_json_context())
+ print_string(PRINT_JSON, NULL, "%s", "tx_csum");
+ else
printf("%s Checksum output packets.", _SL_);
}
+ close_json_array(PRINT_JSON, NULL);
}
static void tnl_print_stats(const struct rtnl_link_stats64 *s)
@@ -417,6 +436,7 @@ static int print_nlmsg_tunnel(struct nlmsghdr *n, void *arg)
int do_tunnels_list(struct tnl_print_nlmsg_info *info)
{
+ new_json_obj(json);
if (rtnl_linkdump_req(&rth, preferred_family) < 0) {
perror("Cannot send dump request\n");
return -1;
@@ -426,6 +446,7 @@ int do_tunnels_list(struct tnl_print_nlmsg_info *info)
fprintf(stderr, "Dump terminated\n");
return -1;
}
+ delete_json_obj();
return 0;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH iproute2-next] ip tunnel: add json output
From: Andrea Claudi @ 2019-08-02 17:30 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: linux-netdev, David Ahern
In-Reply-To: <20190802084933.10992569@hermes.lan>
On Fri, Aug 2, 2019 at 5:49 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Fri, 2 Aug 2019 13:14:15 +0200
> Andrea Claudi <aclaudi@redhat.com> wrote:
>
> > On Thu, Aug 1, 2019 at 5:16 PM Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> > >
> > > On Thu, 1 Aug 2019 12:12:58 +0200
> > > Andrea Claudi <aclaudi@redhat.com> wrote:
> > >
> > > > Add json support on iptunnel and ip6tunnel.
> > > > The plain text output format should remain the same.
> > > >
> > > > Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
> > > > ---
> > > > ip/ip6tunnel.c | 82 +++++++++++++++++++++++++++++++--------------
> > > > ip/iptunnel.c | 90 +++++++++++++++++++++++++++++++++-----------------
> > > > ip/tunnel.c | 42 +++++++++++++++++------
> > > > 3 files changed, 148 insertions(+), 66 deletions(-)
> > > >
> > > > diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
> > > > index d7684a673fdc4..f2b9710c1320f 100644
> > > > --- a/ip/ip6tunnel.c
> > > > +++ b/ip/ip6tunnel.c
> > > > @@ -71,57 +71,90 @@ static void usage(void)
> > > > static void print_tunnel(const void *t)
> > > > {
> > > > const struct ip6_tnl_parm2 *p = t;
> > > > - char s1[1024];
> > > > - char s2[1024];
> > > > + SPRINT_BUF(b1);
> > > >
> > > > /* Do not use format_host() for local addr,
> > > > * symbolic name will not be useful.
> > > > */
> > > > - printf("%s: %s/ipv6 remote %s local %s",
> > > > - p->name,
> > > > - tnl_strproto(p->proto),
> > > > - format_host_r(AF_INET6, 16, &p->raddr, s1, sizeof(s1)),
> > > > - rt_addr_n2a_r(AF_INET6, 16, &p->laddr, s2, sizeof(s2)));
> > > > + open_json_object(NULL);
> > > > + print_string(PRINT_ANY, "ifname", "%s: ", p->name);
> > >
> > > Print this using color for interface name?
> >
> > Thanks for the suggestion, I can do the same also for remote and local
> > addresses?
> >
> > >
> > >
> > > > + snprintf(b1, sizeof(b1), "%s/ipv6", tnl_strproto(p->proto));
> > > > + print_string(PRINT_ANY, "mode", "%s ", b1);
> > > > + print_string(PRINT_ANY,
> > > > + "remote",
> > > > + "remote %s ",
> > > > + format_host_r(AF_INET6, 16, &p->raddr, b1, sizeof(b1)));
> > > > + print_string(PRINT_ANY,
> > > > + "local",
> > > > + "local %s",
> > > > + rt_addr_n2a_r(AF_INET6, 16, &p->laddr, b1, sizeof(b1)));
> > > > +
> > > > if (p->link) {
> > > > const char *n = ll_index_to_name(p->link);
> > > >
> > > > if (n)
> > > > - printf(" dev %s", n);
> > > > + print_string(PRINT_ANY, "link", " dev %s", n);
> > > > }
> > > >
> > > > if (p->flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
> > > > - printf(" encaplimit none");
> > > > + print_bool(PRINT_ANY,
> > > > + "ip6_tnl_f_ign_encap_limit",
> > > > + " encaplimit none",
> > > > + true);
> > >
> > > For flags like this, print_null is more typical JSON than a boolean
> > > value. Null is better for presence flag. Bool is better if both true and
> > > false are printed.
> >
> > Using print_null json JSON output becomes:
> >
> > {
> > "ifname": "gre2",
> > "mode": "gre/ipv6",
> > "remote": "fd::1",
> > "local": "::",
> > "ip6_tnl_f_ign_encap_limit": null,
> > "hoplimit": 64,
> > "tclass": "0x00",
> > "flowlabel": "0x00000",
> > "flowinfo": "0x00000000",
> > "flags": []
> > }
> >
> > Which seems a bit confusing to me (what does the "null" value means?).
> > Using print_null we also introduce an inconsistency with the JSON
> > output of ip/link_gre6.c and ip/link_ip6tnl.c.
> > I would prefer to use print_bool and print out
> > ip6_tnl_f_ign_encap_limit both when true and false, patching both
> > files above to do the same. WDYT?
>
> JSON has several ways to represent the same type of flag value.
> Since JSON always has key/value. Null is used to indicate something is present and
> in that case the value is unnecessary, which is what the null field was meant for.
>
Thanks for the clarification, Stephen.
I'll submit a v2 with print_null and I'll fix cases similar to the one
you highlighted above in subsequent patches.
^ permalink raw reply
* Re: [PATCH V2 7/9] vhost: do not use RCU to synchronize MMU notifier with worker
From: Jason Gunthorpe @ 2019-08-02 17:24 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, kvm, virtualization, netdev, linux-kernel, linux-mm
In-Reply-To: <20190802100414-mutt-send-email-mst@kernel.org>
On Fri, Aug 02, 2019 at 10:27:21AM -0400, Michael S. Tsirkin wrote:
> On Fri, Aug 02, 2019 at 09:46:13AM -0300, Jason Gunthorpe wrote:
> > On Fri, Aug 02, 2019 at 05:40:07PM +0800, Jason Wang wrote:
> > > > This must be a proper barrier, like a spinlock, mutex, or
> > > > synchronize_rcu.
> > >
> > >
> > > I start with synchronize_rcu() but both you and Michael raise some
> > > concern.
> >
> > I've also idly wondered if calling synchronize_rcu() under the various
> > mm locks is a deadlock situation.
> >
> > > Then I try spinlock and mutex:
> > >
> > > 1) spinlock: add lots of overhead on datapath, this leads 0 performance
> > > improvement.
> >
> > I think the topic here is correctness not performance improvement
>
> The topic is whether we should revert
> commit 7f466032dc9 ("vhost: access vq metadata through kernel virtual address")
>
> or keep it in. The only reason to keep it is performance.
Yikes, I'm not sure you can ever win against copy_from_user using
mmu_notifiers? The synchronization requirements are likely always
more expensive unless large and scattered copies are being done..
The rcu is about the only simple approach that could be less
expensive, and that gets back to the question if you can block an
invalidate_start_range in synchronize_rcu or not..
So, frankly, I'd revert it until someone could prove the rcu solution is
OK..
BTW, how do you get copy_from_user to work outside a syscall?
Also, why can't this just permanently GUP the pages? In fact, where
does it put_page them anyhow? Worrying that 7f466 adds a get_user page
but does not add a put_page??
Jason
^ permalink raw reply
* [PATCH v2 2/3] net/mlx5: Use refcount_() APIs
From: Chuhong Yuan @ 2019-08-02 17:23 UTC (permalink / raw)
Cc: Saeed Mahameed, Leon Romanovsky, David S . Miller, netdev,
linux-rdma, linux-kernel, Chuhong Yuan
This patch depends on PATCH 1/3.
After converting refcount to refcount_t, use
refcount_() APIs to operate it.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/ethernet/mellanox/mlx5/core/qp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/qp.c b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
index b8ba74de9555..7b44d1e49604 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/qp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
@@ -53,7 +53,7 @@ mlx5_get_rsc(struct mlx5_qp_table *table, u32 rsn)
common = radix_tree_lookup(&table->tree, rsn);
if (common)
- atomic_inc(&common->refcount);
+ refcount_inc(&common->refcount);
spin_unlock_irqrestore(&table->lock, flags);
@@ -62,7 +62,7 @@ mlx5_get_rsc(struct mlx5_qp_table *table, u32 rsn)
void mlx5_core_put_rsc(struct mlx5_core_rsc_common *common)
{
- if (atomic_dec_and_test(&common->refcount))
+ if (refcount_dec_and_test(&common->refcount))
complete(&common->free);
}
@@ -209,7 +209,7 @@ static int create_resource_common(struct mlx5_core_dev *dev,
if (err)
return err;
- atomic_set(&qp->common.refcount, 1);
+ refcount_set(&qp->common.refcount, 1);
init_completion(&qp->common.free);
qp->pid = current->pid;
--
2.20.1
^ permalink raw reply related
* [PATCH v2 1/3] mlx5: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 17:23 UTC (permalink / raw)
Cc: Saeed Mahameed, Leon Romanovsky, netdev, linux-rdma, linux-kernel,
Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
Changes in v2:
- Add #include.
include/linux/mlx5/driver.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 0e6da1840c7d..5b56f343ce87 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -47,6 +47,7 @@
#include <linux/interrupt.h>
#include <linux/idr.h>
#include <linux/notifier.h>
+#include <linux/refcount.h>
#include <linux/mlx5/device.h>
#include <linux/mlx5/doorbell.h>
@@ -398,7 +399,7 @@ enum mlx5_res_type {
struct mlx5_core_rsc_common {
enum mlx5_res_type res;
- atomic_t refcount;
+ refcount_t refcount;
struct completion free;
};
--
2.20.1
^ permalink raw reply related
* [PATCH v2 0/3] Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 17:23 UTC (permalink / raw)
Cc: Saeed Mahameed, Leon Romanovsky, David S . Miller, Doug Ledford,
Jason Gunthorpe, netdev, linux-rdma, linux-kernel, Chuhong Yuan
Reference counters are preferred to use refcount_t instead of
atomic_t.
This is because the implementation of refcount_t can prevent
overflows and detect possible use-after-free.
First convert the refcount field to refcount_t in mlx5/driver.h.
Then convert the uses to refcount_() APIs.
Changelog:
v1 -> v2:
- Add #include in include/linux/mlx5/driver.h.
Chuhong Yuan (3):
mlx5: Use refcount_t for refcount
net/mlx5: Use refcount_() APIs
IB/mlx5: Use refcount_() APIs
drivers/infiniband/hw/mlx5/srq_cmd.c | 6 +++---
drivers/net/ethernet/mellanox/mlx5/core/qp.c | 6 +++---
include/linux/mlx5/driver.h | 3 ++-
3 files changed, 8 insertions(+), 7 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH bpf-next 3/3] selftests/bpf: test_progs: drop extra trailing tab
From: Stanislav Fomichev @ 2019-08-02 17:17 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190802171710.11456-1-sdf@google.com>
Small (un)related cleanup.
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/test_progs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 71c717162ac8..477539d0adeb 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -279,7 +279,7 @@ enum ARG_KEYS {
ARG_VERIFIER_STATS = 's',
ARG_VERBOSE = 'v',
};
-
+
static const struct argp_option opts[] = {
{ "num", ARG_TEST_NUM, "NUM", 0,
"Run test number NUM only " },
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH bpf-next 2/3] selftests/bpf: test_progs: test__printf -> printf
From: Stanislav Fomichev @ 2019-08-02 17:17 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190802171710.11456-1-sdf@google.com>
Now that test__printf is a simple wraper around printf, let's drop it
(and test__vprintf as well).
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
.../selftests/bpf/prog_tests/bpf_verif_scale.c | 4 ++--
.../testing/selftests/bpf/prog_tests/l4lb_all.c | 2 +-
.../testing/selftests/bpf/prog_tests/map_lock.c | 10 +++++-----
.../selftests/bpf/prog_tests/send_signal.c | 4 ++--
.../testing/selftests/bpf/prog_tests/spinlock.c | 2 +-
.../bpf/prog_tests/stacktrace_build_id.c | 4 ++--
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 4 ++--
.../selftests/bpf/prog_tests/xdp_noinline.c | 4 ++--
tools/testing/selftests/bpf/test_progs.c | 16 +---------------
tools/testing/selftests/bpf/test_progs.h | 10 ++++------
10 files changed, 22 insertions(+), 38 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
index b4be96162ff4..3548ba2f24a8 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -5,13 +5,13 @@ static int libbpf_debug_print(enum libbpf_print_level level,
const char *format, va_list args)
{
if (level != LIBBPF_DEBUG) {
- test__vprintf(format, args);
+ vprintf(format, args);
return 0;
}
if (!strstr(format, "verifier log"))
return 0;
- test__vprintf("%s", args);
+ vprintf("%s", args);
return 0;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/l4lb_all.c b/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
index 5ce572c03a5f..20ddca830e68 100644
--- a/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
+++ b/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
@@ -74,7 +74,7 @@ static void test_l4lb(const char *file)
}
if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
error_cnt++;
- test__printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
+ printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
}
out:
bpf_object__close(obj);
diff --git a/tools/testing/selftests/bpf/prog_tests/map_lock.c b/tools/testing/selftests/bpf/prog_tests/map_lock.c
index 2e78217ed3fd..ee99368c595c 100644
--- a/tools/testing/selftests/bpf/prog_tests/map_lock.c
+++ b/tools/testing/selftests/bpf/prog_tests/map_lock.c
@@ -9,12 +9,12 @@ static void *parallel_map_access(void *arg)
for (i = 0; i < 10000; i++) {
err = bpf_map_lookup_elem_flags(map_fd, &key, vars, BPF_F_LOCK);
if (err) {
- test__printf("lookup failed\n");
+ printf("lookup failed\n");
error_cnt++;
goto out;
}
if (vars[0] != 0) {
- test__printf("lookup #%d var[0]=%d\n", i, vars[0]);
+ printf("lookup #%d var[0]=%d\n", i, vars[0]);
error_cnt++;
goto out;
}
@@ -22,8 +22,8 @@ static void *parallel_map_access(void *arg)
for (j = 2; j < 17; j++) {
if (vars[j] == rnd)
continue;
- test__printf("lookup #%d var[1]=%d var[%d]=%d\n",
- i, rnd, j, vars[j]);
+ printf("lookup #%d var[1]=%d var[%d]=%d\n",
+ i, rnd, j, vars[j]);
error_cnt++;
goto out;
}
@@ -43,7 +43,7 @@ void test_map_lock(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
if (err) {
- test__printf("test_map_lock:bpf_prog_load errno %d\n", errno);
+ printf("test_map_lock:bpf_prog_load errno %d\n", errno);
goto close_prog;
}
map_fd[0] = bpf_find_map(__func__, obj, "hash_map");
diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
index 461b423d0584..1575f0a1f586 100644
--- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
+++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
@@ -202,8 +202,8 @@ static int test_send_signal_nmi(void)
-1 /* cpu */, -1 /* group_fd */, 0 /* flags */);
if (pmu_fd == -1) {
if (errno == ENOENT) {
- test__printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n",
- __func__);
+ printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n",
+ __func__);
return 0;
}
/* Let the test fail with a more informative message */
diff --git a/tools/testing/selftests/bpf/prog_tests/spinlock.c b/tools/testing/selftests/bpf/prog_tests/spinlock.c
index deb2db5b85b0..114ebe6a438e 100644
--- a/tools/testing/selftests/bpf/prog_tests/spinlock.c
+++ b/tools/testing/selftests/bpf/prog_tests/spinlock.c
@@ -12,7 +12,7 @@ void test_spinlock(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
if (err) {
- test__printf("test_spin_lock:bpf_prog_load errno %d\n", errno);
+ printf("test_spin_lock:bpf_prog_load errno %d\n", errno);
goto close_prog;
}
for (i = 0; i < 4; i++)
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
index 356d2c017a9c..ac44fda84833 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
@@ -109,8 +109,8 @@ void test_stacktrace_build_id(void)
if (build_id_matches < 1 && retry--) {
bpf_link__destroy(link);
bpf_object__close(obj);
- test__printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
- __func__);
+ printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
+ __func__);
goto retry;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
index f44f2c159714..9557b7dfb782 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
@@ -140,8 +140,8 @@ void test_stacktrace_build_id_nmi(void)
if (build_id_matches < 1 && retry--) {
bpf_link__destroy(link);
bpf_object__close(obj);
- test__printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
- __func__);
+ printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
+ __func__);
goto retry;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c b/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
index b5404494b8aa..15f7c272edb0 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
@@ -75,8 +75,8 @@ void test_xdp_noinline(void)
}
if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
error_cnt++;
- test__printf("test_xdp_noinline:FAIL:stats %lld %lld\n",
- bytes, pkts);
+ printf("test_xdp_noinline:FAIL:stats %lld %lld\n",
+ bytes, pkts);
}
out:
bpf_object__close(obj);
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 00d1565d01a3..71c717162ac8 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -106,20 +106,6 @@ void test__force_log() {
env.test->force_log = true;
}
-void test__vprintf(const char *fmt, va_list args)
-{
- vprintf(fmt, args);
-}
-
-void test__printf(const char *fmt, ...)
-{
- va_list args;
-
- va_start(args, fmt);
- test__vprintf(fmt, args);
- va_end(args);
-}
-
struct ipv4_packet pkt_v4 = {
.eth.h_proto = __bpf_constant_htons(ETH_P_IP),
.iph.ihl = 5,
@@ -311,7 +297,7 @@ static int libbpf_print_fn(enum libbpf_print_level level,
{
if (!env.very_verbose && level == LIBBPF_DEBUG)
return 0;
- test__vprintf(format, args);
+ vprintf(format, args);
return 0;
}
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 9fd89078494f..cf0486dbb9b4 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -69,8 +69,6 @@ extern int error_cnt;
extern int pass_cnt;
extern struct test_env env;
-extern void test__printf(const char *fmt, ...);
-extern void test__vprintf(const char *fmt, va_list args);
extern void test__force_log();
extern bool test__start_subtest(const char *name);
@@ -96,12 +94,12 @@ extern struct ipv6_packet pkt_v6;
int __ret = !!(condition); \
if (__ret) { \
error_cnt++; \
- test__printf("%s:FAIL:%s ", __func__, tag); \
- test__printf(format); \
+ printf("%s:FAIL:%s ", __func__, tag); \
+ printf(format); \
} else { \
pass_cnt++; \
- test__printf("%s:PASS:%s %d nsec\n", \
- __func__, tag, duration); \
+ printf("%s:PASS:%s %d nsec\n", \
+ __func__, tag, duration); \
} \
__ret; \
})
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH bpf-next 1/3] selftests/bpf: test_progs: switch to open_memstream
From: Stanislav Fomichev @ 2019-08-02 17:17 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190802171710.11456-1-sdf@google.com>
Use open_memstream to override stdout during test execution.
The copy of the original stdout is held in env.stdout and used
to print subtest info and dump failed log.
test_{v,}printf are now simple wrappers around stdout and will be
removed in the next patch.
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/test_progs.c | 100 ++++++++++-------------
tools/testing/selftests/bpf/test_progs.h | 2 +-
2 files changed, 46 insertions(+), 56 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index db00196c8315..00d1565d01a3 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -40,14 +40,22 @@ static bool should_run(struct test_selector *sel, int num, const char *name)
static void dump_test_log(const struct prog_test_def *test, bool failed)
{
- if (env.verbose || test->force_log || failed) {
- if (env.log_cnt) {
- fprintf(stdout, "%s", env.log_buf);
- if (env.log_buf[env.log_cnt - 1] != '\n')
- fprintf(stdout, "\n");
+ if (stdout == env.stdout)
+ return;
+
+ fflush(stdout); /* exports env.log_buf & env.log_cap */
+
+ if (env.log_cap && (env.verbose || test->force_log || failed)) {
+ int len = strlen(env.log_buf);
+
+ if (len) {
+ fprintf(env.stdout, "%s", env.log_buf);
+ if (env.log_buf[len - 1] != '\n')
+ fprintf(env.stdout, "\n");
+
+ fseeko(stdout, 0, SEEK_SET); /* rewind */
}
}
- env.log_cnt = 0;
}
void test__end_subtest()
@@ -62,7 +70,7 @@ void test__end_subtest()
dump_test_log(test, sub_error_cnt);
- printf("#%d/%d %s:%s\n",
+ fprintf(env.stdout, "#%d/%d %s:%s\n",
test->test_num, test->subtest_num,
test->subtest_name, sub_error_cnt ? "FAIL" : "OK");
}
@@ -100,53 +108,7 @@ void test__force_log() {
void test__vprintf(const char *fmt, va_list args)
{
- size_t rem_sz;
- int ret = 0;
-
- if (env.verbose || (env.test && env.test->force_log)) {
- vfprintf(stderr, fmt, args);
- return;
- }
-
-try_again:
- rem_sz = env.log_cap - env.log_cnt;
- if (rem_sz) {
- va_list ap;
-
- va_copy(ap, args);
- /* we reserved extra byte for \0 at the end */
- ret = vsnprintf(env.log_buf + env.log_cnt, rem_sz + 1, fmt, ap);
- va_end(ap);
-
- if (ret < 0) {
- env.log_buf[env.log_cnt] = '\0';
- fprintf(stderr, "failed to log w/ fmt '%s'\n", fmt);
- return;
- }
- }
-
- if (!rem_sz || ret > rem_sz) {
- size_t new_sz = env.log_cap * 3 / 2;
- char *new_buf;
-
- if (new_sz < 4096)
- new_sz = 4096;
- if (new_sz < ret + env.log_cnt)
- new_sz = ret + env.log_cnt;
-
- /* +1 for guaranteed space for terminating \0 */
- new_buf = realloc(env.log_buf, new_sz + 1);
- if (!new_buf) {
- fprintf(stderr, "failed to realloc log buffer: %d\n",
- errno);
- return;
- }
- env.log_buf = new_buf;
- env.log_cap = new_sz;
- goto try_again;
- }
-
- env.log_cnt += ret;
+ vprintf(fmt, args);
}
void test__printf(const char *fmt, ...)
@@ -477,6 +439,32 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
return 0;
}
+static void stdout_hijack(void)
+{
+ if (env.verbose || (env.test && env.test->force_log)) {
+ /* nothing to do, output to stdout by default */
+ return;
+ }
+
+ /* stdout -> buffer */
+ fflush(stdout);
+ stdout = open_memstream(&env.log_buf, &env.log_cap);
+}
+
+static void stdout_restore(void)
+{
+ if (stdout == env.stdout)
+ return;
+
+ fclose(stdout);
+ free(env.log_buf);
+
+ env.log_buf = NULL;
+ env.log_cap = 0;
+
+ stdout = env.stdout;
+}
+
int main(int argc, char **argv)
{
static const struct argp argp = {
@@ -495,6 +483,7 @@ int main(int argc, char **argv)
srand(time(NULL));
env.jit_enabled = is_jit_enabled();
+ env.stdout = stdout;
for (i = 0; i < prog_test_cnt; i++) {
struct prog_test_def *test = &prog_test_defs[i];
@@ -508,6 +497,7 @@ int main(int argc, char **argv)
test->test_num, test->test_name))
continue;
+ stdout_hijack();
test->run_test();
/* ensure last sub-test is finalized properly */
if (test->subtest_name)
@@ -522,6 +512,7 @@ int main(int argc, char **argv)
env.succ_cnt++;
dump_test_log(test, test->error_cnt);
+ stdout_restore();
printf("#%d %s:%s\n", test->test_num, test->test_name,
test->error_cnt ? "FAIL" : "OK");
@@ -529,7 +520,6 @@ int main(int argc, char **argv)
printf("Summary: %d/%d PASSED, %d FAILED\n",
env.succ_cnt, env.sub_succ_cnt, env.fail_cnt);
- free(env.log_buf);
free(env.test_selector.num_set);
free(env.subtest_selector.num_set);
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index afd14962456f..9fd89078494f 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -56,8 +56,8 @@ struct test_env {
bool jit_enabled;
struct prog_test_def *test;
+ FILE *stdout;
char *log_buf;
- size_t log_cnt;
size_t log_cap;
int succ_cnt; /* successful tests */
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH bpf-next 0/3] selftests/bpf: switch test_progs back to stdio
From: Stanislav Fomichev @ 2019-08-02 17:17 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
I was looking into converting test_sockops* to test_progs framework
and that requires using cgroup_helpers.c which rely on stdio/stderr.
Let's use open_memstream to override stdout into buffer during
subtests instead of custom test_{v,}printf wrappers. That lets
us continue to use stdio in the subtests and dump it on failure
if required.
That would also fix bpf_find_map which currently uses printf to
signal failure (missed during test_printf conversion).
Cc: Andrii Nakryiko <andriin@fb.com>
Stanislav Fomichev (3):
selftests/bpf: test_progs: switch to open_memstream
selftests/bpf: test_progs: test__printf -> printf
selftests/bpf: test_progs: drop extra trailing tab
.../bpf/prog_tests/bpf_verif_scale.c | 4 +-
.../selftests/bpf/prog_tests/l4lb_all.c | 2 +-
.../selftests/bpf/prog_tests/map_lock.c | 10 +-
.../selftests/bpf/prog_tests/send_signal.c | 4 +-
.../selftests/bpf/prog_tests/spinlock.c | 2 +-
.../bpf/prog_tests/stacktrace_build_id.c | 4 +-
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 4 +-
.../selftests/bpf/prog_tests/xdp_noinline.c | 4 +-
tools/testing/selftests/bpf/test_progs.c | 116 +++++++-----------
tools/testing/selftests/bpf/test_progs.h | 12 +-
10 files changed, 68 insertions(+), 94 deletions(-)
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH][net-next] ice: fix potential infinite loop
From: Colin Ian King @ 2019-08-02 17:12 UTC (permalink / raw)
To: Allan, Bruce W, Kirsher, Jeffrey T, David S . Miller,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org
Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <804857E1F29AAC47BF68C404FC60A18401096DB0DF@ORSMSX122.amr.corp.intel.com>
On 02/08/2019 18:07, Allan, Bruce W wrote:
>> -----Original Message-----
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
>> Of Colin King
>> Sent: Friday, August 02, 2019 8:52 AM
>> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; David S . Miller
>> <davem@davemloft.net>; intel-wired-lan@lists.osuosl.org;
>> netdev@vger.kernel.org
>> Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: [Intel-wired-lan] [PATCH][net-next] ice: fix potential infinite loop
>>
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The loop counter of a for-loop is a u8 however this is being compared
>> to an int upper bound and this can lead to an infinite loop if the
>> upper bound is greater than 255 since the loop counter will wrap back
>> to zero. Fix this potential issue by making the loop counter an int.
>>
>> Addresses-Coverity: ("Infinite loop")
>
> Actually, num_alloc_vfs should probably be a u16 instead of an int since num_alloc_vfs cannot exceed 256.
>
> Which Coverity scan reported this and what options are used in the analysis?
One that I run in a private coverity scan with scan analysis cranked up
high on linux-next, so the report is not public.
Colin
>
>> Fixes: c7aeb4d1b9bf ("ice: Disable VFs until reset is completed")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
>> b/drivers/net/ethernet/intel/ice/ice_main.c
>> index c26e6a102dac..088543d50095 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>> @@ -488,7 +488,7 @@ static void
>> ice_prepare_for_reset(struct ice_pf *pf)
>> {
>> struct ice_hw *hw = &pf->hw;
>> - u8 i;
>> + int i;
>>
>> /* already prepared for reset */
>> if (test_bit(__ICE_PREPARED_FOR_RESET, pf->state))
>> --
>> 2.20.1
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply
* Re: [net v1 PATCH 4/4] net: fix bpf_xdp_adjust_head regression for generic-XDP
From: Jakub Kicinski @ 2019-08-02 17:08 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, David S. Miller, xdp-newbies, Daniel Borkmann,
brandon.cazander, Alexei Starovoitov, Willem de Bruijn
In-Reply-To: <20190802095350.7242399b@carbon>
On Fri, 2 Aug 2019 09:53:54 +0200, Jesper Dangaard Brouer wrote:
> On Thu, 1 Aug 2019 17:44:06 -0700
> Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
>
> > On Thu, 01 Aug 2019 20:00:31 +0200, Jesper Dangaard Brouer wrote:
> > > When generic-XDP was moved to a later processing step by commit
> > > 458bf2f224f0 ("net: core: support XDP generic on stacked devices.")
> > > a regression was introduced when using bpf_xdp_adjust_head.
> > >
> > > The issue is that after this commit the skb->network_header is now
> > > changed prior to calling generic XDP and not after. Thus, if the header
> > > is changed by XDP (via bpf_xdp_adjust_head), then skb->network_header
> > > also need to be updated again. Fix by calling skb_reset_network_header().
> > >
> > > Fixes: 458bf2f224f0 ("net: core: support XDP generic on stacked devices.")
> > > Reported-by: Brandon Cazander <brandon.cazander@multapplied.net>
> > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> >
> > Out of curiosity what was your conclusion regarding resetting the
> > transport header as well?
>
> Well, I don't know... I need some review, from e.g. Stephen that
> changed this... I've added code snippets below signature to helper
> reviewers (also helps understand below paragraph).
>
> I think, we perhaps should call skb_reset_transport_header(), as we
> change skb->data (via either __skb_pull() or __skb_push()), *BUT* I'm
> not sure it is needed/required, as someone/something afterwards still
> need to call skb_set_transport_header(), which also calls
> skb_reset_transport_header() anyway.
Perhaps you've seen this, but just in case - this is the last commit
that touched the transport header setting in __netif_receive_skb(),
and it sounds like it matters mostly for qdisc accounting?
commit fda55eca5a33f33ffcd4192c6b2d75179714a52c
Author: Eric Dumazet <edumazet@google.com>
Date: Mon Jan 7 09:28:21 2013 +0000
net: introduce skb_transport_header_was_set()
We have skb_mac_header_was_set() helper to tell if mac_header
was set on a skb. We would like the same for transport_header.
__netif_receive_skb() doesn't reset the transport header if already
set by GRO layer.
Note that network stacks usually reset the transport header anyway,
after pulling the network header, so this change only allows
a followup patch to have more precise qdisc pkt_len computation
for GSO packets at ingress side.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH][net-next] ice: fix potential infinite loop
From: Allan, Bruce W @ 2019-08-02 17:07 UTC (permalink / raw)
To: Colin King, Kirsher, Jeffrey T, David S . Miller,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org
Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190802155217.16996-1-colin.king@canonical.com>
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of Colin King
> Sent: Friday, August 02, 2019 8:52 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; David S . Miller
> <davem@davemloft.net>; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org
> Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH][net-next] ice: fix potential infinite loop
>
> From: Colin Ian King <colin.king@canonical.com>
>
> The loop counter of a for-loop is a u8 however this is being compared
> to an int upper bound and this can lead to an infinite loop if the
> upper bound is greater than 255 since the loop counter will wrap back
> to zero. Fix this potential issue by making the loop counter an int.
>
> Addresses-Coverity: ("Infinite loop")
Actually, num_alloc_vfs should probably be a u16 instead of an int since num_alloc_vfs cannot exceed 256.
Which Coverity scan reported this and what options are used in the analysis?
> Fixes: c7aeb4d1b9bf ("ice: Disable VFs until reset is completed")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index c26e6a102dac..088543d50095 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -488,7 +488,7 @@ static void
> ice_prepare_for_reset(struct ice_pf *pf)
> {
> struct ice_hw *hw = &pf->hw;
> - u8 i;
> + int i;
>
> /* already prepared for reset */
> if (test_bit(__ICE_PREPARED_FOR_RESET, pf->state))
> --
> 2.20.1
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply
* Re: [net-next 2/9] i40e: make visible changed vf mac on host
From: Shannon Nelson @ 2019-08-02 17:00 UTC (permalink / raw)
To: Loktionov, Aleksandr, Kirsher, Jeffrey T, davem@davemloft.net
Cc: netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
Bowers, AndrewX
In-Reply-To: <0EF347314CF65544BA015993979A29CD74513DCB@irsmsx105.ger.corp.intel.com>
On 8/2/19 1:14 AM, Loktionov, Aleksandr wrote:
> Good day Nelson
Please don't top post. The custom on this mailing list is to answer
inline in order to be sure we're answering in context. As it is, I
believe you missed answering one of my questions.
> In 99% cases VF has _only one_ unicast mac anyway, and the last MAC has been chosen because of VF mac address change algo - it marks unicast filter for deletion and appends a new unicast filter to the list.
> The implementation has been chosen because of simplicity /* Just 3 more lines to solve the issue */, from one point it may look wasteful for some 1% of VF corner cases.
> But from another point of view, more complicated code will affect 99% normal cases. Modern cpu are sensitive to cache thrash by code size and pipeline stalls by conditionals
Yes, absolutely. So it follows that (a) we don't want to leave things
in a loop if not necessary to repeat them, (b) we'd like to keep loops
small as possible, (c) we want to keep our spin_lock sections small, and
(d) we don't want to do things that later don't matter if an error
happens when writing to the firmware. So it seems to me that you should
move that copy line from the loop and outside of the spin_lock, and put
it after the call sync the filters. Perhaps track the good mac index
with "good_mac = i" at the end of the loop code and use that later to
know which mac to copy into the vf struct.
I also noticed that you're checking the mac addresses for validity, but
only before copying it to the local vf struct. If you need to check the
addresses, shouldn't you check them before you add them to the vf's
filter list so you don't try to sync bad addresses to the FW?
If the sync to the FW fails, you send the error status to the VF but you
still have this new address copied into the vf struct. I think the copy
line should be after the FW sync, and only if the sync succeeds.
sln
>
> Alex
>
> -----Original Message-----
> From: Shannon Nelson [mailto:snelson@pensando.io]
> Sent: Friday, August 2, 2019 2:11 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; davem@davemloft.net
> Cc: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com; Bowers, AndrewX <andrewx.bowers@intel.com>
> Subject: Re: [net-next 2/9] i40e: make visible changed vf mac on host
>
> On 8/1/19 1:51 PM, Jeff Kirsher wrote:
>> From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>>
>> This patch makes changed VM mac address visible on host via ip link
>> show command. This problem is fixed by copying last unicast mac filter
>> to vf->default_lan_addr.addr. Without this patch if VF MAC was not set
>> from host side and if you run ip link show $pf, on host side you'd
>> always see a zero MAC, not the real VF MAC that VF assigned to itself.
>>
>> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>> drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>> b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>> index 02b09a8ad54c..21f7ac514d1f 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>> @@ -2629,6 +2629,9 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>> } else {
>> vf->num_mac++;
>> }
>> + if (is_valid_ether_addr(al->list[i].addr))
>> + ether_addr_copy(vf->default_lan_addr.addr,
>> + al->list[i].addr);
>> }
>> }
>> spin_unlock_bh(&vsi->mac_filter_hash_lock);
> Since this copy is done inside the for-loop, it looks like you are copying every address in the list, not just the last one. This seems wasteful and unnecessary.
>
> Since it is possible, altho' unlikely, that the filter sync that happens a little later could fail, might it be better to do the copy after you know that the sync has succeeded?
>
> Why is the last mac chosen for display rather than the first? Is there anything special about the last mac as opposed to the first mac?
>
> sln
>
^ permalink raw reply
* Re: [PATCH v4 1/4] dt-bindings: net: phy: Add subnode for LED configuration
From: Andrew Lunn @ 2019-08-02 16:57 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190801190759.28201-2-mka@chromium.org>
On Thu, Aug 01, 2019 at 12:07:56PM -0700, Matthias Kaehlcke wrote:
> The LED behavior of some Ethernet PHYs is configurable. Add an
> optional 'leds' subnode with a child node for each LED to be
> configured. The binding aims to be compatible with the common
> LED binding (see devicetree/bindings/leds/common.txt).
>
> A LED can be configured to be 'on' when a link with a certain speed
> is active, or to blink on RX/TX activity. For the configuration to
> be effective it needs to be supported by the hardware and the
> corresponding PHY driver.
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v4:
> - patch added to the series
> ---
> .../devicetree/bindings/net/ethernet-phy.yaml | 47 +++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index f70f18ff821f..81c5aacc89a5 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -153,6 +153,38 @@ properties:
> Delay after the reset was deasserted in microseconds. If
> this property is missing the delay will be skipped.
>
> +patternProperties:
> + "^leds$":
> + type: object
> + description:
> + Subnode with configuration of the PHY LEDs.
> +
> + patternProperties:
> + "^led@[0-9]+$":
> + type: object
> + description:
> + Subnode with the configuration of a single PHY LED.
> +
> + properties:
> + reg:
> + description:
> + The ID number of the LED, typically corresponds to a hardware ID.
> + $ref: "/schemas/types.yaml#/definitions/uint32"
> +
> + linux,default-trigger:
> + description:
> + This parameter, if present, is a string specifying the trigger
> + assigned to the LED. Supported triggers are:
> + "phy_link_10m_active" - LED will be on when a 10Mb/s link is active
> + "phy_link_100m_active" - LED will be on when a 100Mb/s link is active
> + "phy_link_1g_active" - LED will be on when a 1Gb/s link is active
> + "phy_link_10g_active" - LED will be on when a 10Gb/s link is active
> + "phy_activity" - LED will blink when data is received or transmitted
Matthias
We should think a bit more about these names.
I can see in future needing 1G link, but it blinks off when there is
active traffic? So phy_link_1g_active could be confusing, and very similar to
phy_link_1g_activity? So maybe
> + "phy_link_10m" - LED will be solid on when a 10Mb/s link is active
> + "phy_link_100m" - LED will be solid on when a 100Mb/s link is active
> + "phy_link_1g" - LED will be solid on when a 1Gb/s link is active
etc.
And then in the future we can have
"phy_link_1g_activity' - LED will be on when 1Gbp/s
link is active and blink off
with activity.
What other use cases do we have? I don't want to support everything,
but we should be able to represent the most common modes without the
names getting too confusing.
Andrew
^ permalink raw reply
* [PATCH v2] net/mlx5e: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 16:48 UTC (permalink / raw)
Cc: Saeed Mahameed, Leon Romanovsky, David S . Miller, netdev,
linux-rdma, linux-kernel, Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
Changes in v2:
- Add #include.
drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c
index b9d4f4e19ff9..148b55c3db7a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c
@@ -32,6 +32,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/refcount.h>
#include <linux/mlx5/driver.h>
#include <net/vxlan.h>
#include "mlx5_core.h"
@@ -48,7 +49,7 @@ struct mlx5_vxlan {
struct mlx5_vxlan_port {
struct hlist_node hlist;
- atomic_t refcount;
+ refcount_t refcount;
u16 udp_port;
};
@@ -113,7 +114,7 @@ int mlx5_vxlan_add_port(struct mlx5_vxlan *vxlan, u16 port)
vxlanp = mlx5_vxlan_lookup_port(vxlan, port);
if (vxlanp) {
- atomic_inc(&vxlanp->refcount);
+ refcount_inc(&vxlanp->refcount);
return 0;
}
@@ -137,7 +138,7 @@ int mlx5_vxlan_add_port(struct mlx5_vxlan *vxlan, u16 port)
}
vxlanp->udp_port = port;
- atomic_set(&vxlanp->refcount, 1);
+ refcount_set(&vxlanp->refcount, 1);
spin_lock_bh(&vxlan->lock);
hash_add(vxlan->htable, &vxlanp->hlist, port);
@@ -170,7 +171,7 @@ int mlx5_vxlan_del_port(struct mlx5_vxlan *vxlan, u16 port)
goto out_unlock;
}
- if (atomic_dec_and_test(&vxlanp->refcount)) {
+ if (refcount_dec_and_test(&vxlanp->refcount)) {
hash_del(&vxlanp->hlist);
remove = true;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v2] mkiss: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 16:48 UTC (permalink / raw)
Cc: David S . Miller, netdev, linux-kernel, Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
Changes in v2:
- Add #include.
drivers/net/hamradio/mkiss.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/hamradio/mkiss.c b/drivers/net/hamradio/mkiss.c
index 442018ccd65e..c5bfa19ddb93 100644
--- a/drivers/net/hamradio/mkiss.c
+++ b/drivers/net/hamradio/mkiss.c
@@ -25,6 +25,7 @@
#include <linux/skbuff.h>
#include <linux/if_arp.h>
#include <linux/jiffies.h>
+#include <linux/refcount.h>
#include <net/ax25.h>
@@ -70,7 +71,7 @@ struct mkiss {
#define CRC_MODE_FLEX_TEST 3
#define CRC_MODE_SMACK_TEST 4
- atomic_t refcnt;
+ refcount_t refcnt;
struct completion dead;
};
@@ -668,7 +669,7 @@ static struct mkiss *mkiss_get(struct tty_struct *tty)
read_lock(&disc_data_lock);
ax = tty->disc_data;
if (ax)
- atomic_inc(&ax->refcnt);
+ refcount_inc(&ax->refcnt);
read_unlock(&disc_data_lock);
return ax;
@@ -676,7 +677,7 @@ static struct mkiss *mkiss_get(struct tty_struct *tty)
static void mkiss_put(struct mkiss *ax)
{
- if (atomic_dec_and_test(&ax->refcnt))
+ if (refcount_dec_and_test(&ax->refcnt))
complete(&ax->dead);
}
@@ -704,7 +705,7 @@ static int mkiss_open(struct tty_struct *tty)
ax->dev = dev;
spin_lock_init(&ax->buflock);
- atomic_set(&ax->refcnt, 1);
+ refcount_set(&ax->refcnt, 1);
init_completion(&ax->dead);
ax->tty = tty;
@@ -784,7 +785,7 @@ static void mkiss_close(struct tty_struct *tty)
* We have now ensured that nobody can start using ap from now on, but
* we have to wait for all existing users to finish.
*/
- if (!atomic_dec_and_test(&ax->refcnt))
+ if (!refcount_dec_and_test(&ax->refcnt))
wait_for_completion(&ax->dead);
/*
* Halt the transmit queue so that a new transmit cannot scribble
--
2.20.1
^ permalink raw reply related
* [PATCH v2] dpaa_eth: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 16:47 UTC (permalink / raw)
Cc: Madalin Bucur, David S . Miller, netdev, linux-kernel,
Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
Changes in v2:
- Add #include in dpaa_eth.h.
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 6 +++---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index f38c3fa7d705..2df6e745cb3f 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -485,7 +485,7 @@ static struct dpaa_bp *dpaa_bpid2pool(int bpid)
static bool dpaa_bpid2pool_use(int bpid)
{
if (dpaa_bpid2pool(bpid)) {
- atomic_inc(&dpaa_bp_array[bpid]->refs);
+ refcount_inc(&dpaa_bp_array[bpid]->refs);
return true;
}
@@ -496,7 +496,7 @@ static bool dpaa_bpid2pool_use(int bpid)
static void dpaa_bpid2pool_map(int bpid, struct dpaa_bp *dpaa_bp)
{
dpaa_bp_array[bpid] = dpaa_bp;
- atomic_set(&dpaa_bp->refs, 1);
+ refcount_set(&dpaa_bp->refs, 1);
}
static int dpaa_bp_alloc_pool(struct dpaa_bp *dpaa_bp)
@@ -584,7 +584,7 @@ static void dpaa_bp_free(struct dpaa_bp *dpaa_bp)
if (!bp)
return;
- if (!atomic_dec_and_test(&bp->refs))
+ if (!refcount_dec_and_test(&bp->refs))
return;
if (bp->free_buf_cb)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
index af320f83c742..f7e59e8db075 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
@@ -32,6 +32,7 @@
#define __DPAA_H
#include <linux/netdevice.h>
+#include <linux/refcount.h>
#include <soc/fsl/qman.h>
#include <soc/fsl/bman.h>
@@ -99,7 +100,7 @@ struct dpaa_bp {
int (*seed_cb)(struct dpaa_bp *);
/* bpool can be emptied before freeing by this cb */
void (*free_buf_cb)(const struct dpaa_bp *, struct bm_buffer *);
- atomic_t refs;
+ refcount_t refs;
};
struct dpaa_rx_errors {
--
2.20.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox