* [PATCH net-next] liquidio: pass date and time info to NIC firmware
From: Felix Manlunas @ 2017-09-23 0:35 UTC (permalink / raw)
To: davem
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
manish.awasthi, veerasenareddy.burru
From: Veerasenareddy Burru <veerasenareddy.burru@cavium.com>
Signed-off-by: Veerasenareddy Burru <veerasenareddy.burru@cavium.com>
Signed-off-by: Manish Awasthi <manish.awasthi@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
.../net/ethernet/cavium/liquidio/octeon_console.c | 28 +++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_console.c b/drivers/net/ethernet/cavium/liquidio/octeon_console.c
index ec3dd69..eda799b 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_console.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_console.c
@@ -803,15 +803,19 @@ static int octeon_console_read(struct octeon_device *oct, u32 console_num,
}
#define FBUF_SIZE (4 * 1024 * 1024)
+#define MAX_DATE_SIZE 30
int octeon_download_firmware(struct octeon_device *oct, const u8 *data,
size_t size)
{
- int ret = 0;
+ struct octeon_firmware_file_header *h;
+ char date[MAX_DATE_SIZE];
+ struct timeval time;
u32 crc32_result;
+ struct tm tm_val;
u64 load_addr;
u32 image_len;
- struct octeon_firmware_file_header *h;
+ int ret = 0;
u32 i, rem;
if (size < sizeof(struct octeon_firmware_file_header)) {
@@ -890,11 +894,29 @@ int octeon_download_firmware(struct octeon_device *oct, const u8 *data,
load_addr += size;
}
}
+
+ /* Get time of the day */
+ do_gettimeofday(&time);
+ time_to_tm(time.tv_sec, (-sys_tz.tz_minuteswest) * 60, &tm_val);
+ ret = snprintf(date, MAX_DATE_SIZE,
+ " date=%04ld.%02d.%02d-%02d:%02d:%02d",
+ tm_val.tm_year + 1900, tm_val.tm_mon + 1, tm_val.tm_mday,
+ tm_val.tm_hour, tm_val.tm_min, tm_val.tm_sec);
+ if ((sizeof(h->bootcmd) - strnlen(h->bootcmd, sizeof(h->bootcmd))) <
+ ret) {
+ dev_err(&oct->pci_dev->dev, "Boot command buffer too small\n");
+ return -EINVAL;
+ }
+ strncat(h->bootcmd, date,
+ sizeof(h->bootcmd) - strnlen(h->bootcmd, sizeof(h->bootcmd)));
+
dev_info(&oct->pci_dev->dev, "Writing boot command: %s\n",
h->bootcmd);
/* Invoke the bootcmd */
ret = octeon_console_send_cmd(oct, h->bootcmd, 50);
+ if (ret)
+ dev_info(&oct->pci_dev->dev, "Boot command send failed\n");
- return 0;
+ return ret;
}
^ permalink raw reply related
* Re: [PATCH 0/3] fix reuseaddr regression
From: Josef Bacik @ 2017-09-23 0:28 UTC (permalink / raw)
To: David Miller; +Cc: josef, netdev, linux-kernel, crobinso, labbott, kernel-team
In-Reply-To: <20170919.135056.44228457394918392.davem@davemloft.net>
On Tue, Sep 19, 2017 at 01:50:56PM -0700, David Miller wrote:
> From: josef@toxicpanda.com
> Date: Mon, 18 Sep 2017 12:28:54 -0400
>
> > I introduced a regression when reworking the fastreuse port stuff that allows
> > bind conflicts to occur once a reuseaddr socket successfully opens on an
> > existing tb. The root cause is I reversed an if statement which caused us to
> > set the tb as if there were no owners on the socket if there were, which
> > obviously is not correct.
> >
> > Dave I have follow up patches that will add a selftest for this case and I ran
> > the other reuseport related tests as well. These need to go in pretty quickly
> > as it breaks kvm, I've marked them for stable. Sorry for the regression,
>
> First, please fix your "From: " field so that it actually has your full
> name rather than just your email address. This matter when I apply
> your patches.
>
> Second, remove the stable CC:. For networking changes, you simply ask
> me to queue the changes up for -stable.
>
Sorry Dave, I've fixed my git email settings and I droped the stable cc and sent
a new round. Didn't see this until just now, my bad.
Josef
^ permalink raw reply
* [PATCH 3/3] inet: fix improper empty comparison
From: Josef Bacik @ 2017-09-23 0:20 UTC (permalink / raw)
To: davem, netdev, kernel-team, linux-kernel; +Cc: Josef Bacik
In-Reply-To: <1506126008-9148-1-git-send-email-josef@toxicpanda.com>
From: Josef Bacik <jbacik@fb.com>
When doing my reuseport rework I screwed up and changed a
if (hlist_empty(&tb->owners))
to
if (!hlist_empty(&tb->owners))
This is obviously bad as all of the reuseport/reuse logic was reversed,
which caused weird problems like allowing an ipv4 bind conflict if we
opened an ipv4 only socket on a port followed by an ipv6 only socket on
the same port.
Fixes: b9470c27607b ("inet: kill smallest_size and smallest_port")
Reported-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
net/ipv4/inet_connection_sock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index a1bf30438bc5..c039c937ba90 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -321,7 +321,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
goto fail_unlock;
}
success:
- if (!hlist_empty(&tb->owners)) {
+ if (hlist_empty(&tb->owners)) {
tb->fastreuse = reuse;
if (sk->sk_reuseport) {
tb->fastreuseport = FASTREUSEPORT_ANY;
--
2.7.4
^ permalink raw reply related
* [PATCH 2/3] net: use inet6_rcv_saddr to compare sockets
From: Josef Bacik @ 2017-09-23 0:20 UTC (permalink / raw)
To: davem, netdev, kernel-team, linux-kernel; +Cc: Josef Bacik
In-Reply-To: <1506126008-9148-1-git-send-email-josef@toxicpanda.com>
From: Josef Bacik <jbacik@fb.com>
In ipv6_rcv_saddr_equal() we need to use inet6_rcv_saddr(sk) for the
ipv6 compare with the fast socket information to make sure we're doing
the proper comparisons.
Fixes: 637bc8bbe6c0 ("inet: reset tb->fastreuseport when adding a reuseport sk")
Reported-and-tested-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
net/ipv4/inet_connection_sock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index f87f4805e244..a1bf30438bc5 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -266,7 +266,7 @@ static inline int sk_reuseport_match(struct inet_bind_bucket *tb,
#if IS_ENABLED(CONFIG_IPV6)
if (tb->fast_sk_family == AF_INET6)
return ipv6_rcv_saddr_equal(&tb->fast_v6_rcv_saddr,
- &sk->sk_v6_rcv_saddr,
+ inet6_rcv_saddr(sk),
tb->fast_rcv_saddr,
sk->sk_rcv_saddr,
tb->fast_ipv6_only,
--
2.7.4
^ permalink raw reply related
* [PATCH 1/3] net: set tb->fast_sk_family
From: Josef Bacik @ 2017-09-23 0:20 UTC (permalink / raw)
To: davem, netdev, kernel-team, linux-kernel; +Cc: Josef Bacik
In-Reply-To: <1506126008-9148-1-git-send-email-josef@toxicpanda.com>
From: Josef Bacik <jbacik@fb.com>
We need to set the tb->fast_sk_family properly so we can use the proper
comparison function for all subsequent reuseport bind requests.
Fixes: 637bc8bbe6c0 ("inet: reset tb->fastreuseport when adding a reuseport sk")
Reported-and-tested-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
net/ipv4/inet_connection_sock.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index b9c64b40a83a..f87f4805e244 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -328,6 +328,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
tb->fastuid = uid;
tb->fast_rcv_saddr = sk->sk_rcv_saddr;
tb->fast_ipv6_only = ipv6_only_sock(sk);
+ tb->fast_sk_family = sk->sk_family;
#if IS_ENABLED(CONFIG_IPV6)
tb->fast_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
#endif
@@ -354,6 +355,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
tb->fastuid = uid;
tb->fast_rcv_saddr = sk->sk_rcv_saddr;
tb->fast_ipv6_only = ipv6_only_sock(sk);
+ tb->fast_sk_family = sk->sk_family;
#if IS_ENABLED(CONFIG_IPV6)
tb->fast_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
#endif
--
2.7.4
^ permalink raw reply related
* [PATCH 0/3] fix reuseaddr regression
From: Josef Bacik @ 2017-09-23 0:20 UTC (permalink / raw)
To: davem, netdev, kernel-team, linux-kernel
I introduced a regression when reworking the fastreuse port stuff that allows
bind conflicts to occur once a reuseaddr successfully opens on an existing tb.
The root cause is I reversed an if statement which caused us to set the tb as if
there were no owners on the socket if there were, which obviously is not
correct.
Dave could you please queue these changes up for -stable, I've run them through
the net tests and added another test to check for this problem specifically.
Thanks,
Josef
^ permalink raw reply
* [PATCH net-next 3/3] liquidio: update module parameter fw_type to reflect firmware type loaded
From: Felix Manlunas @ 2017-09-23 0:12 UTC (permalink / raw)
To: davem
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
ricardo.farrington
In-Reply-To: <20170923001206.GA1458@felix-thinkpad.cavium.com>
From: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
drivers/net/ethernet/cavium/liquidio/lio_main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index a3c9867..963803b 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -1934,10 +1934,12 @@ static int load_firmware(struct octeon_device *oct)
char fw_name[LIO_MAX_FW_FILENAME_LEN];
char *tmp_fw_type;
- if (fw_type_is_auto())
+ if (fw_type_is_auto()) {
tmp_fw_type = LIO_FW_NAME_TYPE_NIC;
- else
+ strncpy(fw_type, tmp_fw_type, sizeof(fw_type));
+ } else {
tmp_fw_type = fw_type;
+ }
sprintf(fw_name, "%s%s%s_%s%s", LIO_FW_DIR, LIO_FW_BASE_NAME,
octeon_get_conf(oct)->card_name, tmp_fw_type,
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 2/3] liquidio: verify firmware version when auto-loaded from flash.
From: Felix Manlunas @ 2017-09-23 0:12 UTC (permalink / raw)
To: davem
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
ricardo.farrington
In-Reply-To: <20170923001206.GA1458@felix-thinkpad.cavium.com>
From: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
drivers/net/ethernet/cavium/liquidio/lio_main.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index ce08f71..a3c9867 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -3303,7 +3303,7 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
{
struct lio *lio = NULL;
struct net_device *netdev;
- u8 mac[6], i, j;
+ u8 mac[6], i, j, *fw_ver;
struct octeon_soft_command *sc;
struct liquidio_if_cfg_context *ctx;
struct liquidio_if_cfg_resp *resp;
@@ -3414,6 +3414,22 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
goto setup_nic_dev_fail;
}
+ /* Verify f/w version (in case of 'auto' loading from flash) */
+ fw_ver = octeon_dev->fw_info.liquidio_firmware_version;
+ if (memcmp(LIQUIDIO_BASE_VERSION,
+ fw_ver,
+ strlen(LIQUIDIO_BASE_VERSION))) {
+ dev_err(&octeon_dev->pci_dev->dev,
+ "Unmatched firmware version. Expected %s.x, got %s.\n",
+ LIQUIDIO_BASE_VERSION, fw_ver);
+ goto setup_nic_dev_fail;
+ } else if (atomic_read(octeon_dev->adapter_fw_state) ==
+ FW_IS_PRELOADED) {
+ dev_info(&octeon_dev->pci_dev->dev,
+ "Using auto-loaded firmware version %s.\n",
+ fw_ver);
+ }
+
octeon_swap_8B_data((u64 *)(&resp->cfg_info),
(sizeof(struct liquidio_if_cfg_info)) >> 3);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 1/3] liquidio: allow override of firmware present in flash
From: Felix Manlunas @ 2017-09-23 0:12 UTC (permalink / raw)
To: davem
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
ricardo.farrington
In-Reply-To: <20170923001206.GA1458@felix-thinkpad.cavium.com>
From: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
drivers/net/ethernet/cavium/liquidio/lio_main.c | 68 ++++++++++++++--------
.../net/ethernet/cavium/liquidio/liquidio_image.h | 1 +
.../net/ethernet/cavium/liquidio/octeon_device.c | 11 +++-
.../net/ethernet/cavium/liquidio/octeon_device.h | 10 ++++
4 files changed, 64 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index e7f5494..ce08f71 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -59,9 +59,9 @@
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "NETIF_MSG debug bits");
-static char fw_type[LIO_MAX_FW_TYPE_LEN] = LIO_FW_NAME_TYPE_NIC;
+static char fw_type[LIO_MAX_FW_TYPE_LEN] = LIO_FW_NAME_TYPE_AUTO;
module_param_string(fw_type, fw_type, sizeof(fw_type), 0444);
-MODULE_PARM_DESC(fw_type, "Type of firmware to be loaded. Default \"nic\". Use \"none\" to load firmware from flash.");
+MODULE_PARM_DESC(fw_type, "Type of firmware to be loaded (default is \"auto\"), which uses firmware in flash, if present, else loads \"nic\".");
static u32 console_bitmask;
module_param(console_bitmask, int, 0644);
@@ -1115,10 +1115,10 @@ static int liquidio_watchdog(void *param)
return 0;
}
-static bool fw_type_is_none(void)
+static bool fw_type_is_auto(void)
{
- return strncmp(fw_type, LIO_FW_NAME_TYPE_NONE,
- sizeof(LIO_FW_NAME_TYPE_NONE)) == 0;
+ return strncmp(fw_type, LIO_FW_NAME_TYPE_AUTO,
+ sizeof(LIO_FW_NAME_TYPE_AUTO)) == 0;
}
/**
@@ -1302,7 +1302,7 @@ static void octeon_destroy_resources(struct octeon_device *oct)
* Implementation note: only soft-reset the device
* if it is a CN6XXX OR the LAST CN23XX device.
*/
- if (fw_type_is_none())
+ if (atomic_read(oct->adapter_fw_state) == FW_IS_PRELOADED)
octeon_pci_flr(oct);
else if (OCTEON_CN6XXX(oct) || !refcount)
oct->fn_list.soft_reset(oct);
@@ -1934,7 +1934,7 @@ static int load_firmware(struct octeon_device *oct)
char fw_name[LIO_MAX_FW_FILENAME_LEN];
char *tmp_fw_type;
- if (fw_type[0] == '\0')
+ if (fw_type_is_auto())
tmp_fw_type = LIO_FW_NAME_TYPE_NIC;
else
tmp_fw_type = fw_type;
@@ -3882,9 +3882,9 @@ static void nic_starter(struct work_struct *work)
static int octeon_device_init(struct octeon_device *octeon_dev)
{
int j, ret;
- int fw_loaded = 0;
char bootcmd[] = "\n";
char *dbg_enb = NULL;
+ enum lio_fw_state fw_state;
struct octeon_device_priv *oct_priv =
(struct octeon_device_priv *)octeon_dev->priv;
atomic_set(&octeon_dev->status, OCT_DEV_BEGIN_STATE);
@@ -3916,24 +3916,40 @@ static int octeon_device_init(struct octeon_device *octeon_dev)
octeon_dev->app_mode = CVM_DRV_INVALID_APP;
- if (OCTEON_CN23XX_PF(octeon_dev)) {
- if (!cn23xx_fw_loaded(octeon_dev) && !fw_type_is_none()) {
- fw_loaded = 0;
- /* Do a soft reset of the Octeon device. */
- if (octeon_dev->fn_list.soft_reset(octeon_dev))
- return 1;
- /* things might have changed */
- if (!cn23xx_fw_loaded(octeon_dev))
- fw_loaded = 0;
- else
- fw_loaded = 1;
- } else {
- fw_loaded = 1;
- }
- } else if (octeon_dev->fn_list.soft_reset(octeon_dev)) {
- return 1;
+ /* CN23XX supports preloaded firmware if the following is true:
+ *
+ * The adapter indicates that firmware is currently running AND
+ * 'fw_type' is 'auto'.
+ *
+ * (default state is NEEDS_TO_BE_LOADED, override it if appropriate).
+ */
+ if (OCTEON_CN23XX_PF(octeon_dev) &&
+ cn23xx_fw_loaded(octeon_dev) && fw_type_is_auto()) {
+ atomic_cmpxchg(octeon_dev->adapter_fw_state,
+ FW_NEEDS_TO_BE_LOADED, FW_IS_PRELOADED);
}
+ /* If loading firmware, only first device of adapter needs to do so. */
+ fw_state = atomic_cmpxchg(octeon_dev->adapter_fw_state,
+ FW_NEEDS_TO_BE_LOADED,
+ FW_IS_BEING_LOADED);
+
+ /* Here, [local variable] 'fw_state' is set to one of:
+ *
+ * FW_IS_PRELOADED: No firmware is to be loaded (see above)
+ * FW_NEEDS_TO_BE_LOADED: The driver's first instance will load
+ * firmware to the adapter.
+ * FW_IS_BEING_LOADED: The driver's second instance will not load
+ * firmware to the adapter.
+ */
+
+ /* Prior to f/w load, perform a soft reset of the Octeon device;
+ * if error resetting, return w/error.
+ */
+ if (fw_state == FW_NEEDS_TO_BE_LOADED)
+ if (octeon_dev->fn_list.soft_reset(octeon_dev))
+ return 1;
+
/* Initialize the dispatch mechanism used to push packets arriving on
* Octeon Output queues.
*/
@@ -4063,7 +4079,7 @@ static int octeon_device_init(struct octeon_device *octeon_dev)
atomic_set(&octeon_dev->status, OCT_DEV_IO_QUEUES_DONE);
- if ((!OCTEON_CN23XX_PF(octeon_dev)) || !fw_loaded) {
+ if (fw_state == FW_NEEDS_TO_BE_LOADED) {
dev_dbg(&octeon_dev->pci_dev->dev, "Waiting for DDR initialization...\n");
if (!ddr_timeout) {
dev_info(&octeon_dev->pci_dev->dev,
@@ -4125,6 +4141,8 @@ static int octeon_device_init(struct octeon_device *octeon_dev)
dev_err(&octeon_dev->pci_dev->dev, "Could not load firmware to board\n");
return 1;
}
+
+ atomic_set(octeon_dev->adapter_fw_state, FW_HAS_BEEN_LOADED);
}
handshake[octeon_dev->octeon_id].init_ok = 1;
diff --git a/drivers/net/ethernet/cavium/liquidio/liquidio_image.h b/drivers/net/ethernet/cavium/liquidio/liquidio_image.h
index 78a3685..5bf5e87 100644
--- a/drivers/net/ethernet/cavium/liquidio/liquidio_image.h
+++ b/drivers/net/ethernet/cavium/liquidio/liquidio_image.h
@@ -24,6 +24,7 @@
#define LIO_FW_BASE_NAME "lio_"
#define LIO_FW_NAME_SUFFIX ".bin"
#define LIO_FW_NAME_TYPE_NIC "nic"
+#define LIO_FW_NAME_TYPE_AUTO "auto"
#define LIO_FW_NAME_TYPE_NONE "none"
#define LIO_MAX_FIRMWARE_VERSION_LEN 16
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_device.c b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
index 29d53b1..e4aa339 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
@@ -541,6 +541,7 @@
static struct octeon_device *octeon_device[MAX_OCTEON_DEVICES];
static atomic_t adapter_refcounts[MAX_OCTEON_DEVICES];
+static atomic_t adapter_fw_states[MAX_OCTEON_DEVICES];
static u32 octeon_device_count;
/* locks device array (i.e. octeon_device[]) */
@@ -770,6 +771,10 @@ int octeon_register_device(struct octeon_device *oct,
oct->adapter_refcount = &adapter_refcounts[oct->octeon_id];
atomic_set(oct->adapter_refcount, 0);
+ /* Like the reference count, the f/w state is shared 'per-adapter' */
+ oct->adapter_fw_state = &adapter_fw_states[oct->octeon_id];
+ atomic_set(oct->adapter_fw_state, FW_NEEDS_TO_BE_LOADED);
+
spin_lock(&octeon_devices_lock);
for (idx = (int)oct->octeon_id - 1; idx >= 0; idx--) {
if (!octeon_device[idx]) {
@@ -780,11 +785,15 @@ int octeon_register_device(struct octeon_device *oct,
atomic_inc(oct->adapter_refcount);
return 1; /* here, refcount is guaranteed to be 1 */
}
- /* if another device is at same bus/dev, use its refcounter */
+ /* If another device is at same bus/dev, use its refcounter
+ * (and f/w state variable).
+ */
if ((octeon_device[idx]->loc.bus == bus) &&
(octeon_device[idx]->loc.dev == dev)) {
oct->adapter_refcount =
octeon_device[idx]->adapter_refcount;
+ oct->adapter_fw_state =
+ octeon_device[idx]->adapter_fw_state;
break;
}
}
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_device.h b/drivers/net/ethernet/cavium/liquidio/octeon_device.h
index 894af19..33d19c4 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_device.h
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.h
@@ -50,6 +50,13 @@ enum octeon_pci_swap_mode {
OCTEON_PCI_32BIT_LW_SWAP = 3
};
+enum lio_fw_state {
+ FW_IS_PRELOADED = 0,
+ FW_NEEDS_TO_BE_LOADED = 1,
+ FW_IS_BEING_LOADED = 2,
+ FW_HAS_BEEN_LOADED = 3,
+};
+
enum {
OCTEON_CONFIG_TYPE_DEFAULT = 0,
NUM_OCTEON_CONFS,
@@ -557,6 +564,9 @@ struct octeon_device {
} loc;
atomic_t *adapter_refcount; /* reference count of adapter */
+
+ atomic_t *adapter_fw_state; /* per-adapter, lio_fw_state */
+
bool ptp_enable;
};
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 0/3] liquidio: firmware loading
From: Felix Manlunas @ 2017-09-23 0:12 UTC (permalink / raw)
To: davem
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
ricardo.farrington
From: Rick Farrington <ricardo.farrington@cavium.com>
1. Allow host driver parameter to override auto-loaded firmware (in flash).
2. Verify version of firmware that is auto-loaded from flash.
3. Change value of fw_type module parameter to reflect default firmware
image name that is loaded by host driver (in /sys/module/liquidio/...)
drivers/net/ethernet/cavium/liquidio/lio_main.c | 90 +++++++++++++++-------
.../net/ethernet/cavium/liquidio/liquidio_image.h | 1 +
.../net/ethernet/cavium/liquidio/octeon_device.c | 11 ++-
.../net/ethernet/cavium/liquidio/octeon_device.h | 10 +++
4 files changed, 84 insertions(+), 28 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH net v2] net: orphan frags on stand-alone ptype in dev_queue_xmit_nit
From: Eric Dumazet @ 2017-09-22 23:50 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, davem
In-Reply-To: <20170922234237.43174-1-willemb@google.com>
On Fri, 2017-09-22 at 19:42 -0400, Willem de Bruijn wrote:
> Zerocopy skbs frags are copied when the skb is looped to a local sock.
> Commit 1080e512d44d ("net: orphan frags on receive") introduced calls
> to skb_orphan_frags to deliver_skb and __netif_receive_skb for this.
>
> With msg_zerocopy, these skbs can also exist in the tx path and thus
> loop from dev_queue_xmit_nit. This already calls deliver_skb in its
> loop. But it does not orphan before a separate pt_prev->func().
>
> Add the missing skb_orphan_frags_rx.
>
> Changes
> v1->v2: handle skb_orphan_frags_rx failure
>
> Fixes: 1f8b977ab32d ("sock: enable MSG_ZEROCOPY")
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> net/core/dev.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH net v2] net: orphan frags on stand-alone ptype in dev_queue_xmit_nit
From: Willem de Bruijn @ 2017-09-22 23:42 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
Zerocopy skbs frags are copied when the skb is looped to a local sock.
Commit 1080e512d44d ("net: orphan frags on receive") introduced calls
to skb_orphan_frags to deliver_skb and __netif_receive_skb for this.
With msg_zerocopy, these skbs can also exist in the tx path and thus
loop from dev_queue_xmit_nit. This already calls deliver_skb in its
loop. But it does not orphan before a separate pt_prev->func().
Add the missing skb_orphan_frags_rx.
Changes
v1->v2: handle skb_orphan_frags_rx failure
Fixes: 1f8b977ab32d ("sock: enable MSG_ZEROCOPY")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/core/dev.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 9a2254f9802f..588b473194a8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1948,8 +1948,12 @@ void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
goto again;
}
out_unlock:
- if (pt_prev)
- pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
+ if (pt_prev) {
+ if (!skb_orphan_frags_rx(skb2, GFP_ATOMIC))
+ pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
+ else
+ kfree_skb(skb2);
+ }
rcu_read_unlock();
}
EXPORT_SYMBOL_GPL(dev_queue_xmit_nit);
--
2.14.1.821.g8fa685d3b7-goog
^ permalink raw reply related
* Re: [PATCH net] net: orphan frags on stand-alone ptype in dev_queue_xmit_nit
From: Willem de Bruijn @ 2017-09-22 23:15 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Willem de Bruijn, Network Development, David Miller
In-Reply-To: <1506121446.29839.177.camel@edumazet-glaptop3.roam.corp.google.com>
On Fri, Sep 22, 2017 at 7:04 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2017-09-22 at 18:51 -0400, Willem de Bruijn wrote:
>> Zerocopy skbs frags are copied when the skb is looped to a local sock.
>> Commit 1080e512d44d ("net: orphan frags on receive") introduced calls
>> to skb_orphan_frags to deliver_skb and __netif_receive_skb.
>>
>> With msg_zerocopy, these skbs can also exist in the tx path and thus
>> loop from dev_queue_xmit_nit. This already calls deliver_skb in its
>> loop. But it does not orphan before a separate pt_prev->func().
>>
>> Add the missing skb_orphan_frags_rx.
>>
>> Fixes: 1f8b977ab32d ("sock: enable MSG_ZEROCOPY")
>> Signed-off-by: Willem de Bruijn <willemb@google.com>
>> ---
>> net/core/dev.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 9a2254f9802f..3f5b26ff4f74 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1948,7 +1948,7 @@ void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
>> goto again;
>> }
>> out_unlock:
>> - if (pt_prev)
>> + if (pt_prev && !skb_orphan_frags_rx(skb2, GFP_ATOMIC))
>> pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
>
> Don't you need to kfree_skb(skb2) in case of failure ?
Oh, yes, of course! :/ Will fix right away.
^ permalink raw reply
* Re: [PATCH net] net: orphan frags on stand-alone ptype in dev_queue_xmit_nit
From: Eric Dumazet @ 2017-09-22 23:04 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, davem
In-Reply-To: <20170922225141.126435-1-willemb@google.com>
On Fri, 2017-09-22 at 18:51 -0400, Willem de Bruijn wrote:
> Zerocopy skbs frags are copied when the skb is looped to a local sock.
> Commit 1080e512d44d ("net: orphan frags on receive") introduced calls
> to skb_orphan_frags to deliver_skb and __netif_receive_skb.
>
> With msg_zerocopy, these skbs can also exist in the tx path and thus
> loop from dev_queue_xmit_nit. This already calls deliver_skb in its
> loop. But it does not orphan before a separate pt_prev->func().
>
> Add the missing skb_orphan_frags_rx.
>
> Fixes: 1f8b977ab32d ("sock: enable MSG_ZEROCOPY")
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> net/core/dev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 9a2254f9802f..3f5b26ff4f74 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1948,7 +1948,7 @@ void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
> goto again;
> }
> out_unlock:
> - if (pt_prev)
> + if (pt_prev && !skb_orphan_frags_rx(skb2, GFP_ATOMIC))
> pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
Don't you need to kfree_skb(skb2) in case of failure ?
> rcu_read_unlock();
> }
^ permalink raw reply
* [PATCH net-next v3 2/2] net: dsa: add port enable and disable helpers
From: Vivien Didelot @ 2017-09-22 23:01 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170922230156.19521-1-vivien.didelot@savoirfairelinux.com>
Provide dsa_port_enable and dsa_port_disable helpers to respectively
enable and disable a switch port. This makes the dsa_port_set_state_now
helper static.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
net/dsa/dsa_priv.h | 3 ++-
net/dsa/port.c | 31 ++++++++++++++++++++++++++++++-
net/dsa/slave.c | 19 +++++--------------
3 files changed, 37 insertions(+), 16 deletions(-)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 9803952a5b40..0298a0f6a349 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -117,7 +117,8 @@ void dsa_master_ethtool_restore(struct net_device *dev);
/* port.c */
int dsa_port_set_state(struct dsa_port *dp, u8 state,
struct switchdev_trans *trans);
-void dsa_port_set_state_now(struct dsa_port *dp, u8 state);
+int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy);
+void dsa_port_disable(struct dsa_port *dp, struct phy_device *phy);
int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br);
void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br);
int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 76d43a82d397..72c8dbd3d3f2 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -56,7 +56,7 @@ int dsa_port_set_state(struct dsa_port *dp, u8 state,
return 0;
}
-void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
+static void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
{
int err;
@@ -65,6 +65,35 @@ void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
pr_err("DSA: failed to set STP state %u (%d)\n", state, err);
}
+int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy)
+{
+ u8 stp_state = dp->bridge_dev ? BR_STATE_BLOCKING : BR_STATE_FORWARDING;
+ struct dsa_switch *ds = dp->ds;
+ int port = dp->index;
+ int err;
+
+ if (ds->ops->port_enable) {
+ err = ds->ops->port_enable(ds, port, phy);
+ if (err)
+ return err;
+ }
+
+ dsa_port_set_state_now(dp, stp_state);
+
+ return 0;
+}
+
+void dsa_port_disable(struct dsa_port *dp, struct phy_device *phy)
+{
+ struct dsa_switch *ds = dp->ds;
+ int port = dp->index;
+
+ dsa_port_set_state_now(dp, BR_STATE_DISABLED);
+
+ if (ds->ops->port_disable)
+ ds->ops->port_disable(ds, port, phy);
+}
+
int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br)
{
struct dsa_notifier_bridge_info info = {
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index c2bb48579032..bd51ef56ec5b 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -73,9 +73,7 @@ static int dsa_slave_open(struct net_device *dev)
{
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_port *dp = p->dp;
- struct dsa_switch *ds = dp->ds;
struct net_device *master = dsa_master_netdev(p);
- u8 stp_state = dp->bridge_dev ? BR_STATE_BLOCKING : BR_STATE_FORWARDING;
int err;
if (!(master->flags & IFF_UP))
@@ -98,13 +96,9 @@ static int dsa_slave_open(struct net_device *dev)
goto clear_allmulti;
}
- if (ds->ops->port_enable) {
- err = ds->ops->port_enable(ds, p->dp->index, p->phy);
- if (err)
- goto clear_promisc;
- }
-
- dsa_port_set_state_now(p->dp, stp_state);
+ err = dsa_port_enable(dp, p->phy);
+ if (err)
+ goto clear_promisc;
if (p->phy)
phy_start(p->phy);
@@ -128,15 +122,12 @@ static int dsa_slave_close(struct net_device *dev)
{
struct dsa_slave_priv *p = netdev_priv(dev);
struct net_device *master = dsa_master_netdev(p);
- struct dsa_switch *ds = p->dp->ds;
+ struct dsa_port *dp = p->dp;
if (p->phy)
phy_stop(p->phy);
- dsa_port_set_state_now(p->dp, BR_STATE_DISABLED);
-
- if (ds->ops->port_disable)
- ds->ops->port_disable(ds, p->dp->index, p->phy);
+ dsa_port_disable(dp, p->phy);
dev_mc_unsync(master, dev);
dev_uc_unsync(master, dev);
--
2.14.1
^ permalink raw reply related
* [PATCH net-next v3 1/2] net: dsa: make slave close symmetrical to open
From: Vivien Didelot @ 2017-09-22 23:01 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170922230156.19521-1-vivien.didelot@savoirfairelinux.com>
The DSA slave open function configures the unicast MAC addresses on the
master device, enable the switch port, change its STP state, then start
the PHY device.
Make the close function symmetric, by first stopping the PHY device,
then changing the STP state, disabling the switch port and restore the
master device.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
net/dsa/slave.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 02ace7d462c4..c2bb48579032 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -133,6 +133,11 @@ static int dsa_slave_close(struct net_device *dev)
if (p->phy)
phy_stop(p->phy);
+ dsa_port_set_state_now(p->dp, BR_STATE_DISABLED);
+
+ if (ds->ops->port_disable)
+ ds->ops->port_disable(ds, p->dp->index, p->phy);
+
dev_mc_unsync(master, dev);
dev_uc_unsync(master, dev);
if (dev->flags & IFF_ALLMULTI)
@@ -143,11 +148,6 @@ static int dsa_slave_close(struct net_device *dev)
if (!ether_addr_equal(dev->dev_addr, master->dev_addr))
dev_uc_del(master, dev->dev_addr);
- if (ds->ops->port_disable)
- ds->ops->port_disable(ds, p->dp->index, p->phy);
-
- dsa_port_set_state_now(p->dp, BR_STATE_DISABLED);
-
return 0;
}
--
2.14.1
^ permalink raw reply related
* [PATCH net-next v3 0/2] net: dsa: port enabling
From: Vivien Didelot @ 2017-09-22 23:01 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
This patchset makes slave open and close symmetrical and provides
helpers for enabling or disabling a given DSA port.
Changes in v3:
- save the phy_device change for a future patchset
Changes in v2:
- do not remove the phy argument from port enable/disable
Vivien Didelot (2):
net: dsa: make slave close symmetrical to open
net: dsa: add port enable and disable helpers
net/dsa/dsa_priv.h | 3 ++-
net/dsa/port.c | 31 ++++++++++++++++++++++++++++++-
net/dsa/slave.c | 21 ++++++---------------
3 files changed, 38 insertions(+), 17 deletions(-)
--
2.14.1
^ permalink raw reply
* [PATCH net] net: orphan frags on stand-alone ptype in dev_queue_xmit_nit
From: Willem de Bruijn @ 2017-09-22 22:51 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
Zerocopy skbs frags are copied when the skb is looped to a local sock.
Commit 1080e512d44d ("net: orphan frags on receive") introduced calls
to skb_orphan_frags to deliver_skb and __netif_receive_skb.
With msg_zerocopy, these skbs can also exist in the tx path and thus
loop from dev_queue_xmit_nit. This already calls deliver_skb in its
loop. But it does not orphan before a separate pt_prev->func().
Add the missing skb_orphan_frags_rx.
Fixes: 1f8b977ab32d ("sock: enable MSG_ZEROCOPY")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 9a2254f9802f..3f5b26ff4f74 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1948,7 +1948,7 @@ void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
goto again;
}
out_unlock:
- if (pt_prev)
+ if (pt_prev && !skb_orphan_frags_rx(skb2, GFP_ATOMIC))
pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
rcu_read_unlock();
}
--
2.14.1.821.g8fa685d3b7-goog
^ permalink raw reply related
* [PATCH net-next] hv_netvsc: Fix the real number of queues of non-vRSS cases
From: Haiyang Zhang @ 2017-09-22 22:31 UTC (permalink / raw)
To: davem, netdev; +Cc: haiyangz, kys, olaf, vkuznets, linux-kernel
From: Haiyang Zhang <haiyangz@microsoft.com>
For older hosts without multi-channel (vRSS) support, and some error
cases, we still need to set the real number of queues to one.
This patch adds this missing setting.
Fixes: 8195b1396ec8 ("hv_netvsc: fix deadlock on hotplug")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index d4902ee5f260..68eac12fbf75 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1929,6 +1929,12 @@ static int netvsc_probe(struct hv_device *dev,
/* We always need headroom for rndis header */
net->needed_headroom = RNDIS_AND_PPI_SIZE;
+ /* Initialize the number of queues to be 1, we may change it if more
+ * channels are offered later.
+ */
+ netif_set_real_num_tx_queues(net, 1);
+ netif_set_real_num_rx_queues(net, 1);
+
/* Notify the netvsc driver of the new device */
memset(&device_info, 0, sizeof(device_info));
device_info.ring_size = ring_size;
--
2.14.1
^ permalink raw reply related
* Re: [RFC PATCH 00/11] udp: full early demux for unconnected sockets
From: Eric Dumazet @ 2017-09-22 21:58 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, David S. Miller, Pablo Neira Ayuso, Florian Westphal,
Eric Dumazet, Hannes Frederic Sowa
In-Reply-To: <cover.1506114055.git.pabeni@redhat.com>
On Fri, 2017-09-22 at 23:06 +0200, Paolo Abeni wrote:
> This series refactor the UDP early demux code so that:
>
> * full socket lookup is performed for unicast packets
> * a sk is grabbed even for unconnected socket match
> * a dst cache is used even in such scenario
>
> To perform this tasks a couple of facilities are added:
>
> * noref socket references, scoped inside the current RCU section, to be
> explicitly cleared before leaving such section
> * a dst cache inside the inet and inet6 local addresses tables, caching the
> related local dst entry
>
> The measured performance gain under small packet UDP flood is as follow:
>
> ingress NIC vanilla patched delta
> rx queues (kpps) (kpps) (%)
> [ipv4]
> 1 2177 2414 10
> 2 2527 2892 14
> 3 3050 3733 22
This is a clear sign your program is not using latest SO_REUSEPORT +
[ec]BPF filter [1]
return socket[RX_QUEUE# | or CPU#];
If udp_sink uses SO_REUSEPORT with no extra hint, socket selection is
based on a lazy hash, meaning that you do not have proper siloing.
return socket[hash(skb)];
Multiple cpus can then :
- compete on grabbing same socket refcount
- compete on grabbing the receive queue lock
- compete for releasing lock and socket refcount
- skb freeing done on different cpus than where allocated.
You are adding complexity to the kernel because you are using a
sub-optimal user space program, favoring false sharing.
First solve the false sharing issue.
Performance with 2 rx queues should be almost twice the performance with
1 rx queue.
Then we can see if the gains you claim are still applicable.
Thanks
PS: Wei Wan is about to release the IPV6 changes so that the big
differences you showed are going to disappear soon.
Refs [1]
tools/testing/selftests/net/reuseport_bpf.c
6a5ef90c58daada158ba16ba330558efc3471491 Merge branch 'faster-soreuseport'
3ca8e4029969d40ab90e3f1ecd83ab1cadd60fbb soreuseport: BPF selection functional test
538950a1b7527a0a52ccd9337e3fcd304f027f13 soreuseport: setsockopt SO_ATTACH_REUSEPORT_[CE]BPF
e32ea7e747271a0abcd37e265005e97cc81d9df5 soreuseport: fast reuseport UDP socket selection
ef456144da8ef507c8cf504284b6042e9201a05c soreuseport: define reuseport groups
^ permalink raw reply
* [PATCH v4 5/9] r820t: fix r820t_write_reg for KASAN
From: Arnd Bergmann @ 2017-09-22 21:29 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Arnd Bergmann, Jiri Pirko, Arend van Spriel, Kalle Valo,
David S. Miller, Andrey Ryabinin, Alexander Potapenko,
Dmitry Vyukov, Masahiro Yamada, Michal Marek, Andrew Morton,
Kees Cook, Geert Uytterhoeven, Greg Kroah-Hartman, linux-media,
linux-kernel, netdev, linux-wireless, brcm80211-dev-list.pdl,
brcm80211-dev-list, kasan-dev
In-Reply-To: <20170922212930.620249-1-arnd@arndb.de>
With CONFIG_KASAN, we get an overly long stack frame due to inlining
the register access functions:
drivers/media/tuners/r820t.c: In function 'generic_set_freq.isra.7':
drivers/media/tuners/r820t.c:1334:1: error: the frame size of 2880 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
This is caused by a gcc bug that has now been fixed in gcc-8.
To work around the problem, we can pass the register data
through a local variable that older gcc versions can optimize
out as well.
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/tuners/r820t.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
index ba80376a3b86..d097eb04a0e9 100644
--- a/drivers/media/tuners/r820t.c
+++ b/drivers/media/tuners/r820t.c
@@ -396,9 +396,11 @@ static int r820t_write(struct r820t_priv *priv, u8 reg, const u8 *val,
return 0;
}
-static int r820t_write_reg(struct r820t_priv *priv, u8 reg, u8 val)
+static inline int r820t_write_reg(struct r820t_priv *priv, u8 reg, u8 val)
{
- return r820t_write(priv, reg, &val, 1);
+ u8 tmp = val; /* work around GCC PR81715 with asan-stack=1 */
+
+ return r820t_write(priv, reg, &tmp, 1);
}
static int r820t_read_cache_reg(struct r820t_priv *priv, int reg)
@@ -411,17 +413,18 @@ static int r820t_read_cache_reg(struct r820t_priv *priv, int reg)
return -EINVAL;
}
-static int r820t_write_reg_mask(struct r820t_priv *priv, u8 reg, u8 val,
+static inline int r820t_write_reg_mask(struct r820t_priv *priv, u8 reg, u8 val,
u8 bit_mask)
{
+ u8 tmp = val;
int rc = r820t_read_cache_reg(priv, reg);
if (rc < 0)
return rc;
- val = (rc & ~bit_mask) | (val & bit_mask);
+ tmp = (rc & ~bit_mask) | (tmp & bit_mask);
- return r820t_write(priv, reg, &val, 1);
+ return r820t_write(priv, reg, &tmp, 1);
}
static int r820t_read(struct r820t_priv *priv, u8 reg, u8 *val, int len)
--
2.9.0
^ permalink raw reply related
* [PATCH v4 9/9] kasan: rework Kconfig settings
From: Arnd Bergmann @ 2017-09-22 21:29 UTC (permalink / raw)
To: Andrey Ryabinin, Masahiro Yamada, Michal Marek, Andrew Morton
Cc: Arnd Bergmann, Mauro Carvalho Chehab, Jiri Pirko,
Arend van Spriel, Kalle Valo, David S. Miller,
Alexander Potapenko, Dmitry Vyukov, Kees Cook, Geert Uytterhoeven,
Greg Kroah-Hartman, linux-media, linux-kernel, netdev,
linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
kasan-dev, linux-kbuild, Jakub Jelinek, Martin Liška
In-Reply-To: <20170922212930.620249-1-arnd@arndb.de>
We get a lot of very large stack frames using gcc-7.0.1 with the default
-fsanitize-address-use-after-scope --param asan-stack=1 options, which
can easily cause an overflow of the kernel stack, e.g.
drivers/gpu/drm/i915/gvt/handlers.c:2407:1: error: the frame size of 31216 bytes is larger than 2048 bytes
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5650:1: error: the frame size of 23632 bytes is larger than 2048 bytes
drivers/scsi/fnic/fnic_trace.c:451:1: error: the frame size of 5152 bytes is larger than 2048 bytes
fs/btrfs/relocation.c:1202:1: error: the frame size of 4256 bytes is larger than 2048 bytes
fs/fscache/stats.c:287:1: error: the frame size of 6552 bytes is larger than 2048 bytes
lib/atomic64_test.c:250:1: error: the frame size of 12616 bytes is larger than 2048 bytes
mm/vmscan.c:1367:1: error: the frame size of 5080 bytes is larger than 2048 bytes
net/wireless/nl80211.c:1905:1: error: the frame size of 4232 bytes is larger than 2048 bytes
To reduce this risk, -fsanitize-address-use-after-scope is now split
out into a separate CONFIG_KASAN_EXTRA Kconfig option, leading to stack
frames that are smaller than 2 kilobytes most of the time on x86_64. An
earlier version of this patch also prevented combining KASAN_EXTRA with
KASAN_INLINE, but that is no longer necessary with gcc-7.0.1.
A lot of warnings with KASAN_EXTRA go away if we disable KMEMCHECK,
as -fsanitize-address-use-after-scope seems to understand the builtin
memcpy, but adds checking code around an extern memcpy call. I had to work
around a circular dependency, as DEBUG_SLAB/SLUB depended on !KMEMCHECK,
while KASAN did it the other way round. Now we handle both the same way
and make KASAN and KMEMCHECK mutually exclusive.
All patches to get the frame size below 2048 bytes with CONFIG_KASAN=y
and CONFIG_KASAN_EXTRA=n have been submitted along with this patch, so
we can bring back that default now. KASAN_EXTRA=y still causes lots of
warnings but now defaults to !COMPILE_TEST to disable it in allmodconfig,
and it remains disabled in all other defconfigs since it is a new option.
I arbitrarily raise the warning limit for KASAN_EXTRA to 3072 to reduce
the noise, but an allmodconfig kernel still has around 50 warnings
on gcc-7.
I experimented a bit more with smaller stack frames and have another
follow-up series that reduces the warning limit for 64-bit architectures
to 1280 bytes (without CONFIG_KASAN).
With earlier versions of this patch series, I also had patches to
address the warnings we get with KASAN and/or KASAN_EXTRA, using a
"noinline_if_stackbloat" annotation. That annotation now got replaced with
a gcc-8 bugfix (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715)
and a workaround for older compilers, which means that KASAN_EXTRA is
now just as bad as before and will lead to an instant stack overflow in
a few extreme cases.
This reverts parts of commit commit 3f181b4 ("lib/Kconfig.debug: disable
-Wframe-larger-than warnings with KASAN=y").
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
lib/Kconfig.debug | 4 ++--
lib/Kconfig.kasan | 13 ++++++++++++-
lib/Kconfig.kmemcheck | 1 +
scripts/Makefile.kasan | 3 +++
4 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index b19c491cbc4e..5755875d4a80 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -217,7 +217,7 @@ config ENABLE_MUST_CHECK
config FRAME_WARN
int "Warn for stack frames larger than (needs gcc 4.4)"
range 0 8192
- default 0 if KASAN
+ default 3072 if KASAN_EXTRA
default 2048 if GCC_PLUGIN_LATENT_ENTROPY
default 1024 if !64BIT
default 2048 if 64BIT
@@ -503,7 +503,7 @@ config DEBUG_OBJECTS_ENABLE_DEFAULT
config DEBUG_SLAB
bool "Debug slab memory allocations"
- depends on DEBUG_KERNEL && SLAB && !KMEMCHECK
+ depends on DEBUG_KERNEL && SLAB && !KMEMCHECK && !KASAN
help
Say Y here to have the kernel do limited verification on memory
allocation as well as poisoning memory on free to catch use of freed
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index bd38aab05929..db799e6e9dba 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -5,7 +5,7 @@ if HAVE_ARCH_KASAN
config KASAN
bool "KASan: runtime memory debugger"
- depends on SLUB || (SLAB && !DEBUG_SLAB)
+ depends on SLUB || SLAB
select CONSTRUCTORS
select STACKDEPOT
help
@@ -20,6 +20,17 @@ config KASAN
Currently CONFIG_KASAN doesn't work with CONFIG_DEBUG_SLAB
(the resulting kernel does not boot).
+config KASAN_EXTRA
+ bool "KAsan: extra checks"
+ depends on KASAN && DEBUG_KERNEL && !COMPILE_TEST
+ help
+ This enables further checks in the kernel address sanitizer, for now
+ it only includes the address-use-after-scope check that can lead
+ to excessive kernel stack usage, frame size warnings and longer
+ compile time.
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715 has more
+
+
choice
prompt "Instrumentation type"
depends on KASAN
diff --git a/lib/Kconfig.kmemcheck b/lib/Kconfig.kmemcheck
index 846e039a86b4..1a534e638635 100644
--- a/lib/Kconfig.kmemcheck
+++ b/lib/Kconfig.kmemcheck
@@ -7,6 +7,7 @@ menuconfig KMEMCHECK
bool "kmemcheck: trap use of uninitialized memory"
depends on DEBUG_KERNEL
depends on !X86_USE_3DNOW
+ depends on !KASAN
depends on SLUB || SLAB
depends on !CC_OPTIMIZE_FOR_SIZE
depends on !FUNCTION_TRACER
diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
index 9576775a86f6..3b3148faf866 100644
--- a/scripts/Makefile.kasan
+++ b/scripts/Makefile.kasan
@@ -29,5 +29,8 @@ else
endif
endif
+ifdef CONFIG_KASAN_EXTRA
CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
endif
+
+endif
--
2.9.0
^ permalink raw reply related
* [PATCH v4 8/9] netlink: fix nla_put_{u8,u16,u32} for KASAN
From: Arnd Bergmann @ 2017-09-22 21:29 UTC (permalink / raw)
To: David S. Miller
Cc: Arnd Bergmann, Mauro Carvalho Chehab, Jiri Pirko,
Arend van Spriel, Kalle Valo, Andrey Ryabinin,
Alexander Potapenko, Dmitry Vyukov, Masahiro Yamada, Michal Marek,
Andrew Morton, Kees Cook, Geert Uytterhoeven, Greg Kroah-Hartman,
linux-media, linux-kernel, netdev, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, kasan-dev
In-Reply-To: <20170922212930.620249-1-arnd@arndb.de>
When CONFIG_KASAN is enabled, the "--param asan-stack=1" causes rather large
stack frames in some functions. This goes unnoticed normally because
CONFIG_FRAME_WARN is disabled with CONFIG_KASAN by default as of commit
3f181b4d8652 ("lib/Kconfig.debug: disable -Wframe-larger-than warnings with
KASAN=y").
The kernelci.org build bot however has the warning enabled and that led
me to investigate it a little further, as every build produces these warnings:
net/wireless/nl80211.c:4389:1: warning: the frame size of 2240 bytes is larger than 2048 bytes [-Wframe-larger-than=]
net/wireless/nl80211.c:1895:1: warning: the frame size of 3776 bytes is larger than 2048 bytes [-Wframe-larger-than=]
net/wireless/nl80211.c:1410:1: warning: the frame size of 2208 bytes is larger than 2048 bytes [-Wframe-larger-than=]
net/bridge/br_netlink.c:1282:1: warning: the frame size of 2544 bytes is larger than 2048 bytes [-Wframe-larger-than=]
Most of this problem is now solved in gcc-8, which can consolidate
the stack slots for the inline function arguments. On older compilers
we can add a workaround by declaring a local variable in each function
to pass the inline function argument.
Cc: stable@vger.kernel.org
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/net/netlink.h | 73 ++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 55 insertions(+), 18 deletions(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index e51cf5f81597..14c289393071 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -773,7 +773,10 @@ static inline int nla_parse_nested(struct nlattr *tb[], int maxtype,
*/
static inline int nla_put_u8(struct sk_buff *skb, int attrtype, u8 value)
{
- return nla_put(skb, attrtype, sizeof(u8), &value);
+ /* temporary variables to work around GCC PR81715 with asan-stack=1 */
+ u8 tmp = value;
+
+ return nla_put(skb, attrtype, sizeof(u8), &tmp);
}
/**
@@ -784,7 +787,9 @@ static inline int nla_put_u8(struct sk_buff *skb, int attrtype, u8 value)
*/
static inline int nla_put_u16(struct sk_buff *skb, int attrtype, u16 value)
{
- return nla_put(skb, attrtype, sizeof(u16), &value);
+ u16 tmp = value;
+
+ return nla_put(skb, attrtype, sizeof(u16), &tmp);
}
/**
@@ -795,7 +800,9 @@ static inline int nla_put_u16(struct sk_buff *skb, int attrtype, u16 value)
*/
static inline int nla_put_be16(struct sk_buff *skb, int attrtype, __be16 value)
{
- return nla_put(skb, attrtype, sizeof(__be16), &value);
+ __be16 tmp = value;
+
+ return nla_put(skb, attrtype, sizeof(__be16), &tmp);
}
/**
@@ -806,7 +813,9 @@ static inline int nla_put_be16(struct sk_buff *skb, int attrtype, __be16 value)
*/
static inline int nla_put_net16(struct sk_buff *skb, int attrtype, __be16 value)
{
- return nla_put_be16(skb, attrtype | NLA_F_NET_BYTEORDER, value);
+ __be16 tmp = value;
+
+ return nla_put_be16(skb, attrtype | NLA_F_NET_BYTEORDER, tmp);
}
/**
@@ -817,7 +826,9 @@ static inline int nla_put_net16(struct sk_buff *skb, int attrtype, __be16 value)
*/
static inline int nla_put_le16(struct sk_buff *skb, int attrtype, __le16 value)
{
- return nla_put(skb, attrtype, sizeof(__le16), &value);
+ __le16 tmp = value;
+
+ return nla_put(skb, attrtype, sizeof(__le16), &tmp);
}
/**
@@ -828,7 +839,9 @@ static inline int nla_put_le16(struct sk_buff *skb, int attrtype, __le16 value)
*/
static inline int nla_put_u32(struct sk_buff *skb, int attrtype, u32 value)
{
- return nla_put(skb, attrtype, sizeof(u32), &value);
+ u32 tmp = value;
+
+ return nla_put(skb, attrtype, sizeof(u32), &tmp);
}
/**
@@ -839,7 +852,9 @@ static inline int nla_put_u32(struct sk_buff *skb, int attrtype, u32 value)
*/
static inline int nla_put_be32(struct sk_buff *skb, int attrtype, __be32 value)
{
- return nla_put(skb, attrtype, sizeof(__be32), &value);
+ __be32 tmp = value;
+
+ return nla_put(skb, attrtype, sizeof(__be32), &tmp);
}
/**
@@ -850,7 +865,9 @@ static inline int nla_put_be32(struct sk_buff *skb, int attrtype, __be32 value)
*/
static inline int nla_put_net32(struct sk_buff *skb, int attrtype, __be32 value)
{
- return nla_put_be32(skb, attrtype | NLA_F_NET_BYTEORDER, value);
+ __be32 tmp = value;
+
+ return nla_put_be32(skb, attrtype | NLA_F_NET_BYTEORDER, tmp);
}
/**
@@ -861,7 +878,9 @@ static inline int nla_put_net32(struct sk_buff *skb, int attrtype, __be32 value)
*/
static inline int nla_put_le32(struct sk_buff *skb, int attrtype, __le32 value)
{
- return nla_put(skb, attrtype, sizeof(__le32), &value);
+ __le32 tmp = value;
+
+ return nla_put(skb, attrtype, sizeof(__le32), &tmp);
}
/**
@@ -874,7 +893,9 @@ static inline int nla_put_le32(struct sk_buff *skb, int attrtype, __le32 value)
static inline int nla_put_u64_64bit(struct sk_buff *skb, int attrtype,
u64 value, int padattr)
{
- return nla_put_64bit(skb, attrtype, sizeof(u64), &value, padattr);
+ u64 tmp = value;
+
+ return nla_put_64bit(skb, attrtype, sizeof(u64), &tmp, padattr);
}
/**
@@ -887,7 +908,9 @@ static inline int nla_put_u64_64bit(struct sk_buff *skb, int attrtype,
static inline int nla_put_be64(struct sk_buff *skb, int attrtype, __be64 value,
int padattr)
{
- return nla_put_64bit(skb, attrtype, sizeof(__be64), &value, padattr);
+ __be64 tmp = value;
+
+ return nla_put_64bit(skb, attrtype, sizeof(__be64), &tmp, padattr);
}
/**
@@ -900,7 +923,9 @@ static inline int nla_put_be64(struct sk_buff *skb, int attrtype, __be64 value,
static inline int nla_put_net64(struct sk_buff *skb, int attrtype, __be64 value,
int padattr)
{
- return nla_put_be64(skb, attrtype | NLA_F_NET_BYTEORDER, value,
+ __be64 tmp = value;
+
+ return nla_put_be64(skb, attrtype | NLA_F_NET_BYTEORDER, tmp,
padattr);
}
@@ -914,7 +939,9 @@ static inline int nla_put_net64(struct sk_buff *skb, int attrtype, __be64 value,
static inline int nla_put_le64(struct sk_buff *skb, int attrtype, __le64 value,
int padattr)
{
- return nla_put_64bit(skb, attrtype, sizeof(__le64), &value, padattr);
+ __le64 tmp = value;
+
+ return nla_put_64bit(skb, attrtype, sizeof(__le64), &tmp, padattr);
}
/**
@@ -925,7 +952,9 @@ static inline int nla_put_le64(struct sk_buff *skb, int attrtype, __le64 value,
*/
static inline int nla_put_s8(struct sk_buff *skb, int attrtype, s8 value)
{
- return nla_put(skb, attrtype, sizeof(s8), &value);
+ s8 tmp = value;
+
+ return nla_put(skb, attrtype, sizeof(s8), &tmp);
}
/**
@@ -936,7 +965,9 @@ static inline int nla_put_s8(struct sk_buff *skb, int attrtype, s8 value)
*/
static inline int nla_put_s16(struct sk_buff *skb, int attrtype, s16 value)
{
- return nla_put(skb, attrtype, sizeof(s16), &value);
+ s16 tmp = value;
+
+ return nla_put(skb, attrtype, sizeof(s16), &tmp);
}
/**
@@ -947,7 +978,9 @@ static inline int nla_put_s16(struct sk_buff *skb, int attrtype, s16 value)
*/
static inline int nla_put_s32(struct sk_buff *skb, int attrtype, s32 value)
{
- return nla_put(skb, attrtype, sizeof(s32), &value);
+ s32 tmp = value;
+
+ return nla_put(skb, attrtype, sizeof(s32), &tmp);
}
/**
@@ -960,7 +993,9 @@ static inline int nla_put_s32(struct sk_buff *skb, int attrtype, s32 value)
static inline int nla_put_s64(struct sk_buff *skb, int attrtype, s64 value,
int padattr)
{
- return nla_put_64bit(skb, attrtype, sizeof(s64), &value, padattr);
+ s64 tmp = value;
+
+ return nla_put_64bit(skb, attrtype, sizeof(s64), &tmp, padattr);
}
/**
@@ -1010,7 +1045,9 @@ static inline int nla_put_msecs(struct sk_buff *skb, int attrtype,
static inline int nla_put_in_addr(struct sk_buff *skb, int attrtype,
__be32 addr)
{
- return nla_put_be32(skb, attrtype, addr);
+ __be32 tmp = addr;
+
+ return nla_put_be32(skb, attrtype, tmp);
}
/**
--
2.9.0
^ permalink raw reply related
* [PATCH v4 7/9] rocker: fix rocker_tlv_put_* functions for KASAN
From: Arnd Bergmann @ 2017-09-22 21:29 UTC (permalink / raw)
To: Jiri Pirko
Cc: Arnd Bergmann, Mauro Carvalho Chehab, Arend van Spriel,
Kalle Valo, David S. Miller, Andrey Ryabinin, Alexander Potapenko,
Dmitry Vyukov, Masahiro Yamada, Michal Marek, Andrew Morton,
Kees Cook, Geert Uytterhoeven, Greg Kroah-Hartman, linux-media,
linux-kernel, netdev, linux-wireless, brcm80211-dev-list.pdl,
brcm80211-dev-list, kasan
In-Reply-To: <20170922212930.620249-1-arnd@arndb.de>
Inlining these functions creates lots of stack variables that each take
64 bytes when KASAN is enabled, leading to this warning about potential
stack overflow:
drivers/net/ethernet/rocker/rocker_ofdpa.c: In function 'ofdpa_cmd_flow_tbl_add':
drivers/net/ethernet/rocker/rocker_ofdpa.c:621:1: error: the frame size of 2752 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
gcc-8 can now consolidate the stack slots itself, but on older versions
we get the same behavior by using a temporary variable that holds a
copy of the inline function argument.
Cc: stable@vger.kernel.org
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/rocker/rocker_tlv.h | 48 ++++++++++++++++++++------------
1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/rocker/rocker_tlv.h b/drivers/net/ethernet/rocker/rocker_tlv.h
index a63ef82e7c72..dfae3c9d57c6 100644
--- a/drivers/net/ethernet/rocker/rocker_tlv.h
+++ b/drivers/net/ethernet/rocker/rocker_tlv.h
@@ -139,40 +139,52 @@ rocker_tlv_start(struct rocker_desc_info *desc_info)
int rocker_tlv_put(struct rocker_desc_info *desc_info,
int attrtype, int attrlen, const void *data);
-static inline int rocker_tlv_put_u8(struct rocker_desc_info *desc_info,
- int attrtype, u8 value)
+static inline int
+rocker_tlv_put_u8(struct rocker_desc_info *desc_info, int attrtype, u8 value)
{
- return rocker_tlv_put(desc_info, attrtype, sizeof(u8), &value);
+ u8 tmp = value; /* work around GCC PR81715 */
+
+ return rocker_tlv_put(desc_info, attrtype, sizeof(u8), &tmp);
}
-static inline int rocker_tlv_put_u16(struct rocker_desc_info *desc_info,
- int attrtype, u16 value)
+static inline int
+rocker_tlv_put_u16(struct rocker_desc_info *desc_info, int attrtype, u16 value)
{
- return rocker_tlv_put(desc_info, attrtype, sizeof(u16), &value);
+ u16 tmp = value;
+
+ return rocker_tlv_put(desc_info, attrtype, sizeof(u16), &tmp);
}
-static inline int rocker_tlv_put_be16(struct rocker_desc_info *desc_info,
- int attrtype, __be16 value)
+static inline int
+rocker_tlv_put_be16(struct rocker_desc_info *desc_info, int attrtype, __be16 value)
{
- return rocker_tlv_put(desc_info, attrtype, sizeof(__be16), &value);
+ __be16 tmp = value;
+
+ return rocker_tlv_put(desc_info, attrtype, sizeof(__be16), &tmp);
}
-static inline int rocker_tlv_put_u32(struct rocker_desc_info *desc_info,
- int attrtype, u32 value)
+static inline int
+rocker_tlv_put_u32(struct rocker_desc_info *desc_info, int attrtype, u32 value)
{
- return rocker_tlv_put(desc_info, attrtype, sizeof(u32), &value);
+ u32 tmp = value;
+
+ return rocker_tlv_put(desc_info, attrtype, sizeof(u32), &tmp);
}
-static inline int rocker_tlv_put_be32(struct rocker_desc_info *desc_info,
- int attrtype, __be32 value)
+static inline int
+rocker_tlv_put_be32(struct rocker_desc_info *desc_info, int attrtype, __be32 value)
{
- return rocker_tlv_put(desc_info, attrtype, sizeof(__be32), &value);
+ __be32 tmp = value;
+
+ return rocker_tlv_put(desc_info, attrtype, sizeof(__be32), &tmp);
}
-static inline int rocker_tlv_put_u64(struct rocker_desc_info *desc_info,
- int attrtype, u64 value)
+static inline int
+rocker_tlv_put_u64(struct rocker_desc_info *desc_info, int attrtype, u64 value)
{
- return rocker_tlv_put(desc_info, attrtype, sizeof(u64), &value);
+ u64 tmp = value;
+
+ return rocker_tlv_put(desc_info, attrtype, sizeof(u64), &tmp);
}
static inline struct rocker_tlv *
--
2.9.0
^ permalink raw reply related
* [PATCH v4 6/9] dvb-frontends: fix i2c access helpers for KASAN
From: Arnd Bergmann @ 2017-09-22 21:29 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Arnd Bergmann, Jiri Pirko, Arend van Spriel, Kalle Valo,
David S. Miller, Andrey Ryabinin, Alexander Potapenko,
Dmitry Vyukov, Masahiro Yamada, Michal Marek, Andrew Morton,
Kees Cook, Geert Uytterhoeven, Greg Kroah-Hartman, linux-media,
linux-kernel, netdev, linux-wireless, brcm80211-dev-list.pdl,
brcm80211-dev-list, kasan-dev
In-Reply-To: <20170922212930.620249-1-arnd@arndb.de>
A typical code fragment was copied across many dvb-frontend drivers and
causes large stack frames when built with with CONFIG_KASAN on gcc-5/6/7:
drivers/media/dvb-frontends/cxd2841er.c:3225:1: error: the frame size of 3992 bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
drivers/media/dvb-frontends/cxd2841er.c:3404:1: error: the frame size of 3136 bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
drivers/media/dvb-frontends/stv0367.c:3143:1: error: the frame size of 4016 bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
drivers/media/dvb-frontends/stv090x.c:3430:1: error: the frame size of 5312 bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
drivers/media/dvb-frontends/stv090x.c:4248:1: error: the frame size of 4872 bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
gcc-8 now solves this by consolidating the stack slots for the argument
variables, but on older compilers we can get the same behavior by taking
the pointer of a local variable rather than the inline function argument.
Cc: stable@vger.kernel.org
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I'm undecided here whether there should be a comment pointing
to PR81715 for each file that the bogus local variable workaround
to prevent it from being cleaned up again. It's probably not
necessary since anything that causes actual problems would also
trigger a build warning.
---
drivers/media/dvb-frontends/ascot2e.c | 4 +++-
drivers/media/dvb-frontends/cxd2841er.c | 4 +++-
drivers/media/dvb-frontends/helene.c | 4 +++-
drivers/media/dvb-frontends/horus3a.c | 4 +++-
drivers/media/dvb-frontends/itd1000.c | 5 +++--
drivers/media/dvb-frontends/mt312.c | 4 +++-
drivers/media/dvb-frontends/stb0899_drv.c | 3 ++-
drivers/media/dvb-frontends/stb6100.c | 6 ++++--
drivers/media/dvb-frontends/stv0367.c | 4 +++-
drivers/media/dvb-frontends/stv090x.c | 4 +++-
drivers/media/dvb-frontends/stv6110x.c | 4 +++-
drivers/media/dvb-frontends/zl10039.c | 4 +++-
12 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c
index 0ee0df53b91b..1219272ca3f0 100644
--- a/drivers/media/dvb-frontends/ascot2e.c
+++ b/drivers/media/dvb-frontends/ascot2e.c
@@ -155,7 +155,9 @@ static int ascot2e_write_regs(struct ascot2e_priv *priv,
static int ascot2e_write_reg(struct ascot2e_priv *priv, u8 reg, u8 val)
{
- return ascot2e_write_regs(priv, reg, &val, 1);
+ u8 tmp = val;
+
+ return ascot2e_write_regs(priv, reg, &tmp, 1);
}
static int ascot2e_read_regs(struct ascot2e_priv *priv,
diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c
index 48ee9bc00c06..b7574deff5c6 100644
--- a/drivers/media/dvb-frontends/cxd2841er.c
+++ b/drivers/media/dvb-frontends/cxd2841er.c
@@ -257,7 +257,9 @@ static int cxd2841er_write_regs(struct cxd2841er_priv *priv,
static int cxd2841er_write_reg(struct cxd2841er_priv *priv,
u8 addr, u8 reg, u8 val)
{
- return cxd2841er_write_regs(priv, addr, reg, &val, 1);
+ u8 tmp = val;
+
+ return cxd2841er_write_regs(priv, addr, reg, &tmp, 1);
}
static int cxd2841er_read_regs(struct cxd2841er_priv *priv,
diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c
index 4bf5a551ba40..6e93f2d1575b 100644
--- a/drivers/media/dvb-frontends/helene.c
+++ b/drivers/media/dvb-frontends/helene.c
@@ -331,7 +331,9 @@ static int helene_write_regs(struct helene_priv *priv,
static int helene_write_reg(struct helene_priv *priv, u8 reg, u8 val)
{
- return helene_write_regs(priv, reg, &val, 1);
+ u8 tmp = val;
+
+ return helene_write_regs(priv, reg, &tmp, 1);
}
static int helene_read_regs(struct helene_priv *priv,
diff --git a/drivers/media/dvb-frontends/horus3a.c b/drivers/media/dvb-frontends/horus3a.c
index 68d759c4c52e..fa9e2d373073 100644
--- a/drivers/media/dvb-frontends/horus3a.c
+++ b/drivers/media/dvb-frontends/horus3a.c
@@ -89,7 +89,9 @@ static int horus3a_write_regs(struct horus3a_priv *priv,
static int horus3a_write_reg(struct horus3a_priv *priv, u8 reg, u8 val)
{
- return horus3a_write_regs(priv, reg, &val, 1);
+ u8 tmp = val;
+
+ return horus3a_write_regs(priv, reg, &tmp, 1);
}
static int horus3a_enter_power_save(struct horus3a_priv *priv)
diff --git a/drivers/media/dvb-frontends/itd1000.c b/drivers/media/dvb-frontends/itd1000.c
index 5bb1e73a10b4..1ac5177162f6 100644
--- a/drivers/media/dvb-frontends/itd1000.c
+++ b/drivers/media/dvb-frontends/itd1000.c
@@ -95,8 +95,9 @@ static int itd1000_read_reg(struct itd1000_state *state, u8 reg)
static inline int itd1000_write_reg(struct itd1000_state *state, u8 r, u8 v)
{
- int ret = itd1000_write_regs(state, r, &v, 1);
- state->shadow[r] = v;
+ u8 tmp = v;
+ int ret = itd1000_write_regs(state, r, &tmp, 1);
+ state->shadow[r] = tmp;
return ret;
}
diff --git a/drivers/media/dvb-frontends/mt312.c b/drivers/media/dvb-frontends/mt312.c
index 961b9a2508e0..12c32c024252 100644
--- a/drivers/media/dvb-frontends/mt312.c
+++ b/drivers/media/dvb-frontends/mt312.c
@@ -142,7 +142,9 @@ static inline int mt312_readreg(struct mt312_state *state,
static inline int mt312_writereg(struct mt312_state *state,
const enum mt312_reg_addr reg, const u8 val)
{
- return mt312_write(state, reg, &val, 1);
+ u8 tmp = val;
+
+ return mt312_write(state, reg, &tmp, 1);
}
static inline u32 mt312_div(u32 a, u32 b)
diff --git a/drivers/media/dvb-frontends/stb0899_drv.c b/drivers/media/dvb-frontends/stb0899_drv.c
index 02347598277a..db5dde3215f0 100644
--- a/drivers/media/dvb-frontends/stb0899_drv.c
+++ b/drivers/media/dvb-frontends/stb0899_drv.c
@@ -539,7 +539,8 @@ int stb0899_write_regs(struct stb0899_state *state, unsigned int reg, u8 *data,
int stb0899_write_reg(struct stb0899_state *state, unsigned int reg, u8 data)
{
- return stb0899_write_regs(state, reg, &data, 1);
+ u8 tmp = data;
+ return stb0899_write_regs(state, reg, &tmp, 1);
}
/*
diff --git a/drivers/media/dvb-frontends/stb6100.c b/drivers/media/dvb-frontends/stb6100.c
index 17a955d0031b..0167a3e1ecc5 100644
--- a/drivers/media/dvb-frontends/stb6100.c
+++ b/drivers/media/dvb-frontends/stb6100.c
@@ -226,12 +226,14 @@ static int stb6100_write_reg_range(struct stb6100_state *state, u8 buf[], int st
static int stb6100_write_reg(struct stb6100_state *state, u8 reg, u8 data)
{
+ u8 tmp = data;
+
if (unlikely(reg >= STB6100_NUMREGS)) {
dprintk(verbose, FE_ERROR, 1, "Invalid register offset 0x%x", reg);
return -EREMOTEIO;
}
- data = (data & stb6100_template[reg].mask) | stb6100_template[reg].set;
- return stb6100_write_reg_range(state, &data, reg, 1);
+ tmp = (tmp & stb6100_template[reg].mask) | stb6100_template[reg].set;
+ return stb6100_write_reg_range(state, &tmp, reg, 1);
}
diff --git a/drivers/media/dvb-frontends/stv0367.c b/drivers/media/dvb-frontends/stv0367.c
index f3529df8211d..1964cb7d58b7 100644
--- a/drivers/media/dvb-frontends/stv0367.c
+++ b/drivers/media/dvb-frontends/stv0367.c
@@ -166,7 +166,9 @@ int stv0367_writeregs(struct stv0367_state *state, u16 reg, u8 *data, int len)
static int stv0367_writereg(struct stv0367_state *state, u16 reg, u8 data)
{
- return stv0367_writeregs(state, reg, &data, 1);
+ u8 tmp = data;
+
+ return stv0367_writeregs(state, reg, &tmp, 1);
}
static u8 stv0367_readreg(struct stv0367_state *state, u16 reg)
diff --git a/drivers/media/dvb-frontends/stv090x.c b/drivers/media/dvb-frontends/stv090x.c
index 7ef469c0c866..85ec19305483 100644
--- a/drivers/media/dvb-frontends/stv090x.c
+++ b/drivers/media/dvb-frontends/stv090x.c
@@ -755,7 +755,9 @@ static int stv090x_write_regs(struct stv090x_state *state, unsigned int reg, u8
static int stv090x_write_reg(struct stv090x_state *state, unsigned int reg, u8 data)
{
- return stv090x_write_regs(state, reg, &data, 1);
+ u8 tmp = data;
+
+ return stv090x_write_regs(state, reg, &tmp, 1);
}
static int stv090x_i2c_gate_ctrl(struct stv090x_state *state, int enable)
diff --git a/drivers/media/dvb-frontends/stv6110x.c b/drivers/media/dvb-frontends/stv6110x.c
index 66eba38f1014..f0fd9605aa77 100644
--- a/drivers/media/dvb-frontends/stv6110x.c
+++ b/drivers/media/dvb-frontends/stv6110x.c
@@ -97,7 +97,9 @@ static int stv6110x_write_regs(struct stv6110x_state *stv6110x, int start, u8 da
static int stv6110x_write_reg(struct stv6110x_state *stv6110x, u8 reg, u8 data)
{
- return stv6110x_write_regs(stv6110x, reg, &data, 1);
+ u8 tmp = data;
+
+ return stv6110x_write_regs(stv6110x, reg, &tmp, 1);
}
static int stv6110x_init(struct dvb_frontend *fe)
diff --git a/drivers/media/dvb-frontends/zl10039.c b/drivers/media/dvb-frontends/zl10039.c
index 623355fc2666..4ec9a8fb8c12 100644
--- a/drivers/media/dvb-frontends/zl10039.c
+++ b/drivers/media/dvb-frontends/zl10039.c
@@ -134,7 +134,9 @@ static inline int zl10039_writereg(struct zl10039_state *state,
const enum zl10039_reg_addr reg,
const u8 val)
{
- return zl10039_write(state, reg, &val, 1);
+ const u8 tmp = val;
+
+ return zl10039_write(state, reg, &tmp, 1);
}
static int zl10039_init(struct dvb_frontend *fe)
--
2.9.0
^ 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