netdev - Re: [PATCH bpf-next v2 09/11] nfp: bpf: use extack support to improve debugging

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 16 Jan 2018 12:11:01 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     davem@...emloft.net, daniel@...earbox.net,
        alexei.starovoitov@...il.com, netdev@...r.kernel.org,
        dsahern@...il.com, oss-drivers@...ronome.com,
        john.fastabend@...il.com, jhs@...atatu.com, gerlitz.or@...il.com,
        aring@...atatu.com, xiyou.wangcong@...il.com,
        Quentin Monnet <quentin.monnet@...ronome.com>
Subject: Re: [PATCH bpf-next v2 09/11] nfp: bpf: use extack support to
 improve debugging

On Tue, 16 Jan 2018 10:36:01 +0100, Jiri Pirko wrote:
> >@@ -303,7 +305,8 @@ static int nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog)
> > 	/* Load up the JITed code */
> > 	err = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_BPF);
> > 	if (err)
> >-		nn_err(nn, "FW command error while loading BPF: %d\n", err);
> >+		NL_SET_ERR_MSG_MOD(extack,
> >+				   "FW command error while loading BPF");  
> 
> One line please. Same for all others. Strings may overflow 80 cols.

Sorry, but this is the way I want things in the nfp driver.  If the
string would fit 80 chars placed on a new line, it should be placed 
on a new line.  If it doesn't fit anyway the new line is unnecessary.
This rules is adhered to throughout the driver (to the extent I'm able
to enforce it).

This is the diff of what you asked:

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index a638c3ab6b61..1e987bcbc086 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -126,20 +126,17 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
        int err;
 
        if (type != TC_SETUP_CLSBPF) {
-               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
-                                  "only offload of BPF classifiers supported");
+               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack, "only offload of BPF classifiers supported");
                return -EOPNOTSUPP;
        }
        if (!tc_can_offload_extack(nn->dp.netdev, cls_bpf->common.extack))
                return -EOPNOTSUPP;
        if (!nfp_net_ebpf_capable(nn)) {
-               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
-                                  "NFP firmware does not support eBPF offload");
+               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack, "NFP firmware does not support eBPF offload");
                return -EOPNOTSUPP;
        }
        if (cls_bpf->common.protocol != htons(ETH_P_ALL)) {
-               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
-                                  "only ETH_P_ALL supported as filter protocol");
+               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack, "only ETH_P_ALL supported as filter protocol");
                return -EOPNOTSUPP;
        }
        if (cls_bpf->common.chain_index)
@@ -148,8 +145,7 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
        /* Only support TC direct action */
        if (!cls_bpf->exts_integrated ||
            tcf_exts_has_actions(cls_bpf->exts)) {
-               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
-                                  "only direct action with no legacy actions supported");
+               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack, "only direct action with no legacy actions supported");
                return -EOPNOTSUPP;
        }
 
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index 9c78a09cda24..181c90476854 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -305,8 +305,7 @@ nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog,
        /* Load up the JITed code */
        err = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_BPF);
        if (err)
-               NL_SET_ERR_MSG_MOD(extack,
-                                  "FW command error while loading BPF");
+               NL_SET_ERR_MSG_MOD(extack, "FW command error while loading BPF");
 
        dma_unmap_single(nn->dp.dev, dma_addr, nfp_prog->prog_len * sizeof(u64),
                         DMA_TO_DEVICE);
@@ -325,8 +324,7 @@ nfp_net_bpf_start(struct nfp_net *nn, struct netlink_ext_ack *extack)
        nn_writel(nn, NFP_NET_CFG_CTRL, nn->dp.ctrl);
        err = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_GEN);
        if (err)
-               NL_SET_ERR_MSG_MOD(extack,
-                                  "FW command error while enabling BPF");
+               NL_SET_ERR_MSG_MOD(extack, "FW command error while enabling BPF");
 }
 
 static int nfp_net_bpf_stop(struct nfp_net *nn)
@@ -359,8 +357,7 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
 
                cap = nn_readb(nn, NFP_NET_CFG_BPF_CAP);
                if (!(cap & NFP_NET_BPF_CAP_RELO)) {
-                       NL_SET_ERR_MSG_MOD(extack,
-                                          "FW does not support live reload");
+                       NL_SET_ERR_MSG_MOD(extack, "FW does not support live reload");
                        return -EBUSY;
                }
        }

strings overflow to new line on the left which is yucky and avoidable.

Please speak up if you feel strongly about this, otherwise I will
repost addressing only your other comment.

Thanks for reviewing!