On Fri, Oct 18, 2019 at 10:31 PM Jakub Kicinski
[off-list ref] wrote:

On Fri, 18 Oct 2019 12:04:35 +0530, Vasundhara Volam wrote:

quoted

On Fri, Oct 18, 2019 at 12:52 AM Jakub Kicinski wrote:

quoted

On Thu, 17 Oct 2019 17:31:22 +0530, Sheetal Tigadoli wrote:

quoted

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 51c1404..1596221 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c

@@ -3311,6 +3311,23 @@ static int bnxt_get_coredump(struct bnxt *bp, void *buf, u32 *dump_len)
      return rc;
 }

+static int bnxt_set_dump(struct net_device *dev, struct ethtool_dump *dump)
+{
+     struct bnxt *bp = netdev_priv(dev);
+
+#ifndef CONFIG_TEE_BNXT_FW
+     return -EOPNOTSUPP;
+#endif

        if (!IS_ENABLED(...))
                return x;

reads better IMHO

Okay.

quoted

But also you seem to be breaking live dump for systems with
CONFIG_TEE_BNXT_FW=n

Yes, we are supporting set_dump only if crash dump is supported.

It's wrong.

Sorry not very clear. You are saying that support set_dump all the
time and return
error, if the config option is not enabled? If yes, I will modify the
same way as it
makes sense.

quoted

quoted

quoted

+     if (dump->flag > BNXT_DUMP_CRASH) {
+             netdev_err(dev, "Supports only Live(0) and Crash(1) dumps.\n");

more of an _info than _err, if at all

I made this err, as we are returning error on invalid flag value. I
can modify the log to
something like "Invalid dump flag. Supports only Live(0) and Crash(1)
dumps.\n" to make
it more like error log.

Not an error.

Okay.