* [PATCH 09/18] qlogic/qed: change strncpy+truncation to strlcpy
From: Dominique Martinet @ 2018-07-13 1:25 UTC (permalink / raw)
Cc: Dominique Martinet, Ariel Elior, everest-linux-l2,
David S. Miller, netdev, linux-kernel
In-Reply-To: <1531444483-17338-1-git-send-email-asmadeus@codewreck.org>
Generated by scripts/coccinelle/misc/strncpy_truncation.cocci
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the
first patch of the serie) for the motivation behind this patch
drivers/net/ethernet/qlogic/qed/qed_debug.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_debug.c b/drivers/net/ethernet/qlogic/qed/qed_debug.c
index a14e48489029..4fe0a72230e8 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_debug.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_debug.c
@@ -6212,8 +6212,7 @@ static void qed_read_str_from_buf(void *buf, u32 *offset, u32 size, char *dest)
{
const char *source_str = &((const char *)buf)[*offset];
- strncpy(dest, source_str, size);
- dest[size - 1] = '\0';
+ strlcpy(dest, source_str, size);
*offset += size;
}
--
2.17.1
^ permalink raw reply related
* [PATCH 10/18] brcmsmac: change strncpy+truncation to strlcpy
From: Dominique Martinet @ 2018-07-13 1:25 UTC (permalink / raw)
Cc: Dominique Martinet, Arend van Spriel, Franky Lin, Hante Meuleman,
Chi-Hsien Lin, Wright Feng, Kalle Valo, David S. Miller,
linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
netdev, linux-kernel
In-Reply-To: <1531444483-17338-1-git-send-email-asmadeus@codewreck.org>
Generated by scripts/coccinelle/misc/strncpy_truncation.cocci
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the
first patch of the serie) for the motivation behind this patch
drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c
index b7df576bb84d..58ccd72d672c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c
@@ -584,8 +584,7 @@ struct dma_pub *dma_attach(char *name, struct brcms_c_info *wlc,
rxextheadroom, nrxpost, rxoffset, txregbase, rxregbase);
/* make a private copy of our callers name */
- strncpy(di->name, name, MAXNAMEL);
- di->name[MAXNAMEL - 1] = '\0';
+ strlcpy(di->name, name, MAXNAMEL);
di->dmadev = core->dma_dev;
--
2.17.1
^ permalink raw reply related
* [PATCH 11/18] wireless/ti: change strncpy+truncation to strlcpy
From: Dominique Martinet @ 2018-07-13 1:25 UTC (permalink / raw)
Cc: Dominique Martinet, Kalle Valo, David S. Miller, Thomas Gleixner,
Greg Kroah-Hartman, Kate Stewart, Philippe Ombredanne,
Joe Perches, linux-wireless, netdev, linux-kernel
In-Reply-To: <1531444483-17338-1-git-send-email-asmadeus@codewreck.org>
Generated by scripts/coccinelle/misc/strncpy_truncation.cocci
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the
first patch of the serie) for the motivation behind this patch
drivers/net/wireless/ti/wl1251/acx.c | 9 +--------
drivers/net/wireless/ti/wl18xx/main.c | 5 +----
drivers/net/wireless/ti/wlcore/boot.c | 5 +----
3 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/ti/wl1251/acx.c b/drivers/net/wireless/ti/wl1251/acx.c
index f78fc3880423..c4f1a63300bb 100644
--- a/drivers/net/wireless/ti/wl1251/acx.c
+++ b/drivers/net/wireless/ti/wl1251/acx.c
@@ -150,14 +150,7 @@ int wl1251_acx_fw_version(struct wl1251 *wl, char *buf, size_t len)
}
/* be careful with the buffer sizes */
- strncpy(buf, rev->fw_version, min(len, sizeof(rev->fw_version)));
-
- /*
- * if the firmware version string is exactly
- * sizeof(rev->fw_version) long or fw_len is less than
- * sizeof(rev->fw_version) it won't be null terminated
- */
- buf[min(len, sizeof(rev->fw_version)) - 1] = '\0';
+ strlcpy(buf, rev->fw_version, min(len, sizeof(rev->fw_version)));
out:
kfree(rev);
diff --git a/drivers/net/wireless/ti/wl18xx/main.c b/drivers/net/wireless/ti/wl18xx/main.c
index ca0f936fc119..8595e9bf1cfa 100644
--- a/drivers/net/wireless/ti/wl18xx/main.c
+++ b/drivers/net/wireless/ti/wl18xx/main.c
@@ -1529,12 +1529,9 @@ static int wl18xx_handle_static_data(struct wl1271 *wl,
struct wl18xx_static_data_priv *static_data_priv =
(struct wl18xx_static_data_priv *) static_data->priv;
- strncpy(wl->chip.phy_fw_ver_str, static_data_priv->phy_version,
+ strlcpy(wl->chip.phy_fw_ver_str, static_data_priv->phy_version,
sizeof(wl->chip.phy_fw_ver_str));
- /* make sure the string is NULL-terminated */
- wl->chip.phy_fw_ver_str[sizeof(wl->chip.phy_fw_ver_str) - 1] = '\0';
-
wl1271_info("PHY firmware version: %s", static_data_priv->phy_version);
return 0;
diff --git a/drivers/net/wireless/ti/wlcore/boot.c b/drivers/net/wireless/ti/wlcore/boot.c
index f00509ea8aca..6b33951d5b34 100644
--- a/drivers/net/wireless/ti/wlcore/boot.c
+++ b/drivers/net/wireless/ti/wlcore/boot.c
@@ -55,12 +55,9 @@ static int wlcore_boot_parse_fw_ver(struct wl1271 *wl,
{
int ret;
- strncpy(wl->chip.fw_ver_str, static_data->fw_version,
+ strlcpy(wl->chip.fw_ver_str, static_data->fw_version,
sizeof(wl->chip.fw_ver_str));
- /* make sure the string is NULL-terminated */
- wl->chip.fw_ver_str[sizeof(wl->chip.fw_ver_str) - 1] = '\0';
-
ret = sscanf(wl->chip.fw_ver_str + 4, "%u.%u.%u.%u.%u",
&wl->chip.fw_ver[0], &wl->chip.fw_ver[1],
&wl->chip.fw_ver[2], &wl->chip.fw_ver[3],
--
2.17.1
^ permalink raw reply related
* [PATCH net-next] bnxt_en: remove redundant debug register dma mem allocation
From: YueHaibing @ 2018-07-13 3:02 UTC (permalink / raw)
To: michael.chan; +Cc: linux-kernel, netdev, davem, YueHaibing
hwrm_dbg_resp_addr and hwrm_dbg_resp_dma_addr are never used
and can be removed.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 13 -------------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 ---
2 files changed, 16 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 4394c11..4db3180 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -3012,13 +3012,6 @@ static void bnxt_free_hwrm_resources(struct bnxt *bp)
bp->hwrm_cmd_resp_dma_addr);
bp->hwrm_cmd_resp_addr = NULL;
- if (bp->hwrm_dbg_resp_addr) {
- dma_free_coherent(&pdev->dev, HWRM_DBG_REG_BUF_SIZE,
- bp->hwrm_dbg_resp_addr,
- bp->hwrm_dbg_resp_dma_addr);
-
- bp->hwrm_dbg_resp_addr = NULL;
- }
}
static int bnxt_alloc_hwrm_resources(struct bnxt *bp)
@@ -3030,12 +3023,6 @@ static int bnxt_alloc_hwrm_resources(struct bnxt *bp)
GFP_KERNEL);
if (!bp->hwrm_cmd_resp_addr)
return -ENOMEM;
- bp->hwrm_dbg_resp_addr = dma_alloc_coherent(&pdev->dev,
- HWRM_DBG_REG_BUF_SIZE,
- &bp->hwrm_dbg_resp_dma_addr,
- GFP_KERNEL);
- if (!bp->hwrm_dbg_resp_addr)
- netdev_warn(bp->dev, "fail to alloc debug register dma mem\n");
return 0;
}
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 91575ef..934aa11 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1287,9 +1287,6 @@ struct bnxt {
dma_addr_t hwrm_short_cmd_req_dma_addr;
void *hwrm_cmd_resp_addr;
dma_addr_t hwrm_cmd_resp_dma_addr;
- void *hwrm_dbg_resp_addr;
- dma_addr_t hwrm_dbg_resp_dma_addr;
-#define HWRM_DBG_REG_BUF_SIZE 128
struct rx_port_stats *hw_rx_port_stats;
struct tx_port_stats *hw_tx_port_stats;
--
2.7.0
^ permalink raw reply related
* Re: [PATCH net-next v2 0/5] virtio: support packed ring
From: Michael S. Tsirkin @ 2018-07-13 3:26 UTC (permalink / raw)
To: David Miller
Cc: tiwei.bie, jasowang, virtualization, linux-kernel, netdev,
virtio-dev, wexu, jfreimann
In-Reply-To: <20180712.144458.2076041018423659380.davem@davemloft.net>
On Thu, Jul 12, 2018 at 02:44:58PM -0700, David Miller wrote:
> From: Tiwei Bie <tiwei.bie@intel.com>
> Date: Wed, 11 Jul 2018 10:27:06 +0800
>
> > Hello everyone,
> >
> > This patch set implements packed ring support in virtio driver.
> >
> > Some functional tests have been done with Jason's
> > packed ring implementation in vhost:
> >
> > https://lkml.org/lkml/2018/7/3/33
> >
> > Both of ping and netperf worked as expected.
>
> Michael and Jason, where are we with this series?
I'm at netdev, won't be able to review before Monday.
--
MST
^ permalink raw reply
* Re: Re: [Qemu-devel] [PATCH v3 0/3] Use of unique identifier for pairing virtio and passthrough devices...
From: Michael S. Tsirkin @ 2018-07-13 3:28 UTC (permalink / raw)
To: Samudrala, Sridhar
Cc: Siwei Liu, Cornelia Huck, si-wei liu, Roman Kagan, Venu Busireddy,
Marcel Apfelbaum, virtio-dev, qemu-devel, Alexander Duyck, Netdev
In-Reply-To: <77823ff9-a02a-808b-e919-b4950a64ae3c@intel.com>
On Thu, Jul 12, 2018 at 09:20:41PM -0400, Samudrala, Sridhar wrote:
> On 7/12/2018 6:19 PM, Siwei Liu wrote:
> > On Thu, Jul 12, 2018 at 2:00 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > On Thu, Jul 12, 2018 at 01:52:53PM -0700, Siwei Liu wrote:
> > > > The definition is incomplete due to lack of spec. There's no "host"
> > > > part defined yet in the host-guest interface. If match by MAC is an
> > > > interface, the same must be done on the host(device) side as well,
> > > > which has been agreed not the way to go. However, I don't think that's
> > > > what the author intends to do by interpreting his QEMU patch - it
> > > > missed the other parts as well, such as the feature negotiation and
> > > > how it interacts with the paired device.
> > > >
> > > > What I said is that match by MAC is just a guest implementation that
> > > > one can change at any time. We now have the group ID on QEMU, why
> > > > still sticking to matching by MAC? It shoulnd't be a host-guest
> > > > interface in the first place anyway.
> > > I think that match by MAC is a simple portable way to match devices.
> > > E.g. it will work seamlessly with niche things like zPCI. However
> > That's a good point. I'm not sure if it's a valid assumption that zPCI
> > should always use the same MAC address as that of virtio. Someone
> > who's more familiar with the use case may decide and work on that. It
> > means VFIO device has to take in the MAC address as an identifier to
> > the "-device vfio-pci,.." QEMU option. I think there's no point to
> > match device using group ID in QEMU while using MAC in the guest.
> > Based on that assumption, I'd go with making VIRTIO_NET_F_STANDBY to
> > match device based on group ID, while someone may come up with another
> > feature bit later, say VIRTIO_NET_F_STANDBY_BY_MAC when its QEMU
> > support is available. Would it make sense?
>
> VIRTIO_NET_F_STANDBY as defined in the guest virtio_net driver supports match
> by MAC address. I think we should add support for this feature bit in QEMU.
> If submitting a patch to update the spec is a pre-requisite to add this
> feature bit to QEMU, i can do that.
It's not strictly a prerequisite but we need it in spec all the same, so
pls do that.
> As far as i understand, group id patches to QEMU are still under review.
> Matching by group ID can be another feature bit that could support matching
> by group id as well as MAC.
>
>
> > -Siwei
> >
> > > there are other niche use-cases that aren't addressed by match by MAC
> > > such as PF pass-through as a primary, and the pci bridge trick addresses
> > > that at cost of some portability.
> > >
> > > So I see no issues supporting both mechanisms, but others on the TC
> > > might feel differently.
> > >
> > > --
> > > MST
^ permalink raw reply
* Re: [PATCH v2 net-next 9/9] lan743x: Add PTP support
From: Richard Cochran @ 2018-07-13 3:31 UTC (permalink / raw)
To: Bryan Whitehead; +Cc: davem, netdev, UNGLinuxDriver
In-Reply-To: <1531422306-6547-10-git-send-email-Bryan.Whitehead@microchip.com>
On Thu, Jul 12, 2018 at 03:05:06PM -0400, Bryan Whitehead wrote:
> +static int lan743x_ethtool_get_ts_info(struct net_device *netdev,
> + struct ethtool_ts_info *ts_info)
> +{
> + struct lan743x_adapter *adapter = netdev_priv(netdev);
> +
> + ts_info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE |
> + SOF_TIMESTAMPING_RX_SOFTWARE |
> + SOF_TIMESTAMPING_SOFTWARE |
> + SOF_TIMESTAMPING_TX_HARDWARE |
> + SOF_TIMESTAMPING_RX_HARDWARE |
> + SOF_TIMESTAMPING_RAW_HARDWARE;
> +#ifdef CONFIG_PTP_1588_CLOCK
No need for this ifdeferry - ptp_clock_index() already returns -1 in
that case.
> + if (adapter->ptp.ptp_clock)
> + ts_info->phc_index = ptp_clock_index(adapter->ptp.ptp_clock);
> + else
> + ts_info->phc_index = -1;
> +#else
> + ts_info->phc_index = -1;
> +#endif
> + ts_info->tx_types = BIT(HWTSTAMP_TX_OFF) |
> + BIT(HWTSTAMP_TX_ON);
> + ts_info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) |
> + BIT(HWTSTAMP_FILTER_ALL);
> + return 0;
> +}
> +
> @@ -690,6 +717,7 @@ const struct ethtool_ops lan743x_ethtool_ops = {
> .get_rxfh_indir_size = lan743x_ethtool_get_rxfh_indir_size,
> .get_rxfh = lan743x_ethtool_get_rxfh,
> .set_rxfh = lan743x_ethtool_set_rxfh,
> + .get_ts_info = lan743x_ethtool_get_ts_info,
> .get_eee = lan743x_ethtool_get_eee,
> .set_eee = lan743x_ethtool_set_eee,
> .get_link_ksettings = phy_ethtool_get_link_ksettings,
> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
> index 953b581..ca9ae49 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.c
> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> @@ -267,6 +267,10 @@ static void lan743x_intr_shared_isr(void *context, u32 int_sts, u32 flags)
> lan743x_intr_software_isr(adapter);
> int_sts &= ~INT_BIT_SW_GP_;
> }
> + if (int_sts & INT_BIT_1588_) {
> + lan743x_ptp_isr(adapter);
> + int_sts &= ~INT_BIT_1588_;
> + }
> }
> if (int_sts)
> lan743x_csr_write(adapter, INT_EN_CLR, int_sts);
> @@ -976,6 +980,7 @@ static void lan743x_phy_link_status_change(struct net_device *netdev)
> ksettings.base.duplex,
> local_advertisement,
> remote_advertisement);
> + lan743x_ptp_update_latency(adapter, ksettings.base.speed);
> }
> }
>
> @@ -1256,11 +1261,29 @@ static void lan743x_tx_release_desc(struct lan743x_tx *tx,
> buffer_info->dma_ptr = 0;
> buffer_info->buffer_length = 0;
> }
> - if (buffer_info->skb) {
> + if (!buffer_info->skb)
> + goto clear_active;
> +
> + if (!(buffer_info->flags &
> + TX_BUFFER_INFO_FLAG_TIMESTAMP_REQUESTED)) {
Bad line break.
> dev_kfree_skb(buffer_info->skb);
> - buffer_info->skb = NULL;
> + goto clear_skb;
> }
>
> + if (cleanup) {
> + lan743x_ptp_unrequest_tx_timestamp(tx->adapter);
> + dev_kfree_skb(buffer_info->skb);
> + } else {
> + lan743x_ptp_tx_timestamp_skb(tx->adapter,
> + buffer_info->skb,
> + (buffer_info->flags &
> + TX_BUFFER_INFO_FLAG_IGNORE_SYNC)
> + != 0);
This is poor coding style. Please find a better way.
> + }
> +
> +clear_skb:
> + buffer_info->skb = NULL;
> +
> clear_active:
> buffer_info->flags &= ~TX_BUFFER_INFO_FLAG_ACTIVE;
>
> @@ -1321,10 +1344,25 @@ static int lan743x_tx_get_avail_desc(struct lan743x_tx *tx)
> return last_head - last_tail - 1;
> }
>
> +void lan743x_tx_set_timestamping_mode(struct lan743x_tx *tx,
> + bool enable_timestamping,
> + bool enable_onestep_sync)
> +{
> + if (enable_timestamping)
> + tx->ts_flags |= TX_TS_FLAG_TIMESTAMPING_ENABLED;
> + else
> + tx->ts_flags &= ~TX_TS_FLAG_TIMESTAMPING_ENABLED;
> + if (enable_onestep_sync)
> + tx->ts_flags |= TX_TS_FLAG_ONE_STEP_SYNC;
> + else
> + tx->ts_flags &= ~TX_TS_FLAG_ONE_STEP_SYNC;
> +}
> +
> static int lan743x_tx_frame_start(struct lan743x_tx *tx,
> unsigned char *first_buffer,
> unsigned int first_buffer_length,
> unsigned int frame_length,
> + bool time_stamp,
> bool check_sum)
> {
> /* called only from within lan743x_tx_xmit_frame.
> @@ -1362,6 +1400,8 @@ static int lan743x_tx_frame_start(struct lan743x_tx *tx,
> TX_DESC_DATA0_DTYPE_DATA_ |
> TX_DESC_DATA0_FS_ |
> TX_DESC_DATA0_FCS_;
> + if (time_stamp)
> + tx->frame_data0 |= TX_DESC_DATA0_TSE_;
>
> if (check_sum)
> tx->frame_data0 |= TX_DESC_DATA0_ICE_ |
> @@ -1475,6 +1515,7 @@ static int lan743x_tx_frame_add_fragment(struct lan743x_tx *tx,
>
> static void lan743x_tx_frame_end(struct lan743x_tx *tx,
> struct sk_buff *skb,
> + bool time_stamp,
> bool ignore_sync)
> {
> /* called only from within lan743x_tx_xmit_frame
> @@ -1492,6 +1533,8 @@ static void lan743x_tx_frame_end(struct lan743x_tx *tx,
> tx_descriptor = &tx->ring_cpu_ptr[tx->frame_tail];
> buffer_info = &tx->buffer_info[tx->frame_tail];
> buffer_info->skb = skb;
> + if (time_stamp)
> + buffer_info->flags |= TX_BUFFER_INFO_FLAG_TIMESTAMP_REQUESTED;
> if (ignore_sync)
> buffer_info->flags |= TX_BUFFER_INFO_FLAG_IGNORE_SYNC;
>
> @@ -1520,6 +1563,7 @@ static netdev_tx_t lan743x_tx_xmit_frame(struct lan743x_tx *tx,
> unsigned int frame_length = 0;
> unsigned int head_length = 0;
> unsigned long irq_flags = 0;
> + bool do_timestamp = false;
> bool ignore_sync = false;
> int nr_frags = 0;
> bool gso = false;
> @@ -1541,6 +1585,16 @@ static netdev_tx_t lan743x_tx_xmit_frame(struct lan743x_tx *tx,
> }
>
> /* space available, transmit skb */
> + if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) {
> + if (tx->ts_flags & TX_TS_FLAG_TIMESTAMPING_ENABLED) {
> + if (lan743x_ptp_request_tx_timestamp(tx->adapter)) {
Why not use && instead of three nested tests?
> + skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> + do_timestamp = true;
> + if (tx->ts_flags & TX_TS_FLAG_ONE_STEP_SYNC)
> + ignore_sync = true;
> + }
> + }
> + }
> head_length = skb_headlen(skb);
> frame_length = skb_pagelen(skb);
> nr_frags = skb_shinfo(skb)->nr_frags;
> @@ -1554,6 +1608,7 @@ static netdev_tx_t lan743x_tx_xmit_frame(struct lan743x_tx *tx,
> if (lan743x_tx_frame_start(tx,
> skb->data, head_length,
> start_frame_length,
> + do_timestamp,
> skb->ip_summed == CHECKSUM_PARTIAL)) {
> dev_kfree_skb(skb);
> goto unlock;
> @@ -1581,7 +1636,7 @@ static netdev_tx_t lan743x_tx_xmit_frame(struct lan743x_tx *tx,
> }
>
> finish:
> - lan743x_tx_frame_end(tx, skb, ignore_sync);
> + lan743x_tx_frame_end(tx, skb, do_timestamp, ignore_sync);
>
> unlock:
> spin_unlock_irqrestore(&tx->ring_lock, irq_flags);
> @@ -2410,6 +2465,8 @@ static int lan743x_netdev_close(struct net_device *netdev)
> for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++)
> lan743x_rx_close(&adapter->rx[index]);
>
> + lan743x_ptp_close(adapter);
> +
> lan743x_phy_close(adapter);
>
> lan743x_mac_close(adapter);
> @@ -2437,6 +2494,10 @@ static int lan743x_netdev_open(struct net_device *netdev)
> if (ret)
> goto close_mac;
>
> + ret = lan743x_ptp_open(adapter);
> + if (ret)
> + goto close_phy;
> +
> lan743x_rfe_open(adapter);
>
> for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++) {
> @@ -2456,6 +2517,9 @@ static int lan743x_netdev_open(struct net_device *netdev)
> if (adapter->rx[index].ring_cpu_ptr)
> lan743x_rx_close(&adapter->rx[index]);
> }
> + lan743x_ptp_close(adapter);
> +
> +close_phy:
> lan743x_phy_close(adapter);
>
> close_mac:
> @@ -2483,6 +2547,8 @@ static int lan743x_netdev_ioctl(struct net_device *netdev,
> {
> if (!netif_running(netdev))
> return -EINVAL;
> + if (cmd == SIOCSHWTSTAMP)
> + return lan743x_ptp_ioctl(netdev, ifr, cmd);
> return phy_mii_ioctl(netdev->phydev, ifr, cmd);
> }
>
> @@ -2607,6 +2673,11 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
> adapter->intr.irq = adapter->pdev->irq;
> lan743x_csr_write(adapter, INT_EN_CLR, 0xFFFFFFFF);
> mutex_init(&adapter->dp_lock);
> +
> + ret = lan743x_gpio_init(adapter);
> + if (ret)
> + return ret;
> +
> ret = lan743x_mac_init(adapter);
> if (ret)
> return ret;
> @@ -2615,6 +2686,10 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
> if (ret)
> return ret;
>
> + ret = lan743x_ptp_init(adapter);
> + if (ret)
> + return ret;
> +
> lan743x_rfe_update_mac_address(adapter);
>
> ret = lan743x_dmac_init(adapter);
...
> diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c
> new file mode 100644
> index 0000000..f14565b
> --- /dev/null
> +++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
> @@ -0,0 +1,1194 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/* Copyright (C) 2018 Microchip Technology Inc. */
> +
> +#include <linux/netdevice.h>
> +#include "lan743x_main.h"
> +
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/netdevice.h>
> +#include <linux/net_tstamp.h>
> +
> +#include "lan743x_ptp.h"
> +
> +/* GPIO */
> +#define LAN743X_NUMBER_OF_GPIO (12)
> +
> +int lan743x_gpio_init(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_gpio *gpio = &adapter->gpio;
> +
> + spin_lock_init(&gpio->gpio_lock);
> +
> + gpio->gpio_cfg0 = 0; /* set all direction to input, data = 0 */
> + gpio->gpio_cfg1 = 0x0FFF0000;/* disable all gpio, set to open drain */
> + gpio->gpio_cfg2 = 0;/* set all to 1588 low polarity level */
> + gpio->gpio_cfg3 = 0;/* disable all 1588 output */
> + lan743x_csr_write(adapter, GPIO_CFG0, gpio->gpio_cfg0);
> + lan743x_csr_write(adapter, GPIO_CFG1, gpio->gpio_cfg1);
> + lan743x_csr_write(adapter, GPIO_CFG2, gpio->gpio_cfg2);
> + lan743x_csr_write(adapter, GPIO_CFG3, gpio->gpio_cfg3);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_gpio_reserve_ptp_output(struct lan743x_adapter *adapter,
> + int bit, int ptp_channel)
> +{
> + struct lan743x_gpio *gpio = &adapter->gpio;
> + unsigned long irq_flags = 0;
> + int bit_mask = BIT(bit);
> + int ret = -EBUSY;
> +
> + spin_lock_irqsave(&gpio->gpio_lock, irq_flags);
> +
> + if (!(gpio->used_bits & bit_mask)) {
> + gpio->used_bits |= bit_mask;
> + gpio->output_bits |= bit_mask;
> + gpio->ptp_bits |= bit_mask;
> +
> + /* set as output, and zero initial value */
> + gpio->gpio_cfg0 |= GPIO_CFG0_GPIO_DIR_BIT_(bit);
> + gpio->gpio_cfg0 &= ~GPIO_CFG0_GPIO_DATA_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG0, gpio->gpio_cfg0);
> +
> + /* enable gpio , and set buffer type to push pull */
> + gpio->gpio_cfg1 &= ~GPIO_CFG1_GPIOEN_BIT_(bit);
> + gpio->gpio_cfg1 |= GPIO_CFG1_GPIOBUF_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG1, gpio->gpio_cfg1);
> +
> + /* set 1588 polarity to high */
> + gpio->gpio_cfg2 |= GPIO_CFG2_1588_POL_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG2, gpio->gpio_cfg2);
> +
> + if (!ptp_channel) {
> + /* use channel A */
> + gpio->gpio_cfg3 &= ~GPIO_CFG3_1588_CH_SEL_BIT_(bit);
> + } else {
> + /* use channel B */
> + gpio->gpio_cfg3 |= GPIO_CFG3_1588_CH_SEL_BIT_(bit);
> + }
> + gpio->gpio_cfg3 |= GPIO_CFG3_1588_OE_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG3, gpio->gpio_cfg3);
> +
> + ret = bit;
> + }
> + spin_unlock_irqrestore(&gpio->gpio_lock, irq_flags);
> + return ret;
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_gpio_release(struct lan743x_adapter *adapter, int bit)
> +{
> + struct lan743x_gpio *gpio = &adapter->gpio;
> + unsigned long irq_flags = 0;
> + int bit_mask = BIT(bit);
> +
> + spin_lock_irqsave(&gpio->gpio_lock, irq_flags);
> + if (gpio->used_bits & bit_mask) {
> + gpio->used_bits &= ~bit_mask;
> + if (gpio->output_bits & bit_mask) {
> + gpio->output_bits &= ~bit_mask;
> +
> + if (gpio->ptp_bits & bit_mask) {
> + gpio->ptp_bits &= ~bit_mask;
> + /* disable ptp output */
> + gpio->gpio_cfg3 &= ~GPIO_CFG3_1588_OE_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG3,
> + gpio->gpio_cfg3);
> + }
> + /* release gpio output */
> +
> + /* disable gpio */
> + gpio->gpio_cfg1 |= GPIO_CFG1_GPIOEN_BIT_(bit);
> + gpio->gpio_cfg1 &= ~GPIO_CFG1_GPIOBUF_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG1, gpio->gpio_cfg1);
> +
> + /* reset back to input */
> + gpio->gpio_cfg0 &= ~GPIO_CFG0_GPIO_DIR_BIT_(bit);
> + gpio->gpio_cfg0 &= ~GPIO_CFG0_GPIO_DATA_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG0, gpio->gpio_cfg0);
> + }
> + }
> + spin_unlock_irqrestore(&gpio->gpio_lock, irq_flags);
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +/* PTP */
> +#define LAN743X_PTP_MAX_FREQ_ADJ_IN_PPB (31249999)
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptp_reserve_event_ch(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_release_event_ch(struct lan743x_adapter *adapter,
> + int event_channel);
> +#endif
> +
> +static bool lan743x_ptp_is_enabled(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_enable(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_disable(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_reset(struct lan743x_adapter *adapter);
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_clock_get(struct lan743x_adapter *adapter,
> + u32 *seconds, u32 *nano_seconds,
> + u32 *sub_nano_seconds);
> +static int lan743x_ptp_enable_pps(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_disable_pps(struct lan743x_adapter *adapter);
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_clock_step(struct lan743x_adapter *adapter,
> + s64 time_step_ns);
> +#endif /* CONFIG_PTP_1588_CLOCK */
The constant ifdef CONFIG_PTP_1588_CLOCK is poor style and
unnecessary. Just group this code together under one ifdef, or better
yet put it into its own file.
> +static void lan743x_ptp_clock_set(struct lan743x_adapter *adapter,
> + u32 seconds, u32 nano_seconds,
> + u32 sub_nano_seconds);
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_adjfreq(struct ptp_clock_info *ptpci, s32 delta_ppb)
Please implement adjfine().
* @adjfine: Adjusts the frequency of the hardware clock.
* parameter scaled_ppm: Desired frequency offset from
* nominal frequency in parts per million, but with a
* 16 bit binary fractional field.
*
* @adjfreq: Adjusts the frequency of the hardware clock.
* This method is deprecated. New drivers should implement
* the @adjfine method instead.
* parameter delta: Desired frequency offset from nominal frequency
* in parts per billion
> +{
> + struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> + ptp_clock_info);
> + struct lan743x_adapter *adapter = container_of(ptp,
> + struct lan743x_adapter,
> + ptp);
Coding style:
struct lan743x_adapter *adapter =
container_of(ptp, struct lan743x_adapter, ptp);
> + u32 lan743x_rate_adj = 0;
> + bool positive = true;
> + u32 u32_delta = 0;
> + u64 u64_delta = 0;
> +
> + if ((delta_ppb < (-LAN743X_PTP_MAX_FREQ_ADJ_IN_PPB)) ||
> + delta_ppb > LAN743X_PTP_MAX_FREQ_ADJ_IN_PPB) {
> + return -EINVAL;
> + }
> + if (delta_ppb > 0) {
> + u32_delta = (u32)delta_ppb;
> + positive = true;
> + } else {
> + u32_delta = (u32)(-delta_ppb);
> + positive = false;
> + }
> + u64_delta = (((u64)u32_delta) * 0x800000000ULL);
> + lan743x_rate_adj = (u32)(u64_delta / 1000000000);
You need to use the div_u64() macro here.
> +
> + if (positive)
> + lan743x_rate_adj |= PTP_CLOCK_RATE_ADJ_DIR_;
> +
> + lan743x_csr_write(adapter, PTP_CLOCK_RATE_ADJ,
> + lan743x_rate_adj);
> +
> + netif_info(adapter, drv, adapter->netdev,
> + "adjfreq, delta_ppb = %d, lan743x_rate_adj = 0x%08X\n",
> + delta_ppb, lan743x_rate_adj);
This definitely should be at the debug level, or just delete it altogether.
> + return 0;
> +}
> +#endif /*CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_adjtime(struct ptp_clock_info *ptpci, s64 delta)
> +{
> + struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> + ptp_clock_info);
> + struct lan743x_adapter *adapter = container_of(ptp,
> + struct lan743x_adapter,
> + ptp);
Coding style.
> + bool enable_pps = false;
> +
> + if (ptp->pps_event_ch >= 0) {
> + lan743x_ptp_disable_pps(adapter);
> + enable_pps = true;
> + }
> +
> + lan743x_ptp_clock_step(adapter, delta);
> + netif_info(adapter, drv, adapter->netdev,
> + "adjtime, delta = %lld\n", delta);
Again, debug or delete.
> +
> + if (enable_pps)
> + lan743x_ptp_enable_pps(adapter);
> +
> + return 0;
> +}
> +#endif /*CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_gettime64(struct ptp_clock_info *ptpci,
> + struct timespec64 *ts)
> +{
> + struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> + ptp_clock_info);
> + struct lan743x_adapter *adapter = container_of(ptp,
> + struct lan743x_adapter,
> + ptp);
Style.
> +
> + if (ts) {
> + u32 seconds = 0;
> + u32 nano_seconds = 0;
Please declare stack variables at the top of the function.
> +
> + lan743x_ptp_clock_get(adapter, &seconds, &nano_seconds, NULL);
> + ts->tv_sec = seconds;
> + ts->tv_nsec = nano_seconds;
> + netif_info(adapter, drv, adapter->netdev,
> + "gettime = %u.%09u\n", seconds, nano_seconds);
Debug/delete
> + } else {
> + netif_warn(adapter, drv, adapter->netdev, "ts == NULL\n");
> + return -EINVAL;
No need to test for 'ts'. The caller must supply a valid pointer.
> + }
> + return 0;
> +}
> +#endif /*CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_settime64(struct ptp_clock_info *ptpci,
> + const struct timespec64 *ts)
> +{
> + struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> + ptp_clock_info);
> + struct lan743x_adapter *adapter = container_of(ptp,
> + struct lan743x_adapter,
> + ptp);
> + bool enable_pps = false;
> +
> + if (ptp->pps_event_ch >= 0) {
> + lan743x_ptp_disable_pps(adapter);
> + enable_pps = true;
> + }
> +
> + if (ts) {
> + u32 seconds = 0;
> + u32 nano_seconds = 0;
> +
> + if (ts->tv_sec > 0xFFFFFFFFLL ||
> + ts->tv_sec < 0) {
Actually seconds will exceed four bytes sooner than you think. If
your HW has a restriction, then simply keep the seconds offset in SW
in the driver, adding it in where needed (like when reading the clock
or providing time stamps).
> + netif_warn(adapter, drv, adapter->netdev,
> + "ts->tv_sec out of range, %lld\n",
> + ts->tv_sec);
> + return -EINVAL;
> + }
> + if (ts->tv_nsec >= 1000000000L ||
> + ts->tv_nsec < 0) {
> + netif_warn(adapter, drv, adapter->netdev,
> + "ts->tv_nsec out of range, %ld\n",
> + ts->tv_nsec);
> + return -EINVAL;
> + }
> + seconds = ts->tv_sec;
> + nano_seconds = ts->tv_nsec;
> + netif_info(adapter, drv, adapter->netdev,
> + "settime = %u.%09u\n", seconds, nano_seconds);
> + lan743x_ptp_clock_set(adapter, seconds, nano_seconds, 0);
> + } else {
> + netif_warn(adapter, drv, adapter->netdev, "ts == NULL\n");
> + return -EINVAL;
> + }
> +
> + if (enable_pps)
> + lan743x_ptp_enable_pps(adapter);
> +
> + return 0;
> +}
> +#endif /*CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptp_enable_pps(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + u32 current_seconds = 0;
> + u32 target_seconds = 0;
> + u32 general_config = 0;
> + int result = -ENODEV;
> + int pps_bit = 0;
So this function is really *not* implementing the PTP_CLK_REQ_PPS
feature but rather the PTP_CLK_REQ_PEROUT with a period of once per
second.
PTP_CLK_REQ_PPS means placing a PPS event into the kernel's "hardpps"
subsystem by calling ptp_clock_event().
I'm sorry this isn't really documented. I should fix that.
If you HW can output arbitrary signals, then you should implement
PTP_CLK_REQ_PEROUT. In any case, you shouldn't advertise the
ptp_clock_info.pps capability.
> + if (ptp->pps_event_ch >= 0) {
> + result = 0;
> + goto done;
> + }
> +
> + ptp->pps_event_ch = lan743x_ptp_reserve_event_ch(adapter);
> + if (ptp->pps_event_ch < 0) {
> + netif_warn(adapter, drv, adapter->netdev,
> + "Failed to reserve event channel for PPS\n");
> + goto done;
> + }
> +
> + switch(adapter->csr.id_rev & ID_REV_ID_MASK_) {
> + case ID_REV_ID_LAN7430_:
> + pps_bit = 2;/* GPIO 2 is preferred on EVB LAN7430 */
> + break;
> + case ID_REV_ID_LAN7431_:
> + pps_bit = 4;/* GPIO 4 is preferred on EVB LAN7431 */
> + break;
> + }
> +
> + ptp->pps_gpio_bit = lan743x_gpio_reserve_ptp_output(adapter, pps_bit,
> + ptp->pps_event_ch);
> +
> + if (ptp->pps_gpio_bit < 0) {
> + netif_warn(adapter, drv, adapter->netdev,
> + "Failed to reserve gpio 0 for PPS\n");
> + goto done;
> + }
> +
> + lan743x_ptp_clock_get(adapter, ¤t_seconds, NULL, NULL);
> +
> + /* set the first target ahead by 2 seconds
> + * to make sure its not missed
> + */
> + target_seconds = current_seconds + 2;
> +
> + /* set the new target */
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_SEC_X(ptp->pps_event_ch),
> + 0xFFFF0000);
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_NS_X(ptp->pps_event_ch), 0);
> +
> + general_config = lan743x_csr_read(adapter, PTP_GENERAL_CONFIG);
> +
> + general_config &= ~(PTP_GENERAL_CONFIG_CLOCK_EVENT_X_MASK_
> + (ptp->pps_event_ch));
> + general_config |= PTP_GENERAL_CONFIG_CLOCK_EVENT_X_SET_
> + (ptp->pps_event_ch,
> + PTP_GENERAL_CONFIG_CLOCK_EVENT_100US_);
> + general_config &= ~PTP_GENERAL_CONFIG_RELOAD_ADD_X_
> + (ptp->pps_event_ch);
> + lan743x_csr_write(adapter, PTP_GENERAL_CONFIG, general_config);
> +
> + /* set the reload to one second steps */
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_RELOAD_SEC_X(ptp->pps_event_ch),
> + 1);
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_RELOAD_NS_X(ptp->pps_event_ch),
> + 0);
> +
> + /* set the new target */
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_SEC_X(ptp->pps_event_ch),
> + target_seconds);
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_NS_X(ptp->pps_event_ch),
> + 0);
> + return 0;
> +
> +done:
> + if (ptp->pps_gpio_bit >= 0) {
> + lan743x_gpio_release(adapter, ptp->pps_gpio_bit);
> + ptp->pps_gpio_bit = -1;
> + }
> + if (ptp->pps_event_ch >= 0) {
> + lan743x_ptp_release_event_ch(adapter,
> + ptp->pps_event_ch);
> + ptp->pps_event_ch = -1;
> + }
> + return result;
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_disable_pps(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + if (ptp->pps_gpio_bit >= 0) {
> + lan743x_gpio_release(adapter, ptp->pps_gpio_bit);
> + ptp->pps_gpio_bit = -1;
> + }
> +
> + if (ptp->pps_event_ch >= 0) {
> + u32 general_config = 0;
> +
> + /* set target to far in the future, effectively disabling it */
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_SEC_X(ptp->pps_event_ch),
> + 0xFFFF0000);
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_NS_X(ptp->pps_event_ch), 0);
> +
> + general_config = lan743x_csr_read(adapter, PTP_GENERAL_CONFIG);
> + general_config |= PTP_GENERAL_CONFIG_RELOAD_ADD_X_
> + (ptp->pps_event_ch);
> + lan743x_csr_write(adapter, PTP_GENERAL_CONFIG, general_config);
> + lan743x_ptp_release_event_ch(adapter, ptp->pps_event_ch);
> + ptp->pps_event_ch = -1;
> + }
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_enable(struct ptp_clock_info *ptpci,
> + struct ptp_clock_request *request, int on)
> +{
> + struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> + ptp_clock_info);
> + struct lan743x_adapter *adapter = container_of(ptp,
> + struct lan743x_adapter,
> + ptp);
> +
> + if (request) {
> + switch (request->type) {
> + case PTP_CLK_REQ_EXTTS:
> + return -EINVAL;
> + case PTP_CLK_REQ_PEROUT:
> + return -EINVAL;
> + case PTP_CLK_REQ_PPS:
> + if (on) {
> + if (lan743x_ptp_enable_pps(adapter) >= 0)
> + netif_info(adapter, drv,
> + adapter->netdev,
> + "PPS is ON\n");
> + else
> + netif_warn(adapter, drv,
> + adapter->netdev,
> + "Error starting PPS\n");
> + } else {
> + lan743x_ptp_disable_pps(adapter);
> + netif_info(adapter, drv, adapter->netdev,
> + "PPS is OFF\n");
> + }
> + break;
> + default:
> + netif_err(adapter, drv, adapter->netdev,
> + "request->type == %d, Unknown\n",
> + request->type);
> + break;
> + }
> + } else {
> + netif_err(adapter, drv, adapter->netdev, "request == NULL\n");
> + }
> + return 0;
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +void lan743x_ptp_isr(void *context)
> +{
> + struct lan743x_adapter *adapter = (struct lan743x_adapter *)context;
> + struct lan743x_ptp *ptp = NULL;
> + int enable_flag = 1;
> + u32 ptp_int_sts = 0;
> +
> + ptp = &adapter->ptp;
> +
> + lan743x_csr_write(adapter, INT_EN_CLR, INT_BIT_1588_);
> +
> + ptp_int_sts = lan743x_csr_read(adapter, PTP_INT_STS);
> + ptp_int_sts &= lan743x_csr_read(adapter, PTP_INT_EN_SET);
> +
> + if (ptp_int_sts & PTP_INT_BIT_TX_TS_) {
> + tasklet_schedule(&ptp->ptp_isr_bottom_half);
Please no new tasklets. Instead use a work queue. If you need lower
latency, consider using ptp_schedule_worker().
> + enable_flag = 0;/* tasklet will re-enable later */
> + }
> + if (ptp_int_sts & PTP_INT_BIT_TX_SWTS_ERR_) {
> + netif_err(adapter, drv, adapter->netdev,
> + "PTP TX Software Timestamp Error\n");
> + /* clear int status bit */
> + lan743x_csr_write(adapter, PTP_INT_STS,
> + PTP_INT_BIT_TX_SWTS_ERR_);
> + }
> + if (ptp_int_sts & PTP_INT_BIT_TIMER_B_) {
> + netif_info(adapter, drv, adapter->netdev,
> + "PTP TIMER B Interrupt\n");
Don't print like this from an ISR. Or is this an error, since you
don't enable this bit?
> + /* clear int status bit */
> + lan743x_csr_write(adapter, PTP_INT_STS,
> + PTP_INT_BIT_TIMER_B_);
> + }
> + if (ptp_int_sts & PTP_INT_BIT_TIMER_A_) {
> + netif_info(adapter, drv, adapter->netdev,
> + "PTP TIMER A Interrupt\n");
> + /* clear int status bit */
> + lan743x_csr_write(adapter, PTP_INT_STS,
> + PTP_INT_BIT_TIMER_A_);
> + }
> +
> + if (enable_flag) {
> + /* re-enable isr */
> + lan743x_csr_write(adapter, INT_EN_SET, INT_BIT_1588_);
> + }
> +}
> +
> +static void lan743x_ptp_tx_ts_complete(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + int i;
> + int c;
Put same types on one line:
int c, i;
> +
> + spin_lock_bh(&ptp->tx_ts_lock);
> + c = ptp->tx_ts_skb_queue_size;
> +
> + if (c > ptp->tx_ts_queue_size)
> + c = ptp->tx_ts_queue_size;
> + if (c <= 0)
> + goto done;
> +
> + for (i = 0; i < c; i++) {
> + bool ignore_sync = ((ptp->tx_ts_ignore_sync_queue &
> + BIT(i)) != 0);
> + struct sk_buff *skb = ptp->tx_ts_skb_queue[i];
> + u32 nseconds = ptp->tx_ts_nseconds_queue[i];
> + u32 seconds = ptp->tx_ts_seconds_queue[i];
> + u32 header = ptp->tx_ts_header_queue[i];
> + struct skb_shared_hwtstamps tstamps;
Locals to top of function please.
> + memset(&tstamps, 0, sizeof(tstamps));
> + tstamps.hwtstamp = ktime_set(seconds, nseconds);
> + if (!ignore_sync ||
> + ((header & PTP_TX_MSG_HEADER_MSG_TYPE_) !=
> + PTP_TX_MSG_HEADER_MSG_TYPE_SYNC_))
> + skb_tstamp_tx(skb, &tstamps);
> +
> + dev_kfree_skb(skb);
> +
> + ptp->tx_ts_skb_queue[i] = NULL;
> + ptp->tx_ts_seconds_queue[i] = 0;
> + ptp->tx_ts_nseconds_queue[i] = 0;
> + ptp->tx_ts_header_queue[i] = 0;
> + }
> +
> + /* shift queue */
> + ptp->tx_ts_ignore_sync_queue >>= c;
> + for (i = c; i < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS; i++) {
> + ptp->tx_ts_skb_queue[i - c] = ptp->tx_ts_skb_queue[i];
> + ptp->tx_ts_seconds_queue[i - c] = ptp->tx_ts_seconds_queue[i];
> + ptp->tx_ts_nseconds_queue[i - c] = ptp->tx_ts_nseconds_queue[i];
> + ptp->tx_ts_header_queue[i - c] = ptp->tx_ts_header_queue[i];
> +
> + ptp->tx_ts_skb_queue[i] = NULL;
> + ptp->tx_ts_seconds_queue[i] = 0;
> + ptp->tx_ts_nseconds_queue[i] = 0;
> + ptp->tx_ts_header_queue[i] = 0;
> + }
> + ptp->tx_ts_skb_queue_size -= c;
> + ptp->tx_ts_queue_size -= c;
> +done:
> + ptp->pending_tx_timestamps -= c;
> + spin_unlock_bh(&ptp->tx_ts_lock);
> +}
> +
> +static void lan743x_ptp_tx_ts_enqueue_skb(struct lan743x_adapter *adapter,
> + struct sk_buff *skb, bool ignore_sync)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + spin_lock_bh(&ptp->tx_ts_lock);
> + if (ptp->tx_ts_skb_queue_size < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS) {
> + ptp->tx_ts_skb_queue[ptp->tx_ts_skb_queue_size] = skb;
> + if (ignore_sync)
> + ptp->tx_ts_ignore_sync_queue |=
> + BIT(ptp->tx_ts_skb_queue_size);
> + ptp->tx_ts_skb_queue_size++;
> + } else {
> + /* this should never happen, so long as the tx channel
> + * calls and honors the result from
> + * lan743x_ptp_request_tx_timestamp
> + */
> + netif_err(adapter, drv, adapter->netdev,
> + "tx ts skb queue overflow\n");
> + dev_kfree_skb(skb);
> + }
> + spin_unlock_bh(&ptp->tx_ts_lock);
> +}
> +
> +static void lan743x_ptp_tx_ts_enqueue_ts(struct lan743x_adapter *adapter,
> + u32 seconds, u32 nano_seconds,
> + u32 header)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + spin_lock_bh(&ptp->tx_ts_lock);
> + if (ptp->tx_ts_queue_size < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS) {
> + ptp->tx_ts_seconds_queue[ptp->tx_ts_queue_size] = seconds;
> + ptp->tx_ts_nseconds_queue[ptp->tx_ts_queue_size] = nano_seconds;
> + ptp->tx_ts_header_queue[ptp->tx_ts_queue_size] = header;
> + ptp->tx_ts_queue_size++;
> + } else {
> + netif_err(adapter, drv, adapter->netdev,
> + "tx ts queue overflow\n");
> + }
> + spin_unlock_bh(&ptp->tx_ts_lock);
> +}
> +
> +static void lan743x_ptp_isr_bottom_half(unsigned long param)
> +{
> + struct lan743x_adapter *adapter = (struct lan743x_adapter *)param;
> + bool new_timestamp_available = false;
> +
> + while (lan743x_csr_read(adapter, PTP_INT_STS) & PTP_INT_BIT_TX_TS_) {
As a sanity check, you should break this loop using a counter.
> + u32 cap_info = lan743x_csr_read(adapter, PTP_CAP_INFO);
> +
> + if (PTP_CAP_INFO_TX_TS_CNT_GET_(cap_info) > 0) {
> + u32 seconds = lan743x_csr_read(adapter,
> + PTP_TX_EGRESS_SEC);
> + u32 nsec = lan743x_csr_read(adapter, PTP_TX_EGRESS_NS);
> + u32 cause = (nsec &
> + PTP_TX_EGRESS_NS_CAPTURE_CAUSE_MASK_);
> + u32 header = lan743x_csr_read(adapter,
> + PTP_TX_MSG_HEADER);
> +
> + if (cause == PTP_TX_EGRESS_NS_CAPTURE_CAUSE_SW_) {
> + nsec &= PTP_TX_EGRESS_NS_TS_NS_MASK_;
> + lan743x_ptp_tx_ts_enqueue_ts(adapter,
> + seconds, nsec,
> + header);
> + new_timestamp_available = true;
> + } else if (cause ==
> + PTP_TX_EGRESS_NS_CAPTURE_CAUSE_AUTO_) {
> + netif_err(adapter, drv, adapter->netdev,
> + "Auto capture cause not supported\n");
> + } else {
> + netif_warn(adapter, drv, adapter->netdev,
> + "unknown tx timestamp capture cause\n");
> + }
> + } else {
> + netif_warn(adapter, drv, adapter->netdev,
> + "TX TS INT but no TX TS CNT\n");
> + }
> + lan743x_csr_write(adapter, PTP_INT_STS, PTP_INT_BIT_TX_TS_);
> + }
> +
> + if (new_timestamp_available)
> + lan743x_ptp_tx_ts_complete(adapter);
> +
> + lan743x_csr_write(adapter, INT_EN_SET, INT_BIT_1588_);
> +}
> +
> +static void lan743x_ptp_sync_to_system_clock(struct lan743x_adapter *adapter)
> +{
> + struct timeval tv;
> +
> + memset(&tv, 0, sizeof(tv));
> + do_gettimeofday(&tv);
Use the TAI clock instead.
> + lan743x_ptp_clock_set(adapter, tv.tv_sec, tv.tv_usec * 1000, 0);
> +}
> +
> +void lan743x_ptp_update_latency(struct lan743x_adapter *adapter,
> + u32 link_speed)
> +{
> + switch (link_speed) {
> + case 10:
> + lan743x_csr_write(adapter, PTP_LATENCY,
> + PTP_LATENCY_TX_SET_(0) |
> + PTP_LATENCY_RX_SET_(0));
> + break;
> + case 100:
> + lan743x_csr_write(adapter, PTP_LATENCY,
> + PTP_LATENCY_TX_SET_(181) |
> + PTP_LATENCY_RX_SET_(594));
> + break;
> + case 1000:
> + lan743x_csr_write(adapter, PTP_LATENCY,
> + PTP_LATENCY_TX_SET_(30) |
> + PTP_LATENCY_RX_SET_(525));
> + break;
> + }
> +}
> +
> +int lan743x_ptp_init(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_init(&ptp->command_lock);
> + spin_lock_init(&ptp->tx_ts_lock);
> + tasklet_init(&ptp->ptp_isr_bottom_half,
> + lan743x_ptp_isr_bottom_half, (unsigned long)adapter);
> + tasklet_disable(&ptp->ptp_isr_bottom_half);
> + ptp->used_event_ch = 0;
> + ptp->pps_event_ch = -1;
> + ptp->pps_gpio_bit = -1;
> + return 0;
> +}
> +
> +int lan743x_ptp_open(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + int ret = -ENODEV;
> + u32 temp;
> +
> + lan743x_ptp_reset(adapter);
> + lan743x_ptp_sync_to_system_clock(adapter);
> + temp = lan743x_csr_read(adapter, PTP_TX_MOD2);
> + temp |= PTP_TX_MOD2_TX_PTP_CLR_UDPV4_CHKSUM_;
> + lan743x_csr_write(adapter, PTP_TX_MOD2, temp);
> + lan743x_ptp_enable(adapter);
> + tasklet_enable(&ptp->ptp_isr_bottom_half);
> + lan743x_csr_write(adapter, INT_EN_SET, INT_BIT_1588_);
> + lan743x_csr_write(adapter, PTP_INT_EN_SET,
> + PTP_INT_BIT_TX_SWTS_ERR_ | PTP_INT_BIT_TX_TS_);
> + ptp->flags |= PTP_FLAG_ISR_ENABLED;
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> + snprintf(ptp->pin_config[0].name, 32, "lan743x_ptp_pin_0");
> + ptp->pin_config[0].index = 0;
> + ptp->pin_config[0].func = PTP_PF_PEROUT;
> + ptp->pin_config[0].chan = 0;
> +
> + ptp->ptp_clock_info.owner = THIS_MODULE;
> + snprintf(ptp->ptp_clock_info.name, 16, "%pm",
> + adapter->netdev->dev_addr);
> + ptp->ptp_clock_info.max_adj = LAN743X_PTP_MAX_FREQ_ADJ_IN_PPB;
> + ptp->ptp_clock_info.n_alarm = 0;
> + ptp->ptp_clock_info.n_ext_ts = 0;
> + ptp->ptp_clock_info.n_per_out = 0;
> + ptp->ptp_clock_info.n_pins = 0;
> + ptp->ptp_clock_info.pps = 1;
> + ptp->ptp_clock_info.pin_config = NULL;
> + ptp->ptp_clock_info.adjfreq = lan743x_ptpci_adjfreq;
> + ptp->ptp_clock_info.adjtime = lan743x_ptpci_adjtime;
> + ptp->ptp_clock_info.gettime64 = lan743x_ptpci_gettime64;
> + ptp->ptp_clock_info.getcrosststamp = NULL;
> + ptp->ptp_clock_info.settime64 = lan743x_ptpci_settime64;
> + ptp->ptp_clock_info.enable = lan743x_ptpci_enable;
> + ptp->ptp_clock_info.verify = NULL;
> +
> + ptp->ptp_clock = ptp_clock_register(&ptp->ptp_clock_info,
> + &adapter->pdev->dev);
> +
> + if (IS_ERR(ptp->ptp_clock)) {
> + netif_err(adapter, ifup, adapter->netdev,
> + "ptp_clock_register failed\n");
> + goto done;
> + }
> + ptp->flags |= PTP_FLAG_PTP_CLOCK_REGISTERED;
> + netif_info(adapter, ifup, adapter->netdev,
> + "successfully registered ptp clock\n");
> +#endif
> +
> + return 0;
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +done:
> + lan743x_ptp_close(adapter);
> + return ret;
> +#endif
> +}
> +
> +void lan743x_ptp_close(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + int index;
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> + if (ptp->flags & PTP_FLAG_PTP_CLOCK_REGISTERED) {
> + ptp_clock_unregister(ptp->ptp_clock);
> + ptp->ptp_clock = NULL;
> + ptp->flags &= ~PTP_FLAG_PTP_CLOCK_REGISTERED;
> + netif_info(adapter, drv, adapter->netdev,
> + "ptp clock unregister\n");
> + }
> +#endif
> +
> + if (ptp->flags & PTP_FLAG_ISR_ENABLED) {
> + lan743x_csr_write(adapter, PTP_INT_EN_CLR,
> + PTP_INT_BIT_TX_SWTS_ERR_ |
> + PTP_INT_BIT_TX_TS_);
> + lan743x_csr_write(adapter, INT_EN_CLR, INT_BIT_1588_);
> + tasklet_disable(&ptp->ptp_isr_bottom_half);
> + ptp->flags &= ~PTP_FLAG_ISR_ENABLED;
> + }
> +
> + /* clean up pending timestamp requests */
> + lan743x_ptp_tx_ts_complete(adapter);
> + spin_lock_bh(&ptp->tx_ts_lock);
> + for (index = 0;
> + index < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS;
> + index++) {
> + struct sk_buff *skb = ptp->tx_ts_skb_queue[index];
> +
> + if (skb)
> + dev_kfree_skb(skb);
> + ptp->tx_ts_skb_queue[index] = NULL;
> + ptp->tx_ts_seconds_queue[index] = 0;
> + ptp->tx_ts_nseconds_queue[index] = 0;
> + }
> + ptp->tx_ts_skb_queue_size = 0;
> + ptp->tx_ts_queue_size = 0;
> + ptp->pending_tx_timestamps = 0;
> + spin_unlock_bh(&ptp->tx_ts_lock);
> +
> + lan743x_ptp_disable(adapter);
> +}
> +
> +void lan743x_ptp_set_sync_ts_insert(struct lan743x_adapter *adapter,
> + bool ts_insert_enable)
> +{
> + u32 ptp_tx_mod = lan743x_csr_read(adapter, PTP_TX_MOD);
> +
> + if (ts_insert_enable)
> + ptp_tx_mod |= PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_;
> + else
> + ptp_tx_mod &= ~PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_;
> +
> + lan743x_csr_write(adapter, PTP_TX_MOD, ptp_tx_mod);
> +}
> +
> +static bool lan743x_ptp_is_enabled(struct lan743x_adapter *adapter)
> +{
> + if (lan743x_csr_read(adapter, PTP_CMD_CTL) & PTP_CMD_CTL_PTP_ENABLE_)
> + return true;
> + return false;
> +}
> +
> +static void lan743x_ptp_wait_till_cmd_done(struct lan743x_adapter *adapter,
> + u32 bit_mask)
> +{
> + int timeout = 1000;
> + u32 data = 0;
> +
> + while (timeout &&
> + (data = (lan743x_csr_read(adapter, PTP_CMD_CTL) &
> + bit_mask))) {
> + usleep_range(1000, 20000);
> + timeout--;
> + }
> + if (data) {
> + netif_err(adapter, drv, adapter->netdev,
> + "timeout waiting for cmd to be done, cmd = 0x%08X\n",
> + bit_mask);
> + }
> +}
> +
> +static void lan743x_ptp_enable(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_lock(&ptp->command_lock);
> +
> + if (lan743x_ptp_is_enabled(adapter)) {
> + netif_warn(adapter, drv, adapter->netdev,
> + "PTP already enabled\n");
> + goto done;
> + }
> + lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_ENABLE_);
> +done:
> + mutex_unlock(&ptp->command_lock);
> +}
> +
> +static void lan743x_ptp_disable(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_lock(&ptp->command_lock);
> + if (!lan743x_ptp_is_enabled(adapter)) {
> + netif_warn(adapter, drv, adapter->netdev,
> + "PTP already disabled\n");
> + goto done;
> + }
> + lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_DISABLE_);
> + lan743x_ptp_wait_till_cmd_done(adapter, PTP_CMD_CTL_PTP_ENABLE_);
> +done:
> + mutex_unlock(&ptp->command_lock);
> +}
> +
> +static void lan743x_ptp_reset(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_lock(&ptp->command_lock);
> +
> + if (lan743x_ptp_is_enabled(adapter)) {
> + netif_err(adapter, drv, adapter->netdev,
> + "Attempting reset while enabled\n");
> + goto done;
> + }
> +
> + lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_RESET_);
> + lan743x_ptp_wait_till_cmd_done(adapter, PTP_CMD_CTL_PTP_RESET_);
> +done:
> + mutex_unlock(&ptp->command_lock);
> +}
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptp_reserve_event_ch(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + int result = -ENODEV;
> + int index = 0;
> +
> + mutex_lock(&ptp->command_lock);
> + for (index = 0; index < LAN743X_PTP_NUMBER_OF_EVENT_CHANNELS; index++) {
> + if (!(test_bit(index, &ptp->used_event_ch))) {
> + ptp->used_event_ch |= BIT(index);
> + result = index;
> + break;
> + }
> + }
> + mutex_unlock(&ptp->command_lock);
> + return result;
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_release_event_ch(struct lan743x_adapter *adapter,
> + int event_channel)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_lock(&ptp->command_lock);
> + if (test_bit(event_channel, &ptp->used_event_ch)) {
> + ptp->used_event_ch &= ~BIT(event_channel);
> + } else {
> + netif_warn(adapter, drv, adapter->netdev,
> + "attempted release on a not used event_channel = %d\n",
> + event_channel);
> + }
> + mutex_unlock(&ptp->command_lock);
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_clock_get(struct lan743x_adapter *adapter,
> + u32 *seconds, u32 *nano_seconds,
> + u32 *sub_nano_seconds)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_lock(&ptp->command_lock);
> +
> + lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_CLOCK_READ_);
> + lan743x_ptp_wait_till_cmd_done(adapter, PTP_CMD_CTL_PTP_CLOCK_READ_);
> +
> + if (seconds)
> + (*seconds) = lan743x_csr_read(adapter, PTP_CLOCK_SEC);
> +
> + if (nano_seconds)
> + (*nano_seconds) = lan743x_csr_read(adapter, PTP_CLOCK_NS);
> +
> + if (sub_nano_seconds)
> + (*sub_nano_seconds) =
> + lan743x_csr_read(adapter, PTP_CLOCK_SUBNS);
> +
> + mutex_unlock(&ptp->command_lock);
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +static void lan743x_ptp_clock_set(struct lan743x_adapter *adapter,
> + u32 seconds, u32 nano_seconds,
> + u32 sub_nano_seconds)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_lock(&ptp->command_lock);
> +
> + lan743x_csr_write(adapter, PTP_CLOCK_SEC, seconds);
> + lan743x_csr_write(adapter, PTP_CLOCK_NS, nano_seconds);
> + lan743x_csr_write(adapter, PTP_CLOCK_SUBNS, sub_nano_seconds);
> +
> + lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_CLOCK_LOAD_);
> + lan743x_ptp_wait_till_cmd_done(adapter, PTP_CMD_CTL_PTP_CLOCK_LOAD_);
> + mutex_unlock(&ptp->command_lock);
> +}
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_clock_step(struct lan743x_adapter *adapter,
> + s64 time_step_ns)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + u64 abs_time_step_ns = 0;
> + u32 nano_seconds = 0;
> + s32 seconds = 0;
> +
> + if (time_step_ns > 15000000000LL) {
> + /* convert to clock set */
> + u32 nano_seconds = 0;
> + u32 seconds = 0;
> +
> + lan743x_ptp_clock_get(adapter, &seconds, &nano_seconds, NULL);
> + seconds += (time_step_ns / 1000000000LL);
Use the macro.
> + nano_seconds += (time_step_ns % 1000000000LL);
Actually, use div_u64_rem() to avoid the % operator.
> + if (nano_seconds >= 1000000000) {
How can this test be true?
> + seconds++;
> + nano_seconds -= 1000000000;
> + }
> + lan743x_ptp_clock_set(adapter, seconds, nano_seconds, 0);
> + return;
> + } else if (time_step_ns < -15000000000LL) {
> + /* convert to clock set */
> + u32 nano_seconds_step = 0;
> + u32 nano_seconds = 0;
> + u32 seconds = 0;
Ugh. Now you have these defined twice. Move them to the top, please.
> + time_step_ns = -time_step_ns;
> +
> + lan743x_ptp_clock_get(adapter, &seconds, &nano_seconds, NULL);
> + seconds -= (time_step_ns / 1000000000LL);
> + nano_seconds_step = (time_step_ns % 1000000000LL);
Use division macro for 64 bit.
> + if (nano_seconds < nano_seconds_step) {
> + seconds--;
> + nano_seconds += 1000000000;
> + }
> + nano_seconds -= nano_seconds_step;
> + lan743x_ptp_clock_set(adapter, seconds, nano_seconds, 0);
> + return;
> + }
> +
> + /* do clock step */
> +
> + if (time_step_ns >= 0) {
> + abs_time_step_ns = (u64)(time_step_ns);
> + seconds = (s32)(abs_time_step_ns / 1000000000);
> + nano_seconds = (u32)(abs_time_step_ns % 1000000000);
> + } else {
> + abs_time_step_ns = (u64)(-time_step_ns);
> + seconds = -((s32)(abs_time_step_ns / 1000000000));
> + nano_seconds = (u32)(abs_time_step_ns % 1000000000);
> + if (nano_seconds > 0) {
> + /* subtracting nano seconds is not allowed
> + * convert to subtracting from seconds,
> + * and adding to nanoseconds
> + */
> + seconds--;
> + nano_seconds = (1000000000 - nano_seconds);
> + }
> + }
> +
> + if (nano_seconds > 0) {
> + /* add 8 ns to cover the likely normal increment */
> + nano_seconds += 8;
> + }
> +
> + if (nano_seconds >= 1000000000) {
> + /* carry into seconds */
> + seconds++;
> + nano_seconds -= 1000000000;
> + }
> +
> + while (seconds) {
> + mutex_lock(&ptp->command_lock);
> + if (seconds > 0) {
> + u32 adjustment_value = (u32)seconds;
> +
> + if (adjustment_value > 0xF)
> + adjustment_value = 0xF;
> + lan743x_csr_write(adapter, PTP_CLOCK_STEP_ADJ,
> + PTP_CLOCK_STEP_ADJ_DIR_ |
> + adjustment_value);
> + seconds -= ((s32)adjustment_value);
> + } else {
> + u32 adjustment_value = (u32)(-seconds);
> +
> + if (adjustment_value > 0xF)
> + adjustment_value = 0xF;
> + lan743x_csr_write(adapter, PTP_CLOCK_STEP_ADJ,
> + adjustment_value);
> + seconds += ((s32)adjustment_value);
> + }
> + lan743x_csr_write(adapter, PTP_CMD_CTL,
> + PTP_CMD_CTL_PTP_CLOCK_STEP_SEC_);
> + lan743x_ptp_wait_till_cmd_done(adapter,
> + PTP_CMD_CTL_PTP_CLOCK_STEP_SEC_);
> + mutex_unlock(&ptp->command_lock);
> + }
> + if (nano_seconds) {
> + mutex_lock(&ptp->command_lock);
> + lan743x_csr_write(adapter, PTP_CLOCK_STEP_ADJ,
> + PTP_CLOCK_STEP_ADJ_DIR_ |
> + (nano_seconds &
> + PTP_CLOCK_STEP_ADJ_VALUE_MASK_));
> + lan743x_csr_write(adapter, PTP_CMD_CTL,
> + PTP_CMD_CTL_PTP_CLK_STP_NSEC_);
> + lan743x_ptp_wait_till_cmd_done(adapter,
> + PTP_CMD_CTL_PTP_CLK_STP_NSEC_);
> + mutex_unlock(&ptp->command_lock);
> + }
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +bool lan743x_ptp_request_tx_timestamp(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + bool result = false;
> +
> + spin_lock_bh(&ptp->tx_ts_lock);
> + if (ptp->pending_tx_timestamps < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS) {
> + ptp->pending_tx_timestamps++;
> + result = true;/* request granted */
Avoid tail comments please.
> + }
> + spin_unlock_bh(&ptp->tx_ts_lock);
> + return result;
> +}
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net-next v6 01/11] net: sched: use rcu for action cookie update
From: Cong Wang @ 2018-07-13 3:52 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Yevgeny Kliteynik, Jiri Pirko
In-Reply-To: <1530800673-12280-2-git-send-email-vladbu@mellanox.com>
On Thu, Jul 5, 2018 at 7:24 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Implement functions to atomically update and free action cookie
> using rcu mechanism.
Without stating any reason..... Is this even a changelog?
>
> Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Dear Marcelo, how did it pass your review? See below:
> +static void tcf_set_action_cookie(struct tc_cookie __rcu **old_cookie,
> + struct tc_cookie *new_cookie)
> +{
> + struct tc_cookie *old;
> +
> + old = xchg(old_cookie, new_cookie);
This is an incorrect use of RCU, obviously should be rcu_assign_pointer()
here.
> @@ -65,10 +83,7 @@ static void free_tcf(struct tc_action *p)
> free_percpu(p->cpu_bstats);
> free_percpu(p->cpu_qstats);
>
> - if (p->act_cookie) {
> - kfree(p->act_cookie->data);
> - kfree(p->act_cookie);
> - }
> + tcf_set_action_cookie(&p->act_cookie, NULL);
So, this is called in free_tcf(), where the action is already
invisible from readers so it is ready to be freed.
The question is:
If the action itself is already ready to be freed, why do you
need RCU here? What could still read 'act->act_cookie'
while 'act' is already invisible?
Its last refcnt is already gone, the fast path RCU readers
are gone too given filters use rcu work already.
Standalone action dump? Again, the last refcnt is already
gone.
Marcelo, Vlad, Jiri, please explain.
Thanks!
^ permalink raw reply
* Re: [PATCH net-next v6 00/11] Modify action API for implementing lockless actions
From: Cong Wang @ 2018-07-13 3:54 UTC (permalink / raw)
To: David Miller
Cc: Vlad Buslov, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Yevgeny Kliteynik
In-Reply-To: <20180708.124325.344679298289898945.davem@davemloft.net>
On Sat, Jul 7, 2018 at 8:43 PM David Miller <davem@davemloft.net> wrote:
>
> From: Vlad Buslov <vladbu@mellanox.com>
> Date: Thu, 5 Jul 2018 17:24:22 +0300
>
> > Currently, all netlink protocol handlers for updating rules, actions and
> > qdiscs are protected with single global rtnl lock which removes any
> > possibility for parallelism. This patch set is a first step to remove
> > rtnl lock dependency from TC rules update path.
> ...
>
> I'll apply this for now, I reviewed it a few more times and I see
> where you are going with this.
Dear David,
I don't understand why you even believe the claim of lockless
updaters here, it at least should raise a red flag when you see any
kinda of this claim.
I know you don't trust me, how about thinking it in this way:
Why does RCU still require a lock for RCU writers? (Or at least
RCU recommends a lock, if anyone really wants to point out some
lockless algorithm here.)
or:
If writers could really go lockless as easily as Vlad claims, how could
even Paul E. McKenney never bring it into RCU?
Maybe Vlad is much cleverer than any of us here, and maybe he really
discovers a very brilliant algorithm to allow TC actions to be updated
locklessly, why not wait until he shows a proof (either code or a paper)?
Is there a rush? I don't see it.
In fact, I discussed this with Vlad a little bit at netdev TC workshop.
I never see any brilliant algorithm from him from his slides, and I was
told by him he used "copy and replace" to archive parallel updaters, I
told him that is basically how RCU works and RCU writers have to be
sync'ed with a lock (or at least recommended).
Also, to confirm my judgement, I checked this with Paul privately too.
Paul said you have to be extremely careful to go lockless, it is very hard
to be bug free for lockless, although he _never_ says it is impossible.
My _personal_ bet is that, lockless updates for TC filters or actions
are impossible unless there are more things hiding behind "copy and
replace", for example, some brilliant lockless algorithm. If lockless is
really impossible in this circumstance, then many of your efforts in
this patchset are vain, by the way.
I _do_ believe you can break RTNL down to per device, per filter or per
action, but no matter how small the locking scope is, there is still a lock.
With a lock, there is no need to make things friendly to lockless, like
making an integer increment inside an action to be atomic (your patch
02/11).
Please _do_ prove my personal judgement is wrong, by showing your
final code or a formal paper/article. I am very *happy* to be proved
to be wrong here, I am very open to change my mind here.
Vlad, we need your proof. Please prove I am wrong, seriously!!! :)
Thanks to anyone for proving me I am wrong just in case!!! :)
^ permalink raw reply
* [PATCH] isdn/capi: hide unused procfs helpers
From: YueHaibing @ 2018-07-13 4:11 UTC (permalink / raw)
To: isdn; +Cc: linux-kernel, netdev, keescook, viro, YueHaibing
When CONFIG_PROC_FS isn't set, gcc warning this:
drivers/isdn/capi/capi.c:1324:12: warning: ‘capi20_proc_show’ defined but not used [-Wunused-function]
static int capi20_proc_show(struct seq_file *m, void *v)
^
drivers/isdn/capi/capi.c:1347:12: warning: ‘capi20ncci_proc_show’ defined but not used [-Wunused-function]
static int capi20ncci_proc_show(struct seq_file *m, void *v)
^
drivers/isdn/capi/capidrv.c:2454:12: warning: ‘capidrv_proc_show’ defined but not used [-Wunused-function]
static int capidrv_proc_show(struct seq_file *m, void *v)
^
We can fix the warning by adding the same #ifdef around them.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/isdn/capi/capi.c | 2 ++
drivers/isdn/capi/capidrv.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 6e0c281..594f767 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -1317,6 +1317,7 @@ static inline void capinc_tty_exit(void) { }
/* -------- /proc functions ----------------------------------------- */
+#ifdef CONFIG_PROC_FS
/*
* /proc/capi/capi20:
* minor applid nrecvctlpkt nrecvdatapkt nsendctlpkt nsenddatapkt
@@ -1359,6 +1360,7 @@ static int capi20ncci_proc_show(struct seq_file *m, void *v)
mutex_unlock(&capidev_list_lock);
return 0;
}
+#endif
static void __init proc_init(void)
{
diff --git a/drivers/isdn/capi/capidrv.c b/drivers/isdn/capi/capidrv.c
index ee510f9..c6f45d7 100644
--- a/drivers/isdn/capi/capidrv.c
+++ b/drivers/isdn/capi/capidrv.c
@@ -2447,6 +2447,7 @@ lower_callback(struct notifier_block *nb, unsigned long val, void *v)
return NOTIFY_OK;
}
+#ifdef CONFIG_PROC_FS
/*
* /proc/capi/capidrv:
* nrecvctlpkt nrecvdatapkt nsendctlpkt nsenddatapkt
@@ -2460,6 +2461,7 @@ static int capidrv_proc_show(struct seq_file *m, void *v)
global.ap.nsentdatapkt);
return 0;
}
+#endif
static void __init proc_init(void)
{
--
2.7.0
^ permalink raw reply related
* [PATCH net] tun: Fix use-after-free on XDP_TX
From: Toshiaki Makita @ 2018-07-13 4:24 UTC (permalink / raw)
To: David S . Miller; +Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer
On XDP_TX we need to free up the frame only when tun_xdp_tx() returns a
negative value. A positive value indicates that the packet is
successfully enqueued to the ptr_ring, so freeing the page causes
use-after-free.
Fixes: 735fc4054b3a ("xdp: change ndo_xdp_xmit API to support bulking")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
drivers/net/tun.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index a192a01..f5727ba 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1688,7 +1688,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
case XDP_TX:
get_page(alloc_frag->page);
alloc_frag->offset += buflen;
- if (tun_xdp_tx(tun->dev, &xdp))
+ if (tun_xdp_tx(tun->dev, &xdp) < 0)
goto err_redirect;
rcu_read_unlock();
local_bh_enable();
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net] tun: Fix use-after-free on XDP_TX
From: Jason Wang @ 2018-07-13 5:05 UTC (permalink / raw)
To: Toshiaki Makita, David S . Miller; +Cc: netdev, Jesper Dangaard Brouer
In-Reply-To: <1531455878-2552-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
On 2018年07月13日 12:24, Toshiaki Makita wrote:
> On XDP_TX we need to free up the frame only when tun_xdp_tx() returns a
> negative value. A positive value indicates that the packet is
> successfully enqueued to the ptr_ring, so freeing the page causes
> use-after-free.
>
> Fixes: 735fc4054b3a ("xdp: change ndo_xdp_xmit API to support bulking")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
> drivers/net/tun.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index a192a01..f5727ba 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1688,7 +1688,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
> case XDP_TX:
> get_page(alloc_frag->page);
> alloc_frag->offset += buflen;
> - if (tun_xdp_tx(tun->dev, &xdp))
> + if (tun_xdp_tx(tun->dev, &xdp) < 0)
> goto err_redirect;
> rcu_read_unlock();
> local_bh_enable();
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply
* Re: [PATCH v3 net-next] net/sched: add skbprio scheduler
From: Cong Wang @ 2018-07-13 5:07 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Michel Machado, Nishanth Devarajan, Jamal Hadi Salim, Jiri Pirko,
David Miller, Linux Kernel Network Developers, Cody Doucette
In-Reply-To: <20180711183755.GE8880@localhost.localdomain>
On Wed, Jul 11, 2018 at 11:37 AM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Tue, Jul 10, 2018 at 07:32:43PM -0700, Cong Wang wrote:
> > On Mon, Jul 9, 2018 at 12:53 PM Marcelo Ricardo Leitner
> > <marcelo.leitner@gmail.com> wrote:
> > >
> > > On Mon, Jul 09, 2018 at 02:18:33PM -0400, Michel Machado wrote:
> > > >
> > > > 2. sch_prio.c does not have a global limit on the number of packets on
> > > > all its queues, only a limit per queue.
> > >
> > > It can be useful to sch_prio.c as well, why not?
> > > prio_enqueue()
> > > {
> > > ...
> > > + if (count > sch->global_limit)
> > > + prio_tail_drop(sch); /* to be implemented */
> > > ret = qdisc_enqueue(skb, qdisc, to_free);
> > >
> >
> > Isn't the whole point of sch_prio offloading the queueing to
> > each class? If you need a limit, there is one for each child
> > qdisc if you use for example pfifo or bfifo (depending on you
> > want to limit bytes or packets).
>
> Yes, but Michel wants to drop from other lower priorities if needed,
> and that's not possible if you handle the limit already in a child
> qdisc as they don't know about their siblings. The idea in the example
> above is to discard it from whatever lower priority is needed, then
> queue it. (ok, the example missed to check the priority level)
So it disproves your point of adding a flag to sch_prio, right?
Also, you have to re-introduce qdisc->ops->drop() if you really want
to go this direction.
>
> As for the different units, sch_prio holds a count of how many packets
> are queued on its children, and that's what would be used for the limit.
>
> >
> > Also, what's your plan for backward compatibility here?
>
> say:
> if (sch->global_limit && count > sch->global_limit)
> as in, only do the limit check/enforcing if needed.
Obviously doesn't work, users could pass 0 to effectively
disable the qdisc from enqueue'ing any packet.
^ permalink raw reply
* How are you doing today? Please read my email and reply me!!
From: Billy Wilfred @ 2018-07-13 0:31 UTC (permalink / raw)
Dear Sir/Madam,
How are you doing today? My name is Billy Wilfred. I am California, United States of America. I am a broker of Project Financing Firm who has cutting edge and group capital fund, they can finance any lucrative project and help you to enhance your business plan. Are you in need of a Loan to Finance and Fund your Project or Company? Are you an Investor, Real Estate developer, Construction Company, etc? or Do you need a loan to keep your investment or business going on in order to have a different look? Have you been trying to obtain a Loan from Banks or Loan Companies and got Ripped off and they have refused to grant you the Loan because of bad credit? do not close your company and stop your project because of bankruptcy, we are here for you for real, please be happy, rejoice and celebrate
because your solution have come, we will end your financial worries now. Therefore come to us, we will grant you the loan you need without delay. We offer all types of non-recourse Loan and Funding
at a low Interest Rate of
The categories of Loan Financial Funding we offered include but not limited to: Business Loan, Personal Loan, Company Loan, Mortgage Loan, Debt Consolidation and Financial Funding for both Turnkey and mega projects etc from a minimum of Euro / US$1Million to Euro / US$5 Billion Max. Most importantly, Note that the loan company DO NOT charge any upfront fee or advance fee. This message is not scam for what so ever. This is 100% real, legal and legitimate loan company office in Turkey. Kindly get in touch for further details and procedures. Thanks for your cooperation with us. I will be waiting for your response. Further more details and directives contact me on my private email: bwilalessandro@gmail.com
Thank you & best regards,
Billy Wilfred.
Contact me on my private email: bwilalessandro@gmail.com
^ permalink raw reply
* Re: [PATCH net-next] bnxt_en: remove redundant debug register dma mem allocation
From: Michael Chan @ 2018-07-13 5:42 UTC (permalink / raw)
To: YueHaibing; +Cc: open list, Netdev, David Miller
In-Reply-To: <20180713030204.884-1-yuehaibing@huawei.com>
On Thu, Jul 12, 2018 at 8:02 PM, YueHaibing <yuehaibing@huawei.com> wrote:
> hwrm_dbg_resp_addr and hwrm_dbg_resp_dma_addr are never used
> and can be removed.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Thanks.
Acked-by: Michael Chan <michael.chan@broadcom.com>
^ permalink raw reply
* Attractive interest rate @ (2.75%) for project funding/Loan/finance from Coutts & Co U.K !!
From: COUTTS & CO @ 2018-07-13 4:38 UTC (permalink / raw)
To: netdev
Coutts & Co Ltd
440 Strand, London, WC2R 0QS
Website: https://www.coutts.com
Telephone:+4420 3389 7785 & +4420 7753 1000
Fax:+44 872 110 3479
OUR REF: Coutts/UK/2018/LOANAPP
YOUR REF:LOANAPPT/Coutts/JULY/2018
TO WHOM IT MAY CONCERN
We give out both Local/International Loan starting from 2.75%* rate of interest annually.We offer non-collateral loans to both firms and individuals.
Funds available For Most Projects Covering Real Estate, Industry And Other Forms Of Creativity In Some Major Countries.
You Might Be Interested In Submitting Your Business Proposal And Study For Evaluation.
We Do Not Promise Success Of Funding But We Shall Do Our Best To Help And Assist.
No Upfront Fees Or Payments.We Can Also Provide Bank Guarantees And SBLC's For Your Projects Including Monetization.
Please Contact Us For An Application And For More Instruction Please Email Us with the following information-FULL NAMES/COMPANY NAME,TELEPHONE NUMBER,PURPOSE OF LOAN/FINANCE AND PRESENT LOCATION At Email address:couttsfinance@mail.uk
Quick approvals | Disbursal in 72 hours* | FUND DIRECTLY DEPOSITED
© Copyright Coutts & Co 2018
^ permalink raw reply
* [PATCH net-next] net: ip6_gre: get ipv6hdr after skb_cow_head()
From: Prashant Bhole @ 2018-07-13 5:40 UTC (permalink / raw)
To: David S . Miller
Cc: Prashant Bhole, Alexey Kuznetsov, Hideaki YOSHIFUJI, William Tu,
netdev
A KASAN:use-after-free bug was found related to ip6-erspan
while running selftests/net/ip6_gre_headroom.sh
It happens because of following sequence:
- ipv6hdr pointer is obtained from skb
- skb_cow_head() is called, skb->head memory is reallocated
- old data is accessed using ipv6hdr pointer
skb_cow_head() call was added in e41c7c68ea77 ("ip6erspan: make sure
enough headroom at xmit."), but looking at the history there was a
chance of similar bug because gre_handle_offloads() and pskb_trim()
can also reallocate skb->head memory. Fixes tag points to commit
which introduced possibility of this bug.
This patch moves ipv6hdr pointer assignment after skb_cow_head() call.
Fixes: 5a963eb61b7c ("ip6_gre: Add ERSPAN native tunnel support")
Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
---
net/ipv6/ip6_gre.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 367177786e34..fc7dd3a04360 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -927,7 +927,6 @@ static netdev_tx_t ip6gre_tunnel_xmit(struct sk_buff *skb,
static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
struct net_device *dev)
{
- struct ipv6hdr *ipv6h = ipv6_hdr(skb);
struct ip6_tnl *t = netdev_priv(dev);
struct dst_entry *dst = skb_dst(skb);
struct net_device_stats *stats;
@@ -1012,6 +1011,8 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
goto tx_err;
}
} else {
+ struct ipv6hdr *ipv6h = ipv6_hdr(skb);
+
switch (skb->protocol) {
case htons(ETH_P_IP):
memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net] tun: Fix use-after-free on XDP_TX
From: Jesper Dangaard Brouer @ 2018-07-13 5:57 UTC (permalink / raw)
To: Jason Wang; +Cc: Toshiaki Makita, David S . Miller, netdev, brouer
In-Reply-To: <d18b205d-1d9d-b435-3043-1872145c60b9@redhat.com>
On Fri, 13 Jul 2018 13:05:04 +0800
Jason Wang <jasowang@redhat.com> wrote:
> On 2018年07月13日 12:24, Toshiaki Makita wrote:
> > On XDP_TX we need to free up the frame only when tun_xdp_tx() returns a
> > negative value. A positive value indicates that the packet is
> > successfully enqueued to the ptr_ring, so freeing the page causes
> > use-after-free.
> >
> > Fixes: 735fc4054b3a ("xdp: change ndo_xdp_xmit API to support bulking")
> > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> > ---
> > drivers/net/tun.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index a192a01..f5727ba 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -1688,7 +1688,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
> > case XDP_TX:
> > get_page(alloc_frag->page);
> > alloc_frag->offset += buflen;
> > - if (tun_xdp_tx(tun->dev, &xdp))
> > + if (tun_xdp_tx(tun->dev, &xdp) < 0)
> > goto err_redirect;
> > rcu_read_unlock();
> > local_bh_enable();
>
> Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Thanks for catching and fixing this!
--
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 v3 net-next] net/sched: add skbprio scheduler
From: Cong Wang @ 2018-07-13 6:05 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Michel Machado, Nishanth Devarajan, Jamal Hadi Salim, Jiri Pirko,
David Miller, Linux Kernel Network Developers, Cody Doucette
In-Reply-To: <20180711193336.GF8880@localhost.localdomain>
On Wed, Jul 11, 2018 at 12:33 PM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Tue, Jul 10, 2018 at 07:25:53PM -0700, Cong Wang wrote:
> > On Mon, Jul 9, 2018 at 2:40 PM Marcelo Ricardo Leitner
> > <marcelo.leitner@gmail.com> wrote:
> > >
> > > On Mon, Jul 09, 2018 at 05:03:31PM -0400, Michel Machado wrote:
> > > > Changing TC_PRIO_MAX from 15 to 63 risks breaking backward compatibility
> > > > with applications.
> > >
> > > If done, it needs to be done carefully, indeed. I don't know if it's
> > > doable, neither I know how hard is your requirement for 64 different
> > > priorities.
> >
> > struct tc_prio_qopt {
> > int bands; /* Number of bands */
> > __u8 priomap[TC_PRIO_MAX+1]; /* Map: logical priority -> PRIO band */
> > };
> >
> > How would you do it carefully?
>
> quick shot, multiplex v1 and v2 formats based on bands and sizeof():
>
> #define TCQ_PRIO_BANDS_V1 16
> #define TCQ_PRIO_BANDS_V2 64
> #define TC_PRIO_MAX_V2 64
>
> struct tc_prio_qopt_v2 {
> int bands; /* Number of bands */
> __u8 priomap[TC_PRIO_MAX_V2+1]; /* Map: logical priority -> PRIO band */
> };
>
Good try, but:
1. You don't take padding into account, although the difference
between 16 and 64 is big here. If it were 16 and 20, almost certainly
wouldn't work.
2. What if I compile a new iproute2 on an old kernel? The iproute2
will use V2, while old kernel has no knowledge of V2, so it only
copies a part of V2 in the end....
^ permalink raw reply
* Attractive interest rate @ (2.75%) for project funding/Loan/finance from Coutts & Co U.K !!
From: COUTTS & CO @ 2018-07-13 5:03 UTC (permalink / raw)
To: Recipients
Coutts & Co Ltd
440 Strand, London, WC2R 0QS
Website: https://www.coutts.com
Telephone:+4420 3389 7785 & +4420 7753 1000
Fax:+44 872 110 3479
OUR REF: Coutts/UK/2018/LOANAPP
YOUR REF:LOANAPPT/Coutts/JULY/2018
TO WHOM IT MAY CONCERN
We give out both Local/International Loan starting from 2.75%* rate of interest annually.We offer non-collateral loans to both firms and individuals.
Funds available For Most Projects Covering Real Estate, Industry And Other Forms Of Creativity In Some Major Countries.
You Might Be Interested In Submitting Your Business Proposal And Study For Evaluation.
We Do Not Promise Success Of Funding But We Shall Do Our Best To Help And Assist.
No Upfront Fees Or Payments.We Can Also Provide Bank Guarantees And SBLC's For Your Projects Including Monetization.
Please Contact Us For An Application And For More Instruction Please Email Us with the following information-FULL NAMES/COMPANY NAME,TELEPHONE NUMBER,PURPOSE OF LOAN/FINANCE AND PRESENT LOCATION At Email address:couttsfinance@mail.uk
Quick approvals | Disbursal in 72 hours* | FUND DIRECTLY DEPOSITED
© Copyright Coutts & Co 2018
^ permalink raw reply
* [PATCH 03/10] atmel: hide unused procfs helpers
From: YueHaibing @ 2018-07-13 6:46 UTC (permalink / raw)
To: simon, kvalo; +Cc: linux-kernel, netdev, linux-wireless, davem, YueHaibing
When CONFIG_PROC_FS isn't set, gcc warning this:
drivers/net/wireless/atmel/atmel.c:1402:12: warning: ‘atmel_proc_show’ defined but not used [-Wunused-function]
static int atmel_proc_show(struct seq_file *m, void *v)
^
fix this by adding #ifdef around it.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/wireless/atmel/atmel.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/wireless/atmel/atmel.c b/drivers/net/wireless/atmel/atmel.c
index b01dc34..3940876 100644
--- a/drivers/net/wireless/atmel/atmel.c
+++ b/drivers/net/wireless/atmel/atmel.c
@@ -1399,6 +1399,7 @@ static int atmel_validate_channel(struct atmel_private *priv, int channel)
return 0;
}
+#ifdef CONFIG_PROC_FS
static int atmel_proc_show(struct seq_file *m, void *v)
{
struct atmel_private *priv = m->private;
@@ -1481,6 +1482,7 @@ static int atmel_proc_show(struct seq_file *m, void *v)
seq_printf(m, "Current state:\t\t%s\n", s);
return 0;
}
+#endif
static const struct net_device_ops atmel_netdev_ops = {
.ndo_open = atmel_open,
--
2.7.0
^ permalink raw reply related
* [PATCH][net-next] bridge: clean up mtu_set_by_user setting to false and comments
From: Li RongQing @ 2018-07-13 6:47 UTC (permalink / raw)
To: netdev; +Cc: nikolay
Once mtu_set_by_user is set to true, br_mtu_auto_adjust will
not run, and no chance to clear mtu_set_by_user.
and br_mtu_auto_adjust will run only if mtu_set_by_user is
false, so not need to set it to false again
Cc: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
net/bridge/br_device.c | 1 -
net/bridge/br_if.c | 4 ----
2 files changed, 5 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index e682a668ce57..c636bc2749c2 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -227,7 +227,6 @@ static int br_change_mtu(struct net_device *dev, int new_mtu)
dev->mtu = new_mtu;
- /* this flag will be cleared if the MTU was automatically adjusted */
br->mtu_set_by_user = true;
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
/* remember the MTU in the rtable for PMTU */
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 05e42d86882d..47c65da4b1be 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -450,11 +450,7 @@ void br_mtu_auto_adjust(struct net_bridge *br)
if (br->mtu_set_by_user)
return;
- /* change to the minimum MTU and clear the flag which was set by
- * the bridge ndo_change_mtu callback
- */
dev_set_mtu(br->dev, br_mtu_min(br));
- br->mtu_set_by_user = false;
}
static void br_set_gso_limits(struct net_bridge *br)
--
2.16.2
^ permalink raw reply related
* [PATCH] connector: hide unused procfs helpers
From: YueHaibing @ 2018-07-13 7:11 UTC (permalink / raw)
To: zbr; +Cc: linux-kernel, netdev, YueHaibing
When CONFIG_PROC_FS isn't set, gcc warning this:
drivers/connector/connector.c:242:12: warning: ‘cn_proc_show’ defined but not used [-Wunused-function]
static int cn_proc_show(struct seq_file *m, void *v)
^
fix this by adding #ifdef around it.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/connector/connector.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index e718b8c..e2636d4 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -239,6 +239,7 @@ void cn_del_callback(struct cb_id *id)
}
EXPORT_SYMBOL_GPL(cn_del_callback);
+#ifdef CONFIG_PROC_FS
static int cn_proc_show(struct seq_file *m, void *v)
{
struct cn_queue_dev *dev = cdev.cbdev;
@@ -259,6 +260,7 @@ static int cn_proc_show(struct seq_file *m, void *v)
return 0;
}
+#endif
static struct cn_dev cdev = {
.input = cn_rx_skb,
--
2.7.0
^ permalink raw reply related
* [PATCH][net-next][v2] net: convert gro_count to bitmask
From: Li RongQing @ 2018-07-13 6:41 UTC (permalink / raw)
To: netdev; +Cc: edumazet, sbrivio
gro_hash size is 192 bytes, and uses 3 cache lines, if there is few
flows, gro_hash may be not fully used, so it is unnecessary to iterate
all gro_hash in napi_gro_flush(), to occupy unnecessary cacheline.
convert gro_count to a bitmask, and rename it as gro_bitmask, each bit
represents a element of gro_hash, only flush a gro_hash element if the
related bit is set, to speed up napi_gro_flush().
and update gro_bitmask only if it will be changed, to reduce cache
update
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Li RongQing <lirongqing@baidu.com>
Cc: Stefano Brivio <sbrivio@redhat.com>
---
netperf shows no difference, maybe because my testing machine has large
cache
include/linux/netdevice.h | 9 +++++++--
net/core/dev.c | 36 ++++++++++++++++++++++++------------
2 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2daf2fa6554f..8837a998de3f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -308,9 +308,14 @@ struct gro_list {
};
/*
- * Structure for NAPI scheduling similar to tasklet but with weighting
+ * size of gro hash buckets, must less than bit number of
+ * napi_struct::gro_bitmask
*/
#define GRO_HASH_BUCKETS 8
+
+/*
+ * Structure for NAPI scheduling similar to tasklet but with weighting
+ */
struct napi_struct {
/* The poll_list must only be managed by the entity which
* changes the state of the NAPI_STATE_SCHED bit. This means
@@ -322,7 +327,7 @@ struct napi_struct {
unsigned long state;
int weight;
- unsigned int gro_count;
+ unsigned long gro_bitmask;
int (*poll)(struct napi_struct *, int);
#ifdef CONFIG_NETPOLL
int poll_owner;
diff --git a/net/core/dev.c b/net/core/dev.c
index 14a748ee8cc9..e39fef62e285 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5283,9 +5283,11 @@ static void __napi_gro_flush_chain(struct napi_struct *napi, u32 index,
list_del(&skb->list);
skb->next = NULL;
napi_gro_complete(skb);
- napi->gro_count--;
napi->gro_hash[index].count--;
}
+
+ if (!napi->gro_hash[index].count)
+ __clear_bit(index, &napi->gro_bitmask);
}
/* napi->gro_hash[].list contains packets ordered by age.
@@ -5296,8 +5298,10 @@ void napi_gro_flush(struct napi_struct *napi, bool flush_old)
{
u32 i;
- for (i = 0; i < GRO_HASH_BUCKETS; i++)
- __napi_gro_flush_chain(napi, i, flush_old);
+ for (i = 0; i < GRO_HASH_BUCKETS; i++) {
+ if (test_bit(i, &napi->gro_bitmask))
+ __napi_gro_flush_chain(napi, i, flush_old);
+ }
}
EXPORT_SYMBOL(napi_gro_flush);
@@ -5389,8 +5393,8 @@ static void gro_flush_oldest(struct list_head *head)
if (WARN_ON_ONCE(!oldest))
return;
- /* Do not adjust napi->gro_count, caller is adding a new SKB to
- * the chain.
+ /* Do not adjust napi->gro_hash[].count, caller is adding a new
+ * SKB to the chain.
*/
list_del(&oldest->list);
napi_gro_complete(oldest);
@@ -5465,7 +5469,6 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
list_del(&pp->list);
pp->next = NULL;
napi_gro_complete(pp);
- napi->gro_count--;
napi->gro_hash[hash].count--;
}
@@ -5478,7 +5481,6 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
if (unlikely(napi->gro_hash[hash].count >= MAX_GRO_SKBS)) {
gro_flush_oldest(gro_head);
} else {
- napi->gro_count++;
napi->gro_hash[hash].count++;
}
NAPI_GRO_CB(skb)->count = 1;
@@ -5493,6 +5495,13 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
if (grow > 0)
gro_pull_from_frag0(skb, grow);
ok:
+ if (napi->gro_hash[hash].count) {
+ if (!test_bit(hash, &napi->gro_bitmask))
+ __set_bit(hash, &napi->gro_bitmask);
+ } else if (test_bit(hash, &napi->gro_bitmask)) {
+ __clear_bit(hash, &napi->gro_bitmask);
+ }
+
return ret;
normal:
@@ -5891,7 +5900,7 @@ bool napi_complete_done(struct napi_struct *n, int work_done)
NAPIF_STATE_IN_BUSY_POLL)))
return false;
- if (n->gro_count) {
+ if (n->gro_bitmask) {
unsigned long timeout = 0;
if (work_done)
@@ -6100,7 +6109,7 @@ static enum hrtimer_restart napi_watchdog(struct hrtimer *timer)
/* Note : we use a relaxed variant of napi_schedule_prep() not setting
* NAPI_STATE_MISSED, since we do not react to a device IRQ.
*/
- if (napi->gro_count && !napi_disable_pending(napi) &&
+ if (napi->gro_bitmask && !napi_disable_pending(napi) &&
!test_and_set_bit(NAPI_STATE_SCHED, &napi->state))
__napi_schedule_irqoff(napi);
@@ -6115,7 +6124,7 @@ void netif_napi_add(struct net_device *dev, struct napi_struct *napi,
INIT_LIST_HEAD(&napi->poll_list);
hrtimer_init(&napi->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED);
napi->timer.function = napi_watchdog;
- napi->gro_count = 0;
+ napi->gro_bitmask = 0;
for (i = 0; i < GRO_HASH_BUCKETS; i++) {
INIT_LIST_HEAD(&napi->gro_hash[i].list);
napi->gro_hash[i].count = 0;
@@ -6175,7 +6184,7 @@ void netif_napi_del(struct napi_struct *napi)
napi_free_frags(napi);
flush_gro_hash(napi);
- napi->gro_count = 0;
+ napi->gro_bitmask = 0;
}
EXPORT_SYMBOL(netif_napi_del);
@@ -6217,7 +6226,7 @@ static int napi_poll(struct napi_struct *n, struct list_head *repoll)
goto out_unlock;
}
- if (n->gro_count) {
+ if (n->gro_bitmask) {
/* flush too old packets
* If HZ < 1000, flush all packets.
*/
@@ -9264,6 +9273,9 @@ static struct hlist_head * __net_init netdev_create_hash(void)
/* Initialize per network namespace state */
static int __net_init netdev_init(struct net *net)
{
+ BUILD_BUG_ON(GRO_HASH_BUCKETS >
+ FIELD_SIZEOF(struct napi_struct, gro_bitmask));
+
if (net != &init_net)
INIT_LIST_HEAD(&net->dev_base_head);
--
2.16.2
^ permalink raw reply related
* [PATCH] hostap: hide unused procfs helpers
From: YueHaibing @ 2018-07-13 7:03 UTC (permalink / raw)
To: j, kvalo; +Cc: linux-kernel, netdev, linux-wireless, davem, YueHaibing
When CONFIG_PROC_FS isn't set, gcc warning this:
drivers/net/wireless/intersil/hostap/hostap_hw.c:2901:12: warning: ‘prism2_registers_proc_show’ defined but not used [-Wunused-function]
static int prism2_registers_proc_show(struct seq_file *m, void *v)
drivers/net/wireless/intersil/hostap/hostap_proc.c:16:12: warning: ‘prism2_debug_proc_show’ defined but not used [-Wunused-function]
static int prism2_debug_proc_show(struct seq_file *m, void *v)
^
drivers/net/wireless/intersil/hostap/hostap_proc.c:49:12: warning: ‘prism2_stats_proc_show’ defined but not used [-Wunused-function]
static int prism2_stats_proc_show(struct seq_file *m, void *v)
^
drivers/net/wireless/intersil/hostap/hostap_proc.c:177:12: warning: ‘prism2_crypt_proc_show’ defined but not used [-Wunused-function]
static int prism2_crypt_proc_show(struct seq_file *m, void *v)
^
fix this by adding #ifdef around them.
hfa384x_read_reg is only used by prism2_registers_proc_show,so move it
into #ifdef.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/wireless/intersil/hostap/hostap_ap.c | 8 ++++----
drivers/net/wireless/intersil/hostap/hostap_hw.c | 17 +++++++----------
drivers/net/wireless/intersil/hostap/hostap_proc.c | 10 ++++++----
3 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/net/wireless/intersil/hostap/hostap_ap.c b/drivers/net/wireless/intersil/hostap/hostap_ap.c
index d1884b8..0094b1d 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_ap.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_ap.c
@@ -66,7 +66,7 @@ static void prism2_send_mgmt(struct net_device *dev,
#endif /* PRISM2_NO_KERNEL_IEEE80211_MGMT */
-#ifndef PRISM2_NO_PROCFS_DEBUG
+#if !defined(PRISM2_NO_PROCFS_DEBUG) && defined(CONFIG_PROC_FS)
static int ap_debug_proc_show(struct seq_file *m, void *v)
{
struct ap_data *ap = PDE_DATA(file_inode(m->file));
@@ -81,8 +81,7 @@ static int ap_debug_proc_show(struct seq_file *m, void *v)
seq_printf(m, "tx_drop_nonassoc=%u\n", ap->tx_drop_nonassoc);
return 0;
}
-#endif /* PRISM2_NO_PROCFS_DEBUG */
-
+#endif
static void ap_sta_hash_add(struct ap_data *ap, struct sta_info *sta)
{
@@ -990,7 +989,7 @@ static void prism2_send_mgmt(struct net_device *dev,
}
#endif /* PRISM2_NO_KERNEL_IEEE80211_MGMT */
-
+#ifdef CONFIG_PROC_FS
static int prism2_sta_proc_show(struct seq_file *m, void *v)
{
struct sta_info *sta = m->private;
@@ -1059,6 +1058,7 @@ static int prism2_sta_proc_show(struct seq_file *m, void *v)
return 0;
}
+#endif
static void handle_add_proc_queue(struct work_struct *work)
{
diff --git a/drivers/net/wireless/intersil/hostap/hostap_hw.c b/drivers/net/wireless/intersil/hostap/hostap_hw.c
index 2720aa3..ad1aa65 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_hw.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_hw.c
@@ -151,13 +151,6 @@ static int prism2_get_ram_size(local_info_t *local);
#define HFA384X_MAGIC 0x8A32
#endif
-
-static u16 hfa384x_read_reg(struct net_device *dev, u16 reg)
-{
- return HFA384X_INW(reg);
-}
-
-
static void hfa384x_read_regs(struct net_device *dev,
struct hfa384x_regs *regs)
{
@@ -2897,7 +2890,12 @@ static void hostap_tick_timer(struct timer_list *t)
}
-#ifndef PRISM2_NO_PROCFS_DEBUG
+#if !defined(PRISM2_NO_PROCFS_DEBUG) && defined(CONFIG_PROC_FS)
+static u16 hfa384x_read_reg(struct net_device *dev, u16 reg)
+{
+ return HFA384X_INW(reg);
+}
+
static int prism2_registers_proc_show(struct seq_file *m, void *v)
{
local_info_t *local = m->private;
@@ -2951,8 +2949,7 @@ static int prism2_registers_proc_show(struct seq_file *m, void *v)
return 0;
}
-#endif /* PRISM2_NO_PROCFS_DEBUG */
-
+#endif
struct set_tim_data {
struct list_head list;
diff --git a/drivers/net/wireless/intersil/hostap/hostap_proc.c b/drivers/net/wireless/intersil/hostap/hostap_proc.c
index 5b33cca..703d74c 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_proc.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_proc.c
@@ -11,8 +11,7 @@
#define PROC_LIMIT (PAGE_SIZE - 80)
-
-#ifndef PRISM2_NO_PROCFS_DEBUG
+#if !defined(PRISM2_NO_PROCFS_DEBUG) && defined(CONFIG_PROC_FS)
static int prism2_debug_proc_show(struct seq_file *m, void *v)
{
local_info_t *local = m->private;
@@ -43,9 +42,9 @@ static int prism2_debug_proc_show(struct seq_file *m, void *v)
return 0;
}
-#endif /* PRISM2_NO_PROCFS_DEBUG */
-
+#endif
+#ifdef CONFIG_PROC_FS
static int prism2_stats_proc_show(struct seq_file *m, void *v)
{
local_info_t *local = m->private;
@@ -82,6 +81,7 @@ static int prism2_stats_proc_show(struct seq_file *m, void *v)
return 0;
}
+#endif
static int prism2_wds_proc_show(struct seq_file *m, void *v)
{
@@ -174,6 +174,7 @@ static const struct seq_operations prism2_bss_list_proc_seqops = {
.show = prism2_bss_list_proc_show,
};
+#ifdef CONFIG_PROC_FS
static int prism2_crypt_proc_show(struct seq_file *m, void *v)
{
local_info_t *local = m->private;
@@ -190,6 +191,7 @@ static int prism2_crypt_proc_show(struct seq_file *m, void *v)
}
return 0;
}
+#endif
static ssize_t prism2_pda_proc_read(struct file *file, char __user *buf,
size_t count, loff_t *_pos)
--
2.7.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