* [PATCH] rtlwifi: rtl8821ae: Fix typo in variable name
From: Nik Nyby @ 2017-10-24 1:49 UTC (permalink / raw)
To: Larry.Finger, kvalo, linux-wireless, netdev, trivial; +Cc: Nik Nyby
In _rtl8821ae_dbi_write(), wrtie_addr should be write_addr.
Signed-off-by: Nik Nyby <nikolas@gnu.org>
---
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
index 1d431d4bf6d2..6a9d8ee0ae9f 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
@@ -1130,13 +1130,13 @@ static u8 _rtl8821ae_dbi_read(struct rtl_priv *rtlpriv, u16 addr)
static void _rtl8821ae_dbi_write(struct rtl_priv *rtlpriv, u16 addr, u8 data)
{
u8 tmp = 0, count = 0;
- u16 wrtie_addr, remainder = addr % 4;
+ u16 write_addr, remainder = addr % 4;
- wrtie_addr = REG_DBI_WDATA + remainder;
- rtl_write_byte(rtlpriv, wrtie_addr, data);
+ write_addr = REG_DBI_WDATA + remainder;
+ rtl_write_byte(rtlpriv, write_addr, data);
- wrtie_addr = (addr & 0xfffc) | (BIT(0) << (remainder + 12));
- rtl_write_word(rtlpriv, REG_DBI_ADDR, wrtie_addr);
+ write_addr = (addr & 0xfffc) | (BIT(0) << (remainder + 12));
+ rtl_write_word(rtlpriv, REG_DBI_ADDR, write_addr);
rtl_write_byte(rtlpriv, REG_DBI_FLAG, 0x1);
--
2.14.2
^ permalink raw reply related
* [PATCH] rtlwifi: Remove seq_number from rtl_tid_data
From: pkshih @ 2017-10-24 2:03 UTC (permalink / raw)
To: Larry.Finger, kvalo
Cc: linux-wireless, netdev, linux-kernel, yhchuang, pkshih
From: Ping-Ke Shih <pkshih@realtek.com>
Since mac80211 maintains the sequence number for each STA/TID,
driver doesn't need to maintain a copy.
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
drivers/net/wireless/realtek/rtlwifi/base.c | 6 ++----
drivers/net/wireless/realtek/rtlwifi/pci.c | 17 -----------------
drivers/net/wireless/realtek/rtlwifi/usb.c | 17 -----------------
drivers/net/wireless/realtek/rtlwifi/wifi.h | 1 -
4 files changed, 2 insertions(+), 39 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/base.c b/drivers/net/wireless/realtek/rtlwifi/base.c
index 0b34886321f1..9f35b25bddf2 100644
--- a/drivers/net/wireless/realtek/rtlwifi/base.c
+++ b/drivers/net/wireless/realtek/rtlwifi/base.c
@@ -1618,9 +1618,8 @@ int rtl_tx_agg_start(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
RT_TRACE(rtlpriv, COMP_SEND, DBG_DMESG,
"on ra = %pM tid = %d seq:%d\n", sta->addr, tid,
- tid_data->seq_number);
+ *ssn);
- *ssn = tid_data->seq_number;
tid_data->agg.agg_state = RTL_AGG_START;
ieee80211_start_tx_ba_cb_irqsafe(vif, sta->addr, tid);
@@ -1679,8 +1678,7 @@ int rtl_rx_agg_start(struct ieee80211_hw *hw,
tid_data = &sta_entry->tids[tid];
RT_TRACE(rtlpriv, COMP_RECV, DBG_DMESG,
- "on ra = %pM tid = %d seq:%d\n", sta->addr, tid,
- tid_data->seq_number);
+ "on ra = %pM tid = %d\n", sta->addr, tid);
tid_data->agg.rx_agg_state = RTL_RX_AGG_START;
return 0;
diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index b9a6d23364be..eb12818b46b3 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -1623,7 +1623,6 @@ static int rtl_pci_tx(struct ieee80211_hw *hw,
struct rtl_tcb_desc *ptcb_desc)
{
struct rtl_priv *rtlpriv = rtl_priv(hw);
- struct rtl_sta_info *sta_entry = NULL;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct rtl8192_tx_ring *ring;
struct rtl_tx_desc *pdesc;
@@ -1635,9 +1634,6 @@ static int rtl_pci_tx(struct ieee80211_hw *hw,
__le16 fc = rtl_get_fc(skb);
u8 *pda_addr = hdr->addr1;
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
- /*ssn */
- u8 tid = 0;
- u16 seq_number = 0;
u8 own;
u8 temp_one = 1;
@@ -1699,19 +1695,6 @@ static int rtl_pci_tx(struct ieee80211_hw *hw,
return skb->len;
}
- if (ieee80211_is_data_qos(fc)) {
- tid = rtl_get_tid(skb);
- if (sta) {
- sta_entry = (struct rtl_sta_info *)sta->drv_priv;
- seq_number = (le16_to_cpu(hdr->seq_ctrl) &
- IEEE80211_SCTL_SEQ) >> 4;
- seq_number += 1;
-
- if (!ieee80211_has_morefrags(hdr->frame_control))
- sta_entry->tids[tid].seq_number = seq_number;
- }
- }
-
if (ieee80211_is_data(fc))
rtlpriv->cfg->ops->led_control(hw, LED_CTL_TX);
diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c
index 5590d07d0918..39b033b3b53a 100644
--- a/drivers/net/wireless/realtek/rtlwifi/usb.c
+++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
@@ -952,17 +952,12 @@ static void _rtl_usb_tx_preprocess(struct ieee80211_hw *hw,
u16 hw_queue)
{
struct rtl_priv *rtlpriv = rtl_priv(hw);
- struct rtl_mac *mac = rtl_mac(rtl_priv(hw));
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct rtl_tx_desc *pdesc = NULL;
struct rtl_tcb_desc tcb_desc;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)(skb->data);
__le16 fc = hdr->frame_control;
u8 *pda_addr = hdr->addr1;
- /* ssn */
- u8 *qc = NULL;
- u8 tid = 0;
- u16 seq_number = 0;
memset(&tcb_desc, 0, sizeof(struct rtl_tcb_desc));
if (ieee80211_is_auth(fc)) {
@@ -983,20 +978,8 @@ static void _rtl_usb_tx_preprocess(struct ieee80211_hw *hw,
rtlpriv->stats.txbytesbroadcast += skb->len;
else
rtlpriv->stats.txbytesunicast += skb->len;
- if (ieee80211_is_data_qos(fc)) {
- qc = ieee80211_get_qos_ctl(hdr);
- tid = qc[0] & IEEE80211_QOS_CTL_TID_MASK;
- seq_number = (le16_to_cpu(hdr->seq_ctrl) &
- IEEE80211_SCTL_SEQ) >> 4;
- seq_number += 1;
- seq_number <<= 4;
- }
rtlpriv->cfg->ops->fill_tx_desc(hw, hdr, (u8 *)pdesc, NULL, info, sta, skb,
hw_queue, &tcb_desc);
- if (!ieee80211_has_morefrags(hdr->frame_control)) {
- if (qc)
- mac->tids[tid].seq_number = seq_number;
- }
if (ieee80211_is_data(fc))
rtlpriv->cfg->ops->led_control(hw, LED_CTL_TX);
}
diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h b/drivers/net/wireless/realtek/rtlwifi/wifi.h
index 7dfc73b554f1..22afc14c3da6 100644
--- a/drivers/net/wireless/realtek/rtlwifi/wifi.h
+++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h
@@ -1324,7 +1324,6 @@ struct rssi_sta {
};
struct rtl_tid_data {
- u16 seq_number;
struct rtl_ht_agg agg;
};
--
2.14.1
^ permalink raw reply related
* Re: [PATCH 0/2] 9p: Fixes for hard-to-hit bugs
From: Al Viro @ 2017-10-24 2:11 UTC (permalink / raw)
To: Tuomas Tynkkynen
Cc: v9fs-developer, Eric Van Hensbergen, Ron Minnich,
Latchesar Ionkov, David S. Miller, linux-kernel, netdev,
linux-fsdevel
In-Reply-To: <1508809268.9653.13.camel@tuxera.com>
On Tue, Oct 24, 2017 at 04:41:08AM +0300, Tuomas Tynkkynen wrote:
> > I can pick those, or, if you (or somebody else) are willing to
> > actively
> > maintain a 9p tree, you could start sending straight to Linus - up to
> > you.
>
> You can pick these up, I don't have plans for more patches right now
> (unless we hit some new bugs that affect our workloads).
Done.
^ permalink raw reply
* Re: [PATCH] net: xfrm_user: use BUG_ON instead of if condition followed by BUG
From: Herbert Xu @ 2017-10-24 3:25 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Steffen Klassert, David S. Miller, netdev, linux-kernel
In-Reply-To: <20171023181857.GA25379@embeddedor.com>
On Mon, Oct 23, 2017 at 01:18:57PM -0500, Gustavo A. R. Silva wrote:
> Use BUG_ON instead of if condition followed by BUG.
>
> This issue was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
I think this patch is terrible. Why on earth is Coccinelle even
warning about this?
If anything we should be converting these constructs to not use
BUG.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH V3 net-next] liquidio: pass date and time info to NIC firmware
From: Felix Manlunas @ 2017-10-24 3:33 UTC (permalink / raw)
To: davem
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
veerasenareddy.burru
From: Veerasenareddy Burru <veerasenareddy.burru@cavium.com>
Pass date and time information to NIC at the time of loading
firmware and periodically update the host time to NIC firmware.
This is to make NIC firmware use the same time reference as Host,
so that it is easy to correlate logs from firmware and host for debugging.
Signed-off-by: Veerasenareddy Burru <veerasenareddy.burru@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
Patch Change Log:
V2 -> V3:
Fix inconsistency of accounting for local timezone at first but then
not accounting for it in subsequent date/time updates. For V3, all
date/time updates are in Unix time. Octeon always uses UTC time.
V1 -> V2:
Follow reviewer's suggestion of periodically updating firmware with
date and time info.
drivers/net/ethernet/cavium/liquidio/lio_main.c | 126 +++++++++++++++++++++
.../net/ethernet/cavium/liquidio/liquidio_common.h | 8 ++
.../net/ethernet/cavium/liquidio/octeon_console.c | 32 +++++-
.../net/ethernet/cavium/liquidio/octeon_network.h | 3 +
4 files changed, 166 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index 963803b..8ab5ed5 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -83,6 +83,11 @@ static int octeon_console_debug_enabled(u32 console)
/* runtime link query interval */
#define LIQUIDIO_LINK_QUERY_INTERVAL_MS 1000
+/* update localtime to octeon firmware every 60 seconds.
+ * make firmware to use same time reference, so that it will be easy to
+ * correlate firmware logged events/errors with host events, for debugging.
+ */
+#define LIO_SYNC_OCTEON_TIME_INTERVAL_MS 60000
struct liquidio_if_cfg_context {
int octeon_id;
@@ -901,6 +906,121 @@ static inline void update_link_status(struct net_device *netdev,
}
}
+/**
+ * lio_sync_octeon_time_cb - callback that is invoked when soft command
+ * sent by lio_sync_octeon_time() has completed successfully or failed
+ *
+ * @oct - octeon device structure
+ * @status - indicates success or failure
+ * @buf - pointer to the command that was sent to firmware
+ **/
+static void lio_sync_octeon_time_cb(struct octeon_device *oct,
+ u32 status, void *buf)
+{
+ struct octeon_soft_command *sc = (struct octeon_soft_command *)buf;
+
+ if (status)
+ dev_err(&oct->pci_dev->dev,
+ "Failed to sync time to octeon; error=%d\n", status);
+
+ octeon_free_soft_command(oct, sc);
+}
+
+/**
+ * lio_sync_octeon_time - send latest localtime to octeon firmware so that
+ * firmware will correct it's time, in case there is a time skew
+ *
+ * @work: work scheduled to send time update to octeon firmware
+ **/
+static void lio_sync_octeon_time(struct work_struct *work)
+{
+ struct cavium_wk *wk = (struct cavium_wk *)work;
+ struct lio *lio = (struct lio *)wk->ctxptr;
+ struct octeon_device *oct = lio->oct_dev;
+ struct octeon_soft_command *sc;
+ struct timespec64 ts;
+ struct lio_time *lt;
+ int ret;
+
+ sc = octeon_alloc_soft_command(oct, sizeof(struct lio_time), 0, 0);
+ if (!sc) {
+ dev_err(&oct->pci_dev->dev,
+ "Failed to sync time to octeon: soft command allocation failed\n");
+ return;
+ }
+
+ lt = (struct lio_time *)sc->virtdptr;
+
+ /* Get time of the day */
+ getnstimeofday64(&ts);
+ lt->sec = ts.tv_sec;
+ lt->nsec = ts.tv_nsec;
+ octeon_swap_8B_data((u64 *)lt, (sizeof(struct lio_time)) / 8);
+
+ sc->iq_no = lio->linfo.txpciq[0].s.q_no;
+ octeon_prepare_soft_command(oct, sc, OPCODE_NIC,
+ OPCODE_NIC_SYNC_OCTEON_TIME, 0, 0, 0);
+
+ sc->callback = lio_sync_octeon_time_cb;
+ sc->callback_arg = sc;
+ sc->wait_time = 1000;
+
+ ret = octeon_send_soft_command(oct, sc);
+ if (ret == IQ_SEND_FAILED) {
+ dev_err(&oct->pci_dev->dev,
+ "Failed to sync time to octeon: failed to send soft command\n");
+ octeon_free_soft_command(oct, sc);
+ }
+
+ queue_delayed_work(lio->sync_octeon_time_wq.wq,
+ &lio->sync_octeon_time_wq.wk.work,
+ msecs_to_jiffies(LIO_SYNC_OCTEON_TIME_INTERVAL_MS));
+}
+
+/**
+ * setup_sync_octeon_time_wq - Sets up the work to periodically update
+ * local time to octeon firmware
+ *
+ * @netdev - network device which should send time update to firmware
+ **/
+static inline int setup_sync_octeon_time_wq(struct net_device *netdev)
+{
+ struct lio *lio = GET_LIO(netdev);
+ struct octeon_device *oct = lio->oct_dev;
+
+ lio->sync_octeon_time_wq.wq =
+ alloc_workqueue("update-octeon-time", WQ_MEM_RECLAIM, 0);
+ if (!lio->sync_octeon_time_wq.wq) {
+ dev_err(&oct->pci_dev->dev, "Unable to create wq to update octeon time\n");
+ return -1;
+ }
+ INIT_DELAYED_WORK(&lio->sync_octeon_time_wq.wk.work,
+ lio_sync_octeon_time);
+ lio->sync_octeon_time_wq.wk.ctxptr = lio;
+ queue_delayed_work(lio->sync_octeon_time_wq.wq,
+ &lio->sync_octeon_time_wq.wk.work,
+ msecs_to_jiffies(LIO_SYNC_OCTEON_TIME_INTERVAL_MS));
+
+ return 0;
+}
+
+/**
+ * cleanup_sync_octeon_time_wq - stop scheduling and destroy the work created
+ * to periodically update local time to octeon firmware
+ *
+ * @netdev - network device which should send time update to firmware
+ **/
+static inline void cleanup_sync_octeon_time_wq(struct net_device *netdev)
+{
+ struct lio *lio = GET_LIO(netdev);
+ struct cavium_wq *time_wq = &lio->sync_octeon_time_wq;
+
+ if (time_wq->wq) {
+ cancel_delayed_work_sync(&time_wq->wk.work);
+ destroy_workqueue(time_wq->wq);
+ }
+}
+
static struct octeon_device *get_other_octeon_device(struct octeon_device *oct)
{
struct octeon_device *other_oct;
@@ -1455,6 +1575,7 @@ static void liquidio_destroy_nic_device(struct octeon_device *oct, int ifidx)
if (atomic_read(&lio->ifstate) & LIO_IFSTATE_REGISTERED)
unregister_netdev(netdev);
+ cleanup_sync_octeon_time_wq(netdev);
cleanup_link_status_change_wq(netdev);
cleanup_rx_oom_poll_fn(netdev);
@@ -3611,6 +3732,11 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
if (setup_link_status_change_wq(netdev))
goto setup_nic_dev_fail;
+ if ((octeon_dev->fw_info.app_cap_flags &
+ LIQUIDIO_TIME_SYNC_CAP) &&
+ setup_sync_octeon_time_wq(netdev))
+ goto setup_nic_dev_fail;
+
if (setup_rx_oom_poll_fn(netdev))
goto setup_nic_dev_fail;
diff --git a/drivers/net/ethernet/cavium/liquidio/liquidio_common.h b/drivers/net/ethernet/cavium/liquidio/liquidio_common.h
index 3788c8c..2033a65 100644
--- a/drivers/net/ethernet/cavium/liquidio/liquidio_common.h
+++ b/drivers/net/ethernet/cavium/liquidio/liquidio_common.h
@@ -84,6 +84,7 @@ enum octeon_tag_type {
#define OPCODE_NIC_IF_CFG 0x09
#define OPCODE_NIC_VF_DRV_NOTICE 0x0A
#define OPCODE_NIC_INTRMOD_PARAMS 0x0B
+#define OPCODE_NIC_SYNC_OCTEON_TIME 0x14
#define VF_DRV_LOADED 1
#define VF_DRV_REMOVED -1
#define VF_DRV_MACADDR_CHANGED 2
@@ -108,6 +109,9 @@ enum octeon_tag_type {
#define SCR2_BIT_FW_LOADED 63
+/* App specific capabilities from firmware to pf driver */
+#define LIQUIDIO_TIME_SYNC_CAP 0x1
+
static inline u32 incr_index(u32 index, u32 count, u32 max)
{
if ((index + count) >= max)
@@ -901,4 +905,8 @@ struct oct_intrmod_cfg {
} s;
};
+struct lio_time {
+ s64 sec; /* seconds */
+ s64 nsec; /* nanoseconds */
+};
#endif
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_console.c b/drivers/net/ethernet/cavium/liquidio/octeon_console.c
index ec3dd69..7f97ae4 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_console.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_console.c
@@ -803,15 +803,18 @@ static int octeon_console_read(struct octeon_device *oct, u32 console_num,
}
#define FBUF_SIZE (4 * 1024 * 1024)
+#define MAX_BOOTTIME_SIZE 80
int octeon_download_firmware(struct octeon_device *oct, const u8 *data,
size_t size)
{
- int ret = 0;
+ struct octeon_firmware_file_header *h;
+ char boottime[MAX_BOOTTIME_SIZE];
+ struct timespec64 ts;
u32 crc32_result;
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 +893,34 @@ int octeon_download_firmware(struct octeon_device *oct, const u8 *data,
load_addr += size;
}
}
+
+ /* Pass date and time information to NIC at the time of loading
+ * firmware and periodically update the host time to NIC firmware.
+ * This is to make NIC firmware use the same time reference as Host,
+ * so that it is easy to correlate logs from firmware and host for
+ * debugging.
+ *
+ * Octeon always uses UTC time. so timezone information is not sent.
+ */
+ getnstimeofday64(&ts);
+ ret = snprintf(boottime, MAX_BOOTTIME_SIZE,
+ " time_sec=%lld time_nsec=%ld",
+ (s64)ts.tv_sec, ts.tv_nsec);
+ 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, boottime,
+ 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;
}
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_network.h b/drivers/net/ethernet/cavium/liquidio/octeon_network.h
index 9e36319..433f361 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_network.h
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_network.h
@@ -136,6 +136,9 @@ struct lio {
/* work queue for link status */
struct cavium_wq link_status_wq;
+ /* work queue to regularly send local time to octeon firmware */
+ struct cavium_wq sync_octeon_time_wq;
+
int netdev_uc_count;
};
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] net: xfrm_user: use BUG_ON instead of if condition followed by BUG
From: Gustavo A. R. Silva @ 2017-10-24 3:50 UTC (permalink / raw)
To: Herbert Xu; +Cc: Steffen Klassert, David S. Miller, netdev, linux-kernel
In-Reply-To: <20171024032508.GC28462@gondor.apana.org.au>
Quoting Herbert Xu <herbert@gondor.apana.org.au>:
> On Mon, Oct 23, 2017 at 01:18:57PM -0500, Gustavo A. R. Silva wrote:
>> Use BUG_ON instead of if condition followed by BUG.
>>
>> This issue was detected with the help of Coccinelle.
>>
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>
> I think this patch is terrible. Why on earth is Coccinelle even
> warning about this?
>
> If anything we should be converting these constructs to not use
> BUG.
>
Yeah, and under this assumption the original code is even worse.
Thanks
--
Gustavo A. R. Silva
^ permalink raw reply
* Re: [PATCH] net: xfrm_user: use BUG_ON instead of if condition followed by BUG
From: Herbert Xu @ 2017-10-24 3:53 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Steffen Klassert, David S. Miller, netdev, linux-kernel
In-Reply-To: <20171023225043.Horde.R0FvJkHEmPPjnRNzKMOrPb3@gator4166.hostgator.com>
On Mon, Oct 23, 2017 at 10:50:43PM -0500, Gustavo A. R. Silva wrote:
>
> Quoting Herbert Xu <herbert@gondor.apana.org.au>:
>
> >On Mon, Oct 23, 2017 at 01:18:57PM -0500, Gustavo A. R. Silva wrote:
> >>Use BUG_ON instead of if condition followed by BUG.
> >>
> >>This issue was detected with the help of Coccinelle.
> >>
> >>Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> >
> >I think this patch is terrible. Why on earth is Coccinelle even
> >warning about this?
> >
> >If anything we should be converting these constructs to not use
> >BUG.
> >
>
> Yeah, and under this assumption the original code is even worse.
No your patch makes it worse. The original construct makes it
clear that the conditional is real code and not just some debugging
aid.
With your patch, real code is now inside BUG_ON.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] net: xfrm_user: use BUG_ON instead of if condition followed by BUG
From: Gustavo A. R. Silva @ 2017-10-24 3:57 UTC (permalink / raw)
To: Herbert Xu; +Cc: Steffen Klassert, David S. Miller, netdev, linux-kernel
In-Reply-To: <20171024035320.GA29795@gondor.apana.org.au>
Quoting Herbert Xu <herbert@gondor.apana.org.au>:
> On Mon, Oct 23, 2017 at 10:50:43PM -0500, Gustavo A. R. Silva wrote:
>>
>> Quoting Herbert Xu <herbert@gondor.apana.org.au>:
>>
>> >On Mon, Oct 23, 2017 at 01:18:57PM -0500, Gustavo A. R. Silva wrote:
>> >>Use BUG_ON instead of if condition followed by BUG.
>> >>
>> >>This issue was detected with the help of Coccinelle.
>> >>
>> >>Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> >
>> >I think this patch is terrible. Why on earth is Coccinelle even
>> >warning about this?
>> >
>> >If anything we should be converting these constructs to not use
>> >BUG.
>> >
>>
>> Yeah, and under this assumption the original code is even worse.
>
> No your patch makes it worse. The original construct makes it
> clear that the conditional is real code and not just some debugging
> aid.
>
> With your patch, real code is now inside BUG_ON.
>
What is the reason for BUG_ON to exist then if it makes the code worse
than an _if_ condition followed by BUG?
Thanks
--
Gustavo A. R. Silva
^ permalink raw reply
* Re: [PATCH] net: xfrm_user: use BUG_ON instead of if condition followed by BUG
From: Alexei Starovoitov @ 2017-10-24 4:01 UTC (permalink / raw)
To: Herbert Xu
Cc: Gustavo A. R. Silva, Steffen Klassert, David S. Miller, netdev,
linux-kernel
In-Reply-To: <20171024035320.GA29795@gondor.apana.org.au>
On Tue, Oct 24, 2017 at 11:53:20AM +0800, Herbert Xu wrote:
> On Mon, Oct 23, 2017 at 10:50:43PM -0500, Gustavo A. R. Silva wrote:
> >
> > Quoting Herbert Xu <herbert@gondor.apana.org.au>:
> >
> > >On Mon, Oct 23, 2017 at 01:18:57PM -0500, Gustavo A. R. Silva wrote:
> > >>Use BUG_ON instead of if condition followed by BUG.
> > >>
> > >>This issue was detected with the help of Coccinelle.
> > >>
> > >>Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> > >
> > >I think this patch is terrible. Why on earth is Coccinelle even
> > >warning about this?
> > >
> > >If anything we should be converting these constructs to not use
> > >BUG.
> > >
> >
> > Yeah, and under this assumption the original code is even worse.
>
> No your patch makes it worse. The original construct makes it
> clear that the conditional is real code and not just some debugging
> aid.
>
> With your patch, real code is now inside BUG_ON.
fwiw I had the same argument earlier:
https://lkml.org/lkml/2017/10/9/1139
^ permalink raw reply
* Re: [PATCH net-next 2/3] bpf: permit multiple bpf attachments for a single perf event
From: kbuild test robot @ 2017-10-24 4:05 UTC (permalink / raw)
To: Yonghong Song
Cc: kbuild-all, peterz, rostedt, ast, daniel, kafai, netdev,
kernel-team
In-Reply-To: <20171023175805.2898366-3-yhs@fb.com>
[-- Attachment #1: Type: text/plain, Size: 865 bytes --]
Hi Yonghong,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Yonghong-Song/bpf-permit-multiple-bpf-attachments-for-a-single-perf-tracepoint-event/20171024-080608
config: i386-randconfig-h1-10241031 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
kernel/events/core.o: In function `_free_event':
core.c:(.text+0x5f1e): undefined reference to `perf_event_detach_bpf_prog'
kernel/events/core.o: In function `perf_ioctl':
>> core.c:(.text+0x72b3): undefined reference to `perf_event_attach_bpf_prog'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23575 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 2/3] bpf: permit multiple bpf attachments for a single perf event
From: kbuild test robot @ 2017-10-24 4:07 UTC (permalink / raw)
To: Yonghong Song
Cc: kbuild-all, peterz, rostedt, ast, daniel, kafai, netdev,
kernel-team
In-Reply-To: <20171023175805.2898366-3-yhs@fb.com>
[-- Attachment #1: Type: text/plain, Size: 1302 bytes --]
Hi Yonghong,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Yonghong-Song/bpf-permit-multiple-bpf-attachments-for-a-single-perf-tracepoint-event/20171024-080608
config: arm-omap2plus_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
kernel/events/core.o: In function `_free_event':
>> kernel/events/core.c:8193: undefined reference to `perf_event_detach_bpf_prog'
>> kernel/events/core.o:(.debug_addr+0x25c): undefined reference to `perf_event_detach_bpf_prog'
vim +8193 kernel/events/core.c
8186
8187 static void perf_event_free_bpf_prog(struct perf_event *event)
8188 {
8189 if (event->attr.type != PERF_TYPE_TRACEPOINT) {
8190 perf_event_free_bpf_handler(event);
8191 return;
8192 }
> 8193 perf_event_detach_bpf_prog(event);
8194 }
8195
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30261 bytes --]
^ permalink raw reply
* Re: [PATCH] net: xfrm_user: use BUG_ON instead of if condition followed by BUG
From: Herbert Xu @ 2017-10-24 4:30 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Gustavo A. R. Silva, Steffen Klassert, David S. Miller, netdev,
linux-kernel
In-Reply-To: <20171024040144.bf2cvq7p6zwg2e5k@ast-mbp>
On Mon, Oct 23, 2017 at 09:01:46PM -0700, Alexei Starovoitov wrote:
>
> fwiw I had the same argument earlier:
> https://lkml.org/lkml/2017/10/9/1139
Fair point on eliminating a branch. But I'd prefer something like
bool cond;
cond = code_that_does_something();
BUG_ON(cond);
rather than just
BUG_ON(code_that_does_something());
But maybe it's just me.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [net-next PATCH] bpf: cpumap fix potential lost wake-up problem
From: Jesper Dangaard Brouer @ 2017-10-24 4:46 UTC (permalink / raw)
To: Daniel Borkmann
Cc: netdev, Michael S. Tsirkin, Rik van Riel, Daniel Borkmann,
Alexei Starovoitov, brouer
In-Reply-To: <59EE525D.6090903@iogearbox.net>
On Mon, 23 Oct 2017 22:34:37 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 10/23/2017 07:39 PM, Jesper Dangaard Brouer wrote:
> > As pointed out by Michael, commit 1c601d829ab0 ("bpf: cpumap xdp_buff
> > to skb conversion and allocation") contains a classical example of the
> > potential lost wake-up problem.
> >
> > We need to recheck the condition __ptr_ring_empty() after changing
> > current->state to TASK_INTERRUPTIBLE, this avoids a race between
> > wake_up_process() and schedule(). After this, a race with
> > wake_up_process() will simply change the state to TASK_RUNNING, and
> > the schedule() call not really put us to sleep.
> >
> > Fixes: 1c601d829ab0 ("bpf: cpumap xdp_buff to skb conversion and allocation")
> > Reported-by: "Michael S. Tsirkin" <mst@redhat.com>
>
> SOB missing ...
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Damn... DaveM do I need to resubmit? Or will patchwork pickup above SOB?
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* RE: [Patch net 14/15] selftests: Introduce a new script to generate tc batch file
From: Chris Mi @ 2017-10-24 4:56 UTC (permalink / raw)
To: Cong Wang, netdev@vger.kernel.org
Cc: paulmck@linux.vnet.ibm.com, jhs@mojatatu.com,
john.fastabend@gmail.com
In-Reply-To: <20171023220304.2268-15-xiyou.wangcong@gmail.com>
> -----Original Message-----
> From: Cong Wang [mailto:xiyou.wangcong@gmail.com]
> Sent: Tuesday, October 24, 2017 6:03 AM
> To: netdev@vger.kernel.org
> Cc: paulmck@linux.vnet.ibm.com; jhs@mojatatu.com;
> john.fastabend@gmail.com; Chris Mi <chrism@mellanox.com>; Cong Wang
> <xiyou.wangcong@gmail.com>
> Subject: [Patch net 14/15] selftests: Introduce a new script to generate tc
> batch file
>
> From: Chris Mi <chrism@mellanox.com>
>
> From: Chris Mi <chrism@mellanox.com>
>
> # ./tdc_batch.py -h
> usage: tdc_batch.py [-h] [-n NUMBER] [-o] [-s] [-p] device file
>
> TC batch file generator
>
> positional arguments:
> device device name
> file batch file name
>
> optional arguments:
> -h, --help show this help message and exit
> -n NUMBER, --number NUMBER
> how many lines in batch file
> -o, --skip_sw skip_sw (offload), by default skip_hw
> -s, --share_action all filters share the same action
> -p, --prio all filters have different prio
>
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Acked-by: Lucas Bates <lucasb@mojatatu.com>
> Signed-off-by: Chris Mi <chrism@mellanox.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> tools/testing/selftests/tc-testing/tdc_batch.py | 62
> +++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 tools/testing/selftests/tc-testing/tdc_batch.py
>
> diff --git a/tools/testing/selftests/tc-testing/tdc_batch.py
> b/tools/testing/selftests/tc-testing/tdc_batch.py
> new file mode 100644
File mode should be 755.
> index 000000000000..707c6bfef689
> --- /dev/null
> +++ b/tools/testing/selftests/tc-testing/tdc_batch.py
> @@ -0,0 +1,62 @@
> +#!/usr/bin/python3
> +
> +"""
> +tdc_batch.py - a script to generate TC batch file
> +
> +Copyright (C) 2017 Chris Mi <chrism@mellanox.com> """
> +
> +import argparse
> +
> +parser = argparse.ArgumentParser(description='TC batch file generator')
> +parser.add_argument("device", help="device name")
> +parser.add_argument("file", help="batch file name")
> +parser.add_argument("-n", "--number", type=int,
> + help="how many lines in batch file")
> +parser.add_argument("-o", "--skip_sw",
> + help="skip_sw (offload), by default skip_hw",
> + action="store_true") parser.add_argument("-s",
> +"--share_action",
> + help="all filters share the same action",
> + action="store_true") parser.add_argument("-p",
> +"--prio",
> + help="all filters have different prio",
> + action="store_true") args = parser.parse_args()
> +
> +device = args.device
> +file = open(args.file, 'w')
> +
> +number = 1
> +if args.number:
> + number = args.number
> +
> +skip = "skip_hw"
> +if args.skip_sw:
> + skip = "skip_sw"
> +
> +share_action = ""
> +if args.share_action:
> + share_action = "index 1"
> +
> +prio = "prio 1"
> +if args.prio:
> + prio = ""
> + if number > 0x4000:
> + number = 0x4000
> +
> +index = 0
> +for i in range(0x100):
> + for j in range(0x100):
> + for k in range(0x100):
> + mac = ("%02x:%02x:%02x" % (i, j, k))
> + src_mac = "e4:11:00:" + mac
> + dst_mac = "e4:12:00:" + mac
> + cmd = ("filter add dev %s %s protocol ip parent ffff: flower %s "
> + "src_mac %s dst_mac %s action drop %s" %
> + (device, prio, skip, src_mac, dst_mac, share_action))
> + file.write("%s\n" % cmd)
> + index += 1
> + if index >= number:
> + file.close()
> + exit(0)
> --
> 2.13.0
^ permalink raw reply
* [PATCHv3 net-next 0/4] net: remove rtmsg_ifinfo used in bridge and bonding
From: Xin Long @ 2017-10-24 5:54 UTC (permalink / raw)
To: network dev; +Cc: davem, David Ahern, Jiri Pirko
It's better to send notifications to userspace by the events in
rtnetlink_event, instead of calling rtmsg_ifinfo directly.
This patcheset is to remove rtmsg_ifinfo called in bonding and
bridge, the notifications can be handled by NETDEV_CHANGEUPPER
and NETDEV_CHANGELOWERSTATE events in rtnetlink_event.
It could also fix some redundant notifications from bonding and
bridge.
v1->v2:
- post to net-next.git instead of net.git, for it's more like an
improvement for bonding
v2->v3:
- add patch 1/4 to remove rtmsg_ifinfo called in add_del_if
Xin Long (4):
bridge: remove rtmsg_ifinfo called in add_del_if
bonding: remove rtmsg_ifinfo called in bond_master_upper_dev_link
rtnetlink: bring NETDEV_CHANGELOWERSTATE event process back to
rtnetlink_event
bonding: remove rtmsg_ifinfo called after bond_lower_state_changed
drivers/net/bonding/bond_main.c | 11 +++--------
include/net/bonding.h | 4 ----
net/bridge/br_ioctl.c | 3 ---
net/core/rtnetlink.c | 2 +-
4 files changed, 4 insertions(+), 16 deletions(-)
--
2.1.0
^ permalink raw reply
* [PATCHv3 net-next 1/4] bridge: remove rtmsg_ifinfo called in add_del_if
From: Xin Long @ 2017-10-24 5:54 UTC (permalink / raw)
To: network dev; +Cc: davem, David Ahern, Jiri Pirko
In-Reply-To: <cover.1508824093.git.lucien.xin@gmail.com>
Since commit dc709f375743 ("rtnetlink: bring NETDEV_CHANGEUPPER event
process back in rtnetlink_event"), rtnetlink_event would process the
NETDEV_CHANGEUPPER event and send a notification to userspace.
In add_del_if, it would generate NETDEV_CHANGEUPPER event by whether
netdev_master_upper_dev_link or netdev_upper_dev_unlink. There's
no need to call rtmsg_ifinfo to send the notification any more.
So this patch is to remove it from add_del_if also to avoid redundant
notifications.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/bridge/br_ioctl.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
index 8f29103..16d01a3 100644
--- a/net/bridge/br_ioctl.c
+++ b/net/bridge/br_ioctl.c
@@ -102,9 +102,6 @@ static int add_del_if(struct net_bridge *br, int ifindex, int isadd)
else
ret = br_del_if(br, dev);
- if (!ret)
- rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_MASTER, GFP_KERNEL);
-
return ret;
}
--
2.1.0
^ permalink raw reply related
* [PATCHv3 net-next 2/4] bonding: remove rtmsg_ifinfo called in bond_master_upper_dev_link
From: Xin Long @ 2017-10-24 5:54 UTC (permalink / raw)
To: network dev; +Cc: davem, David Ahern, Jiri Pirko
In-Reply-To: <cover.1508824093.git.lucien.xin@gmail.com>
Since commit 42e52bf9e3ae ("net: add netnotifier event for upper device
change"), netdev_master_upper_dev_link has generated NETDEV_CHANGEUPPER
event which would send a notification to userspace in rtnetlink_event.
There's no need to call rtmsg_ifinfo to send the notification any more.
So this patch is to remove it from bond_master_upper_dev_link as well
as bond_upper_dev_unlink to avoid the redundant notifications.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
drivers/net/bonding/bond_main.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 172eeeb..18b58e1 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1221,22 +1221,17 @@ static int bond_master_upper_dev_link(struct bonding *bond, struct slave *slave,
struct netlink_ext_ack *extack)
{
struct netdev_lag_upper_info lag_upper_info;
- int err;
lag_upper_info.tx_type = bond_lag_tx_type(bond);
- err = netdev_master_upper_dev_link(slave->dev, bond->dev, slave,
- &lag_upper_info, extack);
- if (err)
- return err;
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, IFF_SLAVE, GFP_KERNEL);
- return 0;
+
+ return netdev_master_upper_dev_link(slave->dev, bond->dev, slave,
+ &lag_upper_info, extack);
}
static void bond_upper_dev_unlink(struct bonding *bond, struct slave *slave)
{
netdev_upper_dev_unlink(slave->dev, bond->dev);
slave->dev->flags &= ~IFF_SLAVE;
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, IFF_SLAVE, GFP_KERNEL);
}
static struct slave *bond_alloc_slave(struct bonding *bond)
--
2.1.0
^ permalink raw reply related
* [PATCHv3 net-next 3/4] rtnetlink: bring NETDEV_CHANGELOWERSTATE event process back to rtnetlink_event
From: Xin Long @ 2017-10-24 5:54 UTC (permalink / raw)
To: network dev; +Cc: davem, David Ahern, Jiri Pirko
In-Reply-To: <cover.1508824093.git.lucien.xin@gmail.com>
This patch is to bring NETDEV_CHANGELOWERSTATE event process back
to rtnetlink_event so that bonding could use it instead of calling
rtmsg_ifinfo to send a notification to userspace after netdev lower
state is changed in the later patch.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/core/rtnetlink.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index df8dba9..854a848 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4385,6 +4385,7 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
case NETDEV_CHANGEUPPER:
case NETDEV_RESEND_IGMP:
case NETDEV_CHANGEINFODATA:
+ case NETDEV_CHANGELOWERSTATE:
case NETDEV_CHANGE_TX_QUEUE_LEN:
rtmsg_ifinfo_event(RTM_NEWLINK, dev, 0, rtnl_get_event(event),
GFP_KERNEL, NULL);
--
2.1.0
^ permalink raw reply related
* [PATCHv3 net-next 4/4] bonding: remove rtmsg_ifinfo called after bond_lower_state_changed
From: Xin Long @ 2017-10-24 5:54 UTC (permalink / raw)
To: network dev; +Cc: davem, David Ahern, Jiri Pirko
In-Reply-To: <cover.1508824093.git.lucien.xin@gmail.com>
After the patch 'rtnetlink: bring NETDEV_CHANGELOWERSTATE event
process back to rtnetlink_event', bond_lower_state_changed would
generate NETDEV_CHANGEUPPER event which would send a notification
to userspace in rtnetlink_event.
There's no need to call rtmsg_ifinfo to send the notification
any more. So this patch is to remove it from these places after
bond_lower_state_changed.
Besides, after this, rtmsg_ifinfo is not needed to be exported.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/bonding.h | 4 ----
net/core/rtnetlink.c | 1 -
2 files changed, 5 deletions(-)
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 2860cc6..f801fc9 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -330,7 +330,6 @@ static inline void bond_set_active_slave(struct slave *slave)
slave->backup = 0;
bond_queue_slave_event(slave);
bond_lower_state_changed(slave);
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
}
}
@@ -340,7 +339,6 @@ static inline void bond_set_backup_slave(struct slave *slave)
slave->backup = 1;
bond_queue_slave_event(slave);
bond_lower_state_changed(slave);
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
}
}
@@ -353,7 +351,6 @@ static inline void bond_set_slave_state(struct slave *slave,
slave->backup = slave_state;
if (notify) {
bond_lower_state_changed(slave);
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
bond_queue_slave_event(slave);
slave->should_notify = 0;
} else {
@@ -385,7 +382,6 @@ static inline void bond_slave_state_notify(struct bonding *bond)
bond_for_each_slave(bond, tmp, iter) {
if (tmp->should_notify) {
bond_lower_state_changed(tmp);
- rtmsg_ifinfo(RTM_NEWLINK, tmp->dev, 0, GFP_ATOMIC);
tmp->should_notify = 0;
}
}
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 854a848..de24d39 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2989,7 +2989,6 @@ void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
{
rtmsg_ifinfo_event(type, dev, change, rtnl_get_event(0), flags, NULL);
}
-EXPORT_SYMBOL(rtmsg_ifinfo);
void rtmsg_ifinfo_newnet(int type, struct net_device *dev, unsigned int change,
gfp_t flags, int *new_nsid)
--
2.1.0
^ permalink raw reply related
* [PATCH net-next] net/sched: Fix actions list corruption when adding offloaded tc flows
From: Or Gerlitz @ 2017-10-24 5:58 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, mlxsw, Roi Dayan, Paul Blakey, Or Gerlitz
Prior to commit b3f55bdda8df, the networking core doesn't wire an in-place
actions list the when the low level driver is called to offload the flow,
but all low level drivers do that (call tcf_exts_to_list()) in their
offloading "add" logic.
Now, the in-place list is set in the core which goes over the list in a loop,
but also by the hw driver when their offloading code is invoked indirectly:
cls_xxx add flow -> tc_setup_cb_call -> tc_exts_setup_cb_egdev_call -> hw driver
which messes up the core list instance upon driver return. Fix that by avoiding
in-place list on the net core code that deals with adding flows.
Fixes: b3f55bdda8df ('net: sched: introduce per-egress action device callbacks')
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_api.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index cdfdc24..0e96cda 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1184,14 +1184,13 @@ static int tc_exts_setup_cb_egdev_call(struct tcf_exts *exts,
#ifdef CONFIG_NET_CLS_ACT
const struct tc_action *a;
struct net_device *dev;
- LIST_HEAD(actions);
- int ret;
+ int i, ret;
if (!tcf_exts_has_actions(exts))
return 0;
- tcf_exts_to_list(exts, &actions);
- list_for_each_entry(a, &actions, list) {
+ for (i = 0; i < exts->nr_actions; i++) {
+ a = exts->actions[i];
if (!a->ops->get_dev)
continue;
dev = a->ops->get_dev(a);
--
2.3.7
^ permalink raw reply related
* RE: [patch net v2 1/4] net/sched: Change tc_action refcnt and bindcnt to atomic
From: Chris Mi @ 2017-10-24 6:17 UTC (permalink / raw)
To: Cong Wang
Cc: Jamal Hadi Salim, Linux Kernel Network Developers, Lucas Bates,
Jiri Pirko, David Miller
In-Reply-To: <CAM_iQpVqhwwyb+9-7Y6po0fAaNhzdjLpSAT1FS7dLm8TuhVcZQ@mail.gmail.com>
> -----Original Message-----
> From: Cong Wang [mailto:xiyou.wangcong@gmail.com]
> Sent: Monday, October 23, 2017 11:40 PM
> To: Chris Mi <chrism@mellanox.com>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>; Linux Kernel Network Developers
> <netdev@vger.kernel.org>; Lucas Bates <lucasb@mojatatu.com>; Jiri Pirko
> <jiri@resnulli.us>; David Miller <davem@davemloft.net>
> Subject: Re: [patch net v2 1/4] net/sched: Change tc_action refcnt and
> bindcnt to atomic
>
> On Sun, Oct 22, 2017 at 7:47 PM, Chris Mi <chrism@mellanox.com> wrote:
> >
> > It seems it is not easy to discard call_rcu(). I'm afraid even if we
> > have a final solution without call_rcu(), it is not mature at the
> > beginning as well. I mean we also need time
>
> Why do you believe it is not easy? RTNL lock is already there,
> list_splice_init_rcu() is there too. I can naturally divide my patches for each
> module so that they are much easier to backport than yours.
I tested your patches. It takes about 17 seconds to run the same test.
I believe your code is simpler and easy to maintain. If it is stable,
we will also get benefit. Thanks!
>
>
> > to fix the possible bugs of the new design. And maybe to destroy the
> > filters in parallel is the right direction. If this bug is the last
> > bug brought by call_rcu(), then changing it may not be a good idea.
>
> Again, you have to prove this is the last bug, I seriously doubt it is.
>
>
> >
> > Patch 1 is straightforward to use atomic. Patch 2 is to convert the list to
> array.
>
> Both are big in size.
>
>
> > I think there is no harm to the new design. Patch 3 and 4 are useful test
> case.
>
> It definitely doesn't harm, but why do we waste time on it when we know
> there is a better way? It is clearly not easy for backport either, in fact it is
> harder w.r.t. size.
>
>
> > We also need it with new design to make sure there is no regression.
> >
>
> Are you saying I can't trust your test cases? ;)
>
>
> > So I think my patch set should not be held so long time.
>
> I think your patches should be dropped except the last two, I will take the
> last two for you.
>
> Thanks!
^ permalink raw reply
* Re: [PATCH v2] sctp: full support for ipv6 ip_nonlocal_bind & IP_FREEBIND
From: Xin Long @ 2017-10-24 6:18 UTC (permalink / raw)
To: Laszlo Toth
Cc: Vlad Yasevich, Neil Horman, linux-sctp, network dev,
David S. Miller
In-Reply-To: <20171023171933.GA4592@ubuntu-desk-vm>
On Tue, Oct 24, 2017 at 1:19 AM, Laszlo Toth <laszlth@gmail.com> wrote:
> Commit 9b9742022888 ("sctp: support ipv6 nonlocal bind")
> introduced support for the above options as v4 sctp did,
> so patched sctp_v6_available().
>
> In the v4 implementation it's enough, because
> sctp_inet_bind_verify() just returns with sctp_v4_available().
> However sctp_inet6_bind_verify() has an extra check before that
> for link-local scope_id, which won't respect the above options.
>
> Added the checks before calling ipv6_chk_addr(), but
> not before the validation of scope_id.
>
> before (w/ both options):
> ./v6test fe80::10 sctp
> bind failed, errno: 99 (Cannot assign requested address)
> ./v6test fe80::10 tcp
> bind success, errno: 0 (Success)
>
> after (w/ both options):
> ./v6test fe80::10 sctp
> bind success, errno: 0 (Success)
>
> Signed-off-by: Laszlo Toth <laszlth@gmail.com>
> ---
> changed in v2:
> - fixed indentation
>
> net/sctp/ipv6.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index 51c4887..7fe9e1d1 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -882,8 +882,10 @@ static int sctp_inet6_bind_verify(struct sctp_sock *opt, union sctp_addr *addr)
> net = sock_net(&opt->inet.sk);
> rcu_read_lock();
> dev = dev_get_by_index_rcu(net, addr->v6.sin6_scope_id);
> - if (!dev ||
> - !ipv6_chk_addr(net, &addr->v6.sin6_addr, dev, 0)) {
> + if (!dev || !(opt->inet.freebind ||
> + net->ipv6.sysctl.ip_nonlocal_bind ||
> + ipv6_chk_addr(net, &addr->v6.sin6_addr,
> + dev, 0))) {
> rcu_read_unlock();
> return 0;
> }
> --
> 2.7.4
>
Reviewed-by: Xin Long <lucien.xin@gmail.com>
Thanks.
^ permalink raw reply
* Re: [for-next 08/12] net/mlx5e: IPoIB, Use hash-table to map between QPN to child netdev
From: Alex Vesker @ 2017-10-24 6:28 UTC (permalink / raw)
To: Jason Gunthorpe, Saeed Mahameed
Cc: David S. Miller, Doug Ledford, netdev, linux-rdma,
Leon Romanovsky
In-Reply-To: <20171023154736.GF11952@obsidianresearch.com>
On 10/23/2017 6:47 PM, Jason Gunthorpe wrote:
> On Sat, Oct 14, 2017 at 11:48:23AM -0700, Saeed Mahameed wrote:
>> From: Alex Vesker <valex@mellanox.com>
>>
>> This change is needed for PKEY support, since the RQs are shared
>> between the child interface and the parent. The parent is responsible
>> for NAPI and the precessing of RX completions. Using the dqpn in the
>> completion descriptor we set the corresponding child IPoIB netdevice
>> on the SKB.
>> The mapping between the dqpn and the netdevice is done using a HT,
>> each mlx5 IPoIB interface registers its mapping on creation.
> It seems really really weird to share the receive Q across all of the
> children and do the sorting in software.. why is this done like this?
>
> Wouldn't it be better to allow the children to progress concurrently,
> potentially on multiple cores? They all have their own QPs after all..
>
> Jason
The child interface RQs are in minimum size since they are not used.
The reason for this is saving resources, since we want to be able to
support many PKEYs
and allocating less memory while reusing HW resources such as flow
steering table.
This solution allowed us save memory on allocating the RQs of the child
and use the same
HW flow steering table, TIR, RQT. Using the hash table doesn't impact on
performance.
The parent have multiple RQs on different cores running in parallel.
^ permalink raw reply
* [PATCH net-next v3 2/3] bpf: permit multiple bpf attachments for a single perf event
From: Yonghong Song @ 2017-10-24 6:53 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, kafai, netdev; +Cc: kernel-team
In-Reply-To: <20171024065309.2401893-1-yhs@fb.com>
This patch enables multiple bpf attachments for a
kprobe/uprobe/tracepoint single trace event.
Each trace_event keeps a list of attached perf events.
When an event happens, all attached bpf programs will
be executed based on the order of attachment.
A global bpf_event_mutex lock is introduced to protect
prog_array attaching and detaching. An alternative will
be introduce a mutex lock in every trace_event_call
structure, but it takes a lot of extra memory.
So a global bpf_event_mutex lock is a good compromise.
The bpf prog detachment involves allocation of memory.
If the allocation fails, a dummy do-nothing program
will replace to-be-detached program in-place.
Signed-off-by: Yonghong Song <yhs@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
include/linux/bpf.h | 30 +++++++++++++---
include/linux/trace_events.h | 43 ++++++++++++++++++++---
include/trace/perf.h | 6 ++--
kernel/bpf/core.c | 81 ++++++++++++++++++++++++++++++++++++++++++
kernel/events/core.c | 26 +++++---------
kernel/trace/bpf_trace.c | 82 ++++++++++++++++++++++++++++++++++++++++---
kernel/trace/trace_kprobe.c | 6 ++--
kernel/trace/trace_syscalls.c | 34 ++++++++++--------
kernel/trace/trace_uprobe.c | 3 +-
9 files changed, 255 insertions(+), 56 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 1e334b2..172be7f 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -273,18 +273,38 @@ int bpf_prog_array_length(struct bpf_prog_array __rcu *progs);
int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
__u32 __user *prog_ids, u32 cnt);
-#define BPF_PROG_RUN_ARRAY(array, ctx, func) \
+void bpf_prog_array_delete_safe(struct bpf_prog_array __rcu *progs,
+ struct bpf_prog *old_prog);
+int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
+ struct bpf_prog *exclude_prog,
+ struct bpf_prog *include_prog,
+ struct bpf_prog_array **new_array);
+
+#define __BPF_PROG_RUN_ARRAY(array, ctx, func, check_non_null) \
({ \
- struct bpf_prog **_prog; \
+ struct bpf_prog **_prog, *__prog; \
+ struct bpf_prog_array *_array; \
u32 _ret = 1; \
rcu_read_lock(); \
- _prog = rcu_dereference(array)->progs; \
- for (; *_prog; _prog++) \
- _ret &= func(*_prog, ctx); \
+ _array = rcu_dereference(array); \
+ if (unlikely(check_non_null && !_array))\
+ goto _out; \
+ _prog = _array->progs; \
+ while ((__prog = READ_ONCE(*_prog))) { \
+ _ret &= func(__prog, ctx); \
+ _prog++; \
+ } \
+_out: \
rcu_read_unlock(); \
_ret; \
})
+#define BPF_PROG_RUN_ARRAY(array, ctx, func) \
+ __BPF_PROG_RUN_ARRAY(array, ctx, func, false)
+
+#define BPF_PROG_RUN_ARRAY_CHECK(array, ctx, func) \
+ __BPF_PROG_RUN_ARRAY(array, ctx, func, true)
+
#ifdef CONFIG_BPF_SYSCALL
DECLARE_PER_CPU(int, bpf_prog_active);
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 2e0f222..fc6aeca 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -271,14 +271,37 @@ struct trace_event_call {
#ifdef CONFIG_PERF_EVENTS
int perf_refcount;
struct hlist_head __percpu *perf_events;
- struct bpf_prog *prog;
- struct perf_event *bpf_prog_owner;
+ struct bpf_prog_array __rcu *prog_array;
int (*perf_perm)(struct trace_event_call *,
struct perf_event *);
#endif
};
+#ifdef CONFIG_PERF_EVENTS
+static inline bool bpf_prog_array_valid(struct trace_event_call *call)
+{
+ /*
+ * This inline function checks whether call->prog_array
+ * is valid or not. The function is called in various places,
+ * outside rcu_read_lock/unlock, as a heuristic to speed up execution.
+ *
+ * If this function returns true, and later call->prog_array
+ * becomes false inside rcu_read_lock/unlock region,
+ * we bail out then. If this function return false,
+ * there is a risk that we might miss a few events if the checking
+ * were delayed until inside rcu_read_lock/unlock region and
+ * call->prog_array happened to become non-NULL then.
+ *
+ * Here, READ_ONCE() is used instead of rcu_access_pointer().
+ * rcu_access_pointer() requires the actual definition of
+ * "struct bpf_prog_array" while READ_ONCE() only needs
+ * a declaration of the same type.
+ */
+ return !!READ_ONCE(call->prog_array);
+}
+#endif
+
static inline const char *
trace_event_name(struct trace_event_call *call)
{
@@ -435,12 +458,23 @@ trace_trigger_soft_disabled(struct trace_event_file *file)
}
#ifdef CONFIG_BPF_EVENTS
-unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx);
+unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx);
+int perf_event_attach_bpf_prog(struct perf_event *event, struct bpf_prog *prog);
+void perf_event_detach_bpf_prog(struct perf_event *event);
#else
-static inline unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx)
+static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
{
return 1;
}
+
+static inline int
+perf_event_attach_bpf_prog(struct perf_event *event, struct bpf_prog *prog)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline void perf_event_detach_bpf_prog(struct perf_event *event) { }
+
#endif
enum {
@@ -511,6 +545,7 @@ perf_trace_buf_submit(void *raw_data, int size, int rctx, u16 type,
{
perf_tp_event(type, count, raw_data, size, regs, head, rctx, task, event);
}
+
#endif
#endif /* _LINUX_TRACE_EVENT_H */
diff --git a/include/trace/perf.h b/include/trace/perf.h
index 04fe68bb..14f127b6 100644
--- a/include/trace/perf.h
+++ b/include/trace/perf.h
@@ -34,7 +34,6 @@ perf_trace_##call(void *__data, proto) \
struct trace_event_call *event_call = __data; \
struct trace_event_data_offsets_##call __maybe_unused __data_offsets;\
struct trace_event_raw_##call *entry; \
- struct bpf_prog *prog = event_call->prog; \
struct pt_regs *__regs; \
u64 __count = 1; \
struct task_struct *__task = NULL; \
@@ -46,8 +45,9 @@ perf_trace_##call(void *__data, proto) \
__data_size = trace_event_get_offsets_##call(&__data_offsets, args); \
\
head = this_cpu_ptr(event_call->perf_events); \
- if (!prog && __builtin_constant_p(!__task) && !__task && \
- hlist_empty(head)) \
+ if (!bpf_prog_array_valid(event_call) && \
+ __builtin_constant_p(!__task) && !__task && \
+ hlist_empty(head)) \
return; \
\
__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 8e7c8bf..7fe4487 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1394,6 +1394,20 @@ struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
}
EXPORT_SYMBOL_GPL(bpf_prog_select_runtime);
+static unsigned int __bpf_prog_ret1(const void *ctx,
+ const struct bpf_insn *insn)
+{
+ return 1;
+}
+
+static struct bpf_prog_dummy {
+ struct bpf_prog prog;
+} dummy_bpf_prog = {
+ .prog = {
+ .bpf_func = __bpf_prog_ret1,
+ },
+};
+
/* to avoid allocating empty bpf_prog_array for cgroups that
* don't have bpf program attached use one global 'empty_prog_array'
* It will not be modified the caller of bpf_prog_array_alloc()
@@ -1463,6 +1477,73 @@ int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
return 0;
}
+void bpf_prog_array_delete_safe(struct bpf_prog_array __rcu *progs,
+ struct bpf_prog *old_prog)
+{
+ struct bpf_prog **prog = progs->progs;
+
+ for (; *prog; prog++)
+ if (*prog == old_prog) {
+ WRITE_ONCE(*prog, &dummy_bpf_prog.prog);
+ break;
+ }
+}
+
+int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
+ struct bpf_prog *exclude_prog,
+ struct bpf_prog *include_prog,
+ struct bpf_prog_array **new_array)
+{
+ int new_prog_cnt, carry_prog_cnt = 0;
+ struct bpf_prog **existing_prog;
+ struct bpf_prog_array *array;
+ int new_prog_idx = 0;
+
+ /* Figure out how many existing progs we need to carry over to
+ * the new array.
+ */
+ if (old_array) {
+ existing_prog = old_array->progs;
+ for (; *existing_prog; existing_prog++) {
+ if (*existing_prog != exclude_prog &&
+ *existing_prog != &dummy_bpf_prog.prog)
+ carry_prog_cnt++;
+ if (*existing_prog == include_prog)
+ return -EEXIST;
+ }
+ }
+
+ /* How many progs (not NULL) will be in the new array? */
+ new_prog_cnt = carry_prog_cnt;
+ if (include_prog)
+ new_prog_cnt += 1;
+
+ /* Do we have any prog (not NULL) in the new array? */
+ if (!new_prog_cnt) {
+ *new_array = NULL;
+ return 0;
+ }
+
+ /* +1 as the end of prog_array is marked with NULL */
+ array = bpf_prog_array_alloc(new_prog_cnt + 1, GFP_KERNEL);
+ if (!array)
+ return -ENOMEM;
+
+ /* Fill in the new prog array */
+ if (carry_prog_cnt) {
+ existing_prog = old_array->progs;
+ for (; *existing_prog; existing_prog++)
+ if (*existing_prog != exclude_prog &&
+ *existing_prog != &dummy_bpf_prog.prog)
+ array->progs[new_prog_idx++] = *existing_prog;
+ }
+ if (include_prog)
+ array->progs[new_prog_idx++] = include_prog;
+ array->progs[new_prog_idx] = NULL;
+ *new_array = array;
+ return 0;
+}
+
static void bpf_prog_free_deferred(struct work_struct *work)
{
struct bpf_prog_aux *aux;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 9f78a682..9660ee6 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7954,11 +7954,9 @@ void perf_trace_run_bpf_submit(void *raw_data, int size, int rctx,
struct pt_regs *regs, struct hlist_head *head,
struct task_struct *task)
{
- struct bpf_prog *prog = call->prog;
-
- if (prog) {
+ if (bpf_prog_array_valid(call)) {
*(struct pt_regs **)raw_data = regs;
- if (!trace_call_bpf(prog, raw_data) || hlist_empty(head)) {
+ if (!trace_call_bpf(call, raw_data) || hlist_empty(head)) {
perf_swevent_put_recursion_context(rctx);
return;
}
@@ -8147,13 +8145,11 @@ static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
{
bool is_kprobe, is_tracepoint, is_syscall_tp;
struct bpf_prog *prog;
+ int ret;
if (event->attr.type != PERF_TYPE_TRACEPOINT)
return perf_event_set_bpf_handler(event, prog_fd);
- if (event->tp_event->prog)
- return -EEXIST;
-
is_kprobe = event->tp_event->flags & TRACE_EVENT_FL_UKPROBE;
is_tracepoint = event->tp_event->flags & TRACE_EVENT_FL_TRACEPOINT;
is_syscall_tp = is_syscall_trace_event(event->tp_event);
@@ -8181,26 +8177,20 @@ static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
return -EACCES;
}
}
- event->tp_event->prog = prog;
- event->tp_event->bpf_prog_owner = event;
- return 0;
+ ret = perf_event_attach_bpf_prog(event, prog);
+ if (ret)
+ bpf_prog_put(prog);
+ return ret;
}
static void perf_event_free_bpf_prog(struct perf_event *event)
{
- struct bpf_prog *prog;
-
if (event->attr.type != PERF_TYPE_TRACEPOINT) {
perf_event_free_bpf_handler(event);
return;
}
-
- prog = event->tp_event->prog;
- if (prog && event->tp_event->bpf_prog_owner == event) {
- event->tp_event->prog = NULL;
- bpf_prog_put(prog);
- }
+ perf_event_detach_bpf_prog(event);
}
#else
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 3126da2..b65011d 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -17,7 +17,7 @@
/**
* trace_call_bpf - invoke BPF program
- * @prog: BPF program
+ * @call: tracepoint event
* @ctx: opaque context pointer
*
* kprobe handlers execute BPF programs via this helper.
@@ -29,7 +29,7 @@
* 1 - store kprobe event into ring buffer
* Other values are reserved and currently alias to 1
*/
-unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx)
+unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
{
unsigned int ret;
@@ -49,9 +49,22 @@ unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx)
goto out;
}
- rcu_read_lock();
- ret = BPF_PROG_RUN(prog, ctx);
- rcu_read_unlock();
+ /*
+ * Instead of moving rcu_read_lock/rcu_dereference/rcu_read_unlock
+ * to all call sites, we did a bpf_prog_array_valid() there to check
+ * whether call->prog_array is empty or not, which is
+ * a heurisitc to speed up execution.
+ *
+ * If bpf_prog_array_valid() fetched prog_array was
+ * non-NULL, we go into trace_call_bpf() and do the actual
+ * proper rcu_dereference() under RCU lock.
+ * If it turns out that prog_array is NULL then, we bail out.
+ * For the opposite, if the bpf_prog_array_valid() fetched pointer
+ * was NULL, you'll skip the prog_array with the risk of missing
+ * out of events when it was updated in between this and the
+ * rcu_dereference() which is accepted risk.
+ */
+ ret = BPF_PROG_RUN_ARRAY_CHECK(call->prog_array, ctx, BPF_PROG_RUN);
out:
__this_cpu_dec(bpf_prog_active);
@@ -741,3 +754,62 @@ const struct bpf_verifier_ops perf_event_verifier_ops = {
const struct bpf_prog_ops perf_event_prog_ops = {
};
+
+static DEFINE_MUTEX(bpf_event_mutex);
+
+int perf_event_attach_bpf_prog(struct perf_event *event,
+ struct bpf_prog *prog)
+{
+ struct bpf_prog_array __rcu *old_array;
+ struct bpf_prog_array *new_array;
+ int ret = -EEXIST;
+
+ mutex_lock(&bpf_event_mutex);
+
+ if (event->prog)
+ goto out;
+
+ old_array = rcu_dereference_protected(event->tp_event->prog_array,
+ lockdep_is_held(&bpf_event_mutex));
+ ret = bpf_prog_array_copy(old_array, NULL, prog, &new_array);
+ if (ret < 0)
+ goto out;
+
+ /* set the new array to event->tp_event and set event->prog */
+ event->prog = prog;
+ rcu_assign_pointer(event->tp_event->prog_array, new_array);
+ bpf_prog_array_free(old_array);
+
+out:
+ mutex_unlock(&bpf_event_mutex);
+ return ret;
+}
+
+void perf_event_detach_bpf_prog(struct perf_event *event)
+{
+ struct bpf_prog_array __rcu *old_array;
+ struct bpf_prog_array *new_array;
+ int ret;
+
+ mutex_lock(&bpf_event_mutex);
+
+ if (!event->prog)
+ goto out;
+
+ old_array = rcu_dereference_protected(event->tp_event->prog_array,
+ lockdep_is_held(&bpf_event_mutex));
+
+ ret = bpf_prog_array_copy(old_array, event->prog, NULL, &new_array);
+ if (ret < 0) {
+ bpf_prog_array_delete_safe(old_array, event->prog);
+ } else {
+ rcu_assign_pointer(event->tp_event->prog_array, new_array);
+ bpf_prog_array_free(old_array);
+ }
+
+ bpf_prog_put(event->prog);
+ event->prog = NULL;
+
+out:
+ mutex_unlock(&bpf_event_mutex);
+}
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 8a907e1..abf92e4 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1174,13 +1174,12 @@ static void
kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs *regs)
{
struct trace_event_call *call = &tk->tp.call;
- struct bpf_prog *prog = call->prog;
struct kprobe_trace_entry_head *entry;
struct hlist_head *head;
int size, __size, dsize;
int rctx;
- if (prog && !trace_call_bpf(prog, regs))
+ if (bpf_prog_array_valid(call) && !trace_call_bpf(call, regs))
return;
head = this_cpu_ptr(call->perf_events);
@@ -1210,13 +1209,12 @@ kretprobe_perf_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
struct pt_regs *regs)
{
struct trace_event_call *call = &tk->tp.call;
- struct bpf_prog *prog = call->prog;
struct kretprobe_trace_entry_head *entry;
struct hlist_head *head;
int size, __size, dsize;
int rctx;
- if (prog && !trace_call_bpf(prog, regs))
+ if (bpf_prog_array_valid(call) && !trace_call_bpf(call, regs))
return;
head = this_cpu_ptr(call->perf_events);
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 696afe7..71a6af3 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -559,9 +559,10 @@ static DECLARE_BITMAP(enabled_perf_exit_syscalls, NR_syscalls);
static int sys_perf_refcount_enter;
static int sys_perf_refcount_exit;
-static int perf_call_bpf_enter(struct bpf_prog *prog, struct pt_regs *regs,
- struct syscall_metadata *sys_data,
- struct syscall_trace_enter *rec) {
+static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *regs,
+ struct syscall_metadata *sys_data,
+ struct syscall_trace_enter *rec)
+{
struct syscall_tp_t {
unsigned long long regs;
unsigned long syscall_nr;
@@ -573,7 +574,7 @@ static int perf_call_bpf_enter(struct bpf_prog *prog, struct pt_regs *regs,
param.syscall_nr = rec->nr;
for (i = 0; i < sys_data->nb_args; i++)
param.args[i] = rec->args[i];
- return trace_call_bpf(prog, ¶m);
+ return trace_call_bpf(call, ¶m);
}
static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
@@ -581,7 +582,7 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
struct syscall_metadata *sys_data;
struct syscall_trace_enter *rec;
struct hlist_head *head;
- struct bpf_prog *prog;
+ bool valid_prog_array;
int syscall_nr;
int rctx;
int size;
@@ -596,9 +597,9 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
if (!sys_data)
return;
- prog = READ_ONCE(sys_data->enter_event->prog);
head = this_cpu_ptr(sys_data->enter_event->perf_events);
- if (!prog && hlist_empty(head))
+ valid_prog_array = bpf_prog_array_valid(sys_data->enter_event);
+ if (!valid_prog_array && hlist_empty(head))
return;
/* get the size after alignment with the u32 buffer size field */
@@ -614,7 +615,8 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
syscall_get_arguments(current, regs, 0, sys_data->nb_args,
(unsigned long *)&rec->args);
- if ((prog && !perf_call_bpf_enter(prog, regs, sys_data, rec)) ||
+ if ((valid_prog_array &&
+ !perf_call_bpf_enter(sys_data->enter_event, regs, sys_data, rec)) ||
hlist_empty(head)) {
perf_swevent_put_recursion_context(rctx);
return;
@@ -659,8 +661,9 @@ static void perf_sysenter_disable(struct trace_event_call *call)
mutex_unlock(&syscall_trace_lock);
}
-static int perf_call_bpf_exit(struct bpf_prog *prog, struct pt_regs *regs,
- struct syscall_trace_exit *rec) {
+static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *regs,
+ struct syscall_trace_exit *rec)
+{
struct syscall_tp_t {
unsigned long long regs;
unsigned long syscall_nr;
@@ -670,7 +673,7 @@ static int perf_call_bpf_exit(struct bpf_prog *prog, struct pt_regs *regs,
*(struct pt_regs **)¶m = regs;
param.syscall_nr = rec->nr;
param.ret = rec->ret;
- return trace_call_bpf(prog, ¶m);
+ return trace_call_bpf(call, ¶m);
}
static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
@@ -678,7 +681,7 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
struct syscall_metadata *sys_data;
struct syscall_trace_exit *rec;
struct hlist_head *head;
- struct bpf_prog *prog;
+ bool valid_prog_array;
int syscall_nr;
int rctx;
int size;
@@ -693,9 +696,9 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
if (!sys_data)
return;
- prog = READ_ONCE(sys_data->exit_event->prog);
head = this_cpu_ptr(sys_data->exit_event->perf_events);
- if (!prog && hlist_empty(head))
+ valid_prog_array = bpf_prog_array_valid(sys_data->exit_event);
+ if (!valid_prog_array && hlist_empty(head))
return;
/* We can probably do that at build time */
@@ -709,7 +712,8 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
rec->nr = syscall_nr;
rec->ret = syscall_get_return_value(current, regs);
- if ((prog && !perf_call_bpf_exit(prog, regs, rec)) ||
+ if ((valid_prog_array &&
+ !perf_call_bpf_exit(sys_data->exit_event, regs, rec)) ||
hlist_empty(head)) {
perf_swevent_put_recursion_context(rctx);
return;
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 4525e02..153c0e4 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -1113,13 +1113,12 @@ static void __uprobe_perf_func(struct trace_uprobe *tu,
{
struct trace_event_call *call = &tu->tp.call;
struct uprobe_trace_entry_head *entry;
- struct bpf_prog *prog = call->prog;
struct hlist_head *head;
void *data;
int size, esize;
int rctx;
- if (prog && !trace_call_bpf(prog, regs))
+ if (bpf_prog_array_valid(call) && !trace_call_bpf(call, regs))
return;
esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
--
2.9.5
^ permalink raw reply related
* [PATCH net-next v3 0/3] bpf: permit multiple bpf attachments for a single perf tracepoint event
From: Yonghong Song @ 2017-10-24 6:53 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, kafai, netdev; +Cc: kernel-team
This patch set adds support to permit multiple bpf prog attachments
for a single perf tracepoint event. Patch 1 does some cleanup such
that perf_event_{set|free}_bpf_handler is called under the
same condition. Patch 2 has the core implementation, and
Patch 3 adds a test case.
Changelogs:
v2 -> v3:
. fix compilation error.
v1 -> v2:
. fix a potential deadlock issue discovered by Daniel.
. fix some coding style issues.
Yonghong Song (3):
bpf: use the same condition in perf event set/free bpf handler
bpf: permit multiple bpf attachments for a single perf event
bpf: add a test case to test single tp multiple bpf attachment
include/linux/bpf.h | 30 +++++++++++++---
include/linux/trace_events.h | 43 ++++++++++++++++++++---
include/trace/perf.h | 6 ++--
kernel/bpf/core.c | 81 ++++++++++++++++++++++++++++++++++++++++++
kernel/events/core.c | 30 ++++++----------
kernel/trace/bpf_trace.c | 82 ++++++++++++++++++++++++++++++++++++++++---
kernel/trace/trace_kprobe.c | 6 ++--
kernel/trace/trace_syscalls.c | 34 ++++++++++--------
kernel/trace/trace_uprobe.c | 3 +-
samples/bpf/syscall_tp_user.c | 66 +++++++++++++++++++++++++++-------
10 files changed, 310 insertions(+), 71 deletions(-)
--
2.9.5
^ permalink raw reply
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