* [PATCH v2 net] ptp: clockmatrix: bug fix for idtcm_strverscmp
From: min.li.xe @ 2020-11-24 16:01 UTC (permalink / raw)
To: richardcochran; +Cc: netdev, linux-kernel, Min Li
From: Min Li <min.li.xe@renesas.com>
Feed kstrtou8 with NULL terminated string.
Changes since v1:
-Use sscanf to get rid of adhoc string parse.
Signed-off-by: Min Li <min.li.xe@renesas.com>
---
drivers/ptp/ptp_clockmatrix.c | 53 +++++++++++++++----------------------------
1 file changed, 18 insertions(+), 35 deletions(-)
diff --git a/drivers/ptp/ptp_clockmatrix.c b/drivers/ptp/ptp_clockmatrix.c
index e020faf..12d939f 100644
--- a/drivers/ptp/ptp_clockmatrix.c
+++ b/drivers/ptp/ptp_clockmatrix.c
@@ -103,43 +103,26 @@ static int timespec_to_char_array(struct timespec64 const *ts,
return 0;
}
-static int idtcm_strverscmp(const char *ver1, const char *ver2)
+static int idtcm_strverscmp(const char *version1, const char *version2)
{
- u8 num1;
- u8 num2;
- int result = 0;
-
- /* loop through each level of the version string */
- while (result == 0) {
- /* extract leading version numbers */
- if (kstrtou8(ver1, 10, &num1) < 0)
- return -1;
-
- if (kstrtou8(ver2, 10, &num2) < 0)
- return -1;
-
- /* if numbers differ, then set the result */
- if (num1 < num2)
- result = -1;
- else if (num1 > num2)
- result = 1;
- else {
- /* if numbers are the same, go to next level */
- ver1 = strchr(ver1, '.');
- ver2 = strchr(ver2, '.');
- if (!ver1 && !ver2)
- break;
- else if (!ver1)
- result = -1;
- else if (!ver2)
- result = 1;
- else {
- ver1++;
- ver2++;
- }
- }
+ u8 ver1[3], ver2[3];
+ int i;
+
+ if (sscanf(version1, "%hhu.%hhu.%hhu",
+ &ver1[0], &ver1[1], &ver1[2]) < 0)
+ return -1;
+ if (sscanf(version2, "%hhu.%hhu.%hhu",
+ &ver2[0], &ver2[1], &ver2[2]) < 0)
+ return -1;
+
+ for (i = 0; i < 3; i++) {
+ if (ver1[i] > ver2[i])
+ return 1;
+ if (ver1[i] < ver2[i])
+ return -1;
}
- return result;
+
+ return 0;
}
static int idtcm_xfer_read(struct idtcm *idtcm,
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 117/141] rtl8xxxu: Fix fall-through warnings for Clang
From: Gustavo A. R. Silva @ 2020-11-24 16:09 UTC (permalink / raw)
To: Jes Sorensen
Cc: Kalle Valo, David S. Miller, Jakub Kicinski, linux-wireless,
netdev, linux-kernel, linux-hardening
In-Reply-To: <691515e4-4d40-56cf-5f7a-1819aad1afa9@gmail.com>
On Fri, Nov 20, 2020 at 04:39:42PM -0500, Jes Sorensen wrote:
> On 11/20/20 1:38 PM, Gustavo A. R. Silva wrote:
> > In preparation to enable -Wimplicit-fallthrough for Clang, fix
> > multiple warnings by replacing /* fall through */ comments with
> > the new pseudo-keyword macro fallthrough; instead of letting the
> > code fall through to the next case.
> >
> > Notice that Clang doesn't recognize /* fall through */ comments as
> > implicit fall-through markings.
> >
> > Link: https://github.com/KSPP/linux/issues/115
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > ---
> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
>
> While I wasn't CC'ed on the cover-letter I see Jakub also raised issues
> about this unnecessary patch noise.
>
> Quite frankly, this seems to be patch churn for the sake of patch churn.
> If clang is broken, fix clang instead.
Just notice that the idea behind this and the following patch is exactly
the same:
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git/commit/?id=3f95e92c8a8516b745594049dfccc8c5f8895eea
I could resend this same patch with a different changelog text, but I
don't think such a thing is necessary. However, if people prefer that
approach, just let me know and I can do it.
Thanks
--
Gustavo
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > index 5cd7ef3625c5..afc97958fa4d 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > @@ -1145,7 +1145,7 @@ void rtl8xxxu_gen1_config_channel(struct ieee80211_hw *hw)
> > switch (hw->conf.chandef.width) {
> > case NL80211_CHAN_WIDTH_20_NOHT:
> > ht = false;
> > - /* fall through */
> > + fallthrough;
> > case NL80211_CHAN_WIDTH_20:
> > opmode |= BW_OPMODE_20MHZ;
> > rtl8xxxu_write8(priv, REG_BW_OPMODE, opmode);
> > @@ -1272,7 +1272,7 @@ void rtl8xxxu_gen2_config_channel(struct ieee80211_hw *hw)
> > switch (hw->conf.chandef.width) {
> > case NL80211_CHAN_WIDTH_20_NOHT:
> > ht = false;
> > - /* fall through */
> > + fallthrough;
> > case NL80211_CHAN_WIDTH_20:
> > rf_mode_bw |= WMAC_TRXPTCL_CTL_BW_20;
> > subchannel = 0;
> > @@ -1741,11 +1741,11 @@ static int rtl8xxxu_identify_chip(struct rtl8xxxu_priv *priv)
> > case 3:
> > priv->ep_tx_low_queue = 1;
> > priv->ep_tx_count++;
> > - /* fall through */
> > + fallthrough;
> > case 2:
> > priv->ep_tx_normal_queue = 1;
> > priv->ep_tx_count++;
> > - /* fall through */
> > + fallthrough;
> > case 1:
> > priv->ep_tx_high_queue = 1;
> > priv->ep_tx_count++;
> >
>
^ permalink raw reply
* Re: pull-request: wireless-drivers-2020-11-23
From: Jakub Kicinski @ 2020-11-24 16:08 UTC (permalink / raw)
To: Kalle Valo; +Cc: netdev, linux-wireless
In-Reply-To: <87im9vql7i.fsf@codeaurora.org>
On Tue, 24 Nov 2020 09:15:45 +0200 Kalle Valo wrote:
> Jakub Kicinski <kuba@kernel.org> writes:
>
> > On Mon, 23 Nov 2020 16:10:37 +0000 (UTC) Kalle Valo wrote:
> >> wireless-drivers fixes for v5.10
> >>
> >> First set of fixes for v5.10. One fix for iwlwifi kernel panic, others
> >> less notable.
> >>
> >> rtw88
> >>
> >> * fix a bogus test found by clang
> >>
> >> iwlwifi
> >>
> >> * fix long memory reads causing soft lockup warnings
> >>
> >> * fix kernel panic during Channel Switch Announcement (CSA)
> >>
> >> * other smaller fixes
> >>
> >> MAINTAINERS
> >>
> >> * email address updates
> >
> > Pulled, thanks!
> >
> > Please watch out for missing sign-offs.
>
> I assume you refer to commit 97cc16943f23, sorry about that. Currently
> I'm just manually checking sign-offs and missed this patch. My plan is
> to implement proper checks to my patchwork script so I'll notice these
> before I commit the patch (or pull request), just have not yet find the
> time to do that.
Check out verify_fixes and verify_signoff in Greg's repo:
https://github.com/gregkh/gregkh-linux/tree/master/work
^ permalink raw reply
* RE: [PATCH net-next v4 2/7] dpaa_eth: add basic XDP support
From: Camelia Alexandra Groza @ 2020-11-24 16:11 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: kuba@kernel.org, brouer@redhat.com, saeed@kernel.org,
davem@davemloft.net, Madalin Bucur (OSS), Ioana Ciornei,
netdev@vger.kernel.org
In-Reply-To: <20201124135117.GA12808@ranger.igk.intel.com>
> -----Original Message-----
> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Sent: Tuesday, November 24, 2020 15:51
> To: Camelia Alexandra Groza <camelia.groza@nxp.com>
> Cc: kuba@kernel.org; brouer@redhat.com; saeed@kernel.org;
> davem@davemloft.net; Madalin Bucur (OSS)
> <madalin.bucur@oss.nxp.com>; Ioana Ciornei <ioana.ciornei@nxp.com>;
> netdev@vger.kernel.org
> Subject: Re: [PATCH net-next v4 2/7] dpaa_eth: add basic XDP support
>
> On Mon, Nov 23, 2020 at 07:36:20PM +0200, Camelia Groza wrote:
> > Implement the XDP_DROP and XDP_PASS actions.
> >
> > Avoid draining and reconfiguring the buffer pool at each XDP
> > setup/teardown by increasing the frame headroom and reserving
> > XDP_PACKET_HEADROOM bytes from the start. Since we always reserve
> an
> > entire page per buffer, this change only impacts Jumbo frame scenarios
> > where the maximum linear frame size is reduced by 256 bytes. Multi
> > buffer Scatter/Gather frames are now used instead in these scenarios.
> >
> > Allow XDP programs to access the entire buffer.
> >
> > The data in the received frame's headroom can be overwritten by the XDP
> > program. Extract the relevant fields from the headroom while they are
> > still available, before running the XDP program.
> >
> > Since the headroom might be resized before the frame is passed up to the
> > stack, remove the check for a fixed headroom value when building an skb.
> >
> > Allow the meta data to be updated and pass the information up the stack.
> >
> > Scatter/Gather frames are dropped when XDP is enabled.
> >
> > Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> > Signed-off-by: Camelia Groza <camelia.groza@nxp.com>
> > ---
> > Changes in v2:
> > - warn only once if extracting the timestamp from a received frame fails
> >
> > Changes in v3:
> > - drop received S/G frames when XDP is enabled
> >
> > Changes in v4:
> > - report a warning if the MTU is too hight for running XDP
> > - report an error if opening the device fails in the XDP setup
> >
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 166
> ++++++++++++++++++++++---
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 2 +
> > 2 files changed, 152 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > index 88533a2..8acce62 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > @@ -53,6 +53,8 @@
> > #include <linux/dma-mapping.h>
> > #include <linux/sort.h>
> > #include <linux/phy_fixed.h>
> > +#include <linux/bpf.h>
> > +#include <linux/bpf_trace.h>
> > #include <soc/fsl/bman.h>
> > #include <soc/fsl/qman.h>
> > #include "fman.h"
> > @@ -177,7 +179,7 @@
> > #define DPAA_HWA_SIZE (DPAA_PARSE_RESULTS_SIZE +
> DPAA_TIME_STAMP_SIZE \
> > + DPAA_HASH_RESULTS_SIZE)
> > #define DPAA_RX_PRIV_DATA_DEFAULT_SIZE
> (DPAA_TX_PRIV_DATA_SIZE + \
> > - dpaa_rx_extra_headroom)
> > + XDP_PACKET_HEADROOM -
> DPAA_HWA_SIZE)
> > #ifdef CONFIG_DPAA_ERRATUM_A050385
> > #define DPAA_RX_PRIV_DATA_A050385_SIZE (DPAA_A050385_ALIGN -
> DPAA_HWA_SIZE)
> > #define DPAA_RX_PRIV_DATA_SIZE (fman_has_errata_a050385() ? \
> > @@ -1733,7 +1735,6 @@ static struct sk_buff *contig_fd_to_skb(const
> struct dpaa_priv *priv,
> > SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
> > if (WARN_ONCE(!skb, "Build skb failure on Rx\n"))
> > goto free_buffer;
> > - WARN_ON(fd_off != priv->rx_headroom);
> > skb_reserve(skb, fd_off);
> > skb_put(skb, qm_fd_get_length(fd));
> >
> > @@ -2349,12 +2350,62 @@ static enum qman_cb_dqrr_result
> rx_error_dqrr(struct qman_portal *portal,
> > return qman_cb_dqrr_consume;
> > }
> >
> > +static u32 dpaa_run_xdp(struct dpaa_priv *priv, struct qm_fd *fd, void
> *vaddr,
> > + unsigned int *xdp_meta_len)
> > +{
> > + ssize_t fd_off = qm_fd_get_offset(fd);
> > + struct bpf_prog *xdp_prog;
> > + struct xdp_buff xdp;
> > + u32 xdp_act;
> > +
> > + rcu_read_lock();
> > +
> > + xdp_prog = READ_ONCE(priv->xdp_prog);
> > + if (!xdp_prog) {
> > + rcu_read_unlock();
> > + return XDP_PASS;
> > + }
> > +
> > + xdp.data = vaddr + fd_off;
> > + xdp.data_meta = xdp.data;
> > + xdp.data_hard_start = xdp.data - XDP_PACKET_HEADROOM;
> > + xdp.data_end = xdp.data + qm_fd_get_length(fd);
> > + xdp.frame_sz = DPAA_BP_RAW_SIZE;
> > +
> > + xdp_act = bpf_prog_run_xdp(xdp_prog, &xdp);
> > +
> > + /* Update the length and the offset of the FD */
> > + qm_fd_set_contig(fd, xdp.data - vaddr, xdp.data_end - xdp.data);
>
> Shouldn't you do this update based on xdp.data_meta, not xdp.data?
Both the xdp.data_meta and xdp.data can be updated when the XDP program runs.
In case of XDP_PASS, when building an skb around an fd, we need to know where the data starts for calling skb_reserve(). The metadata set by the XDP program is part of the skb's headroom, not part of the packet data. So we adjust the fd's offset and length based on the data, not the metadata. We let the skb know it has metadata set in its headroom through skb_metadata_set().
For XDP_TX we don't pass metadata to the hardware, and for XDP_REDIRECT the xdp_frame points to this information, so there is no need for keeping track of it in the fd.
> > +
> > + switch (xdp_act) {
> > + case XDP_PASS:
> > + *xdp_meta_len = xdp.data - xdp.data_meta;
> > + break;
> > + default:
> > + bpf_warn_invalid_xdp_action(xdp_act);
> > + fallthrough;
> > + case XDP_ABORTED:
> > + trace_xdp_exception(priv->net_dev, xdp_prog, xdp_act);
> > + fallthrough;
> > + case XDP_DROP:
> > + /* Free the buffer */
> > + free_pages((unsigned long)vaddr, 0);
> > + break;
> > + }
> > +
> > + rcu_read_unlock();
> > +
> > + return xdp_act;
> > +}
> > +
> > static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal
> *portal,
> > struct qman_fq *fq,
> > const struct qm_dqrr_entry
> *dq,
> > bool sched_napi)
> > {
> > + bool ts_valid = false, hash_valid = false;
> > struct skb_shared_hwtstamps *shhwtstamps;
> > + unsigned int skb_len, xdp_meta_len = 0;
> > struct rtnl_link_stats64 *percpu_stats;
> > struct dpaa_percpu_priv *percpu_priv;
> > const struct qm_fd *fd = &dq->fd;
> > @@ -2362,12 +2413,14 @@ static enum qman_cb_dqrr_result
> rx_default_dqrr(struct qman_portal *portal,
> > enum qm_fd_format fd_format;
> > struct net_device *net_dev;
> > u32 fd_status, hash_offset;
> > + struct qm_sg_entry *sgt;
> > struct dpaa_bp *dpaa_bp;
> > struct dpaa_priv *priv;
> > - unsigned int skb_len;
> > struct sk_buff *skb;
> > int *count_ptr;
> > + u32 xdp_act;
> > void *vaddr;
> > + u32 hash;
> > u64 ns;
> >
> > fd_status = be32_to_cpu(fd->status);
> > @@ -2423,35 +2476,67 @@ static enum qman_cb_dqrr_result
> rx_default_dqrr(struct qman_portal *portal,
> > count_ptr = this_cpu_ptr(dpaa_bp->percpu_count);
> > (*count_ptr)--;
> >
> > - if (likely(fd_format == qm_fd_contig))
> > + /* Extract the timestamp stored in the headroom before running
> XDP */
> > + if (priv->rx_tstamp) {
> > + if (!fman_port_get_tstamp(priv->mac_dev->port[RX], vaddr,
> &ns))
> > + ts_valid = true;
> > + else
> > + WARN_ONCE(1, "fman_port_get_tstamp failed!\n");
> > + }
> > +
> > + /* Extract the hash stored in the headroom before running XDP */
> > + if (net_dev->features & NETIF_F_RXHASH && priv->keygen_in_use
> &&
> > + !fman_port_get_hash_result_offset(priv->mac_dev->port[RX],
> > + &hash_offset)) {
> > + hash = be32_to_cpu(*(u32 *)(vaddr + hash_offset));
> > + hash_valid = true;
> > + }
> > +
> > + if (likely(fd_format == qm_fd_contig)) {
> > + xdp_act = dpaa_run_xdp(priv, (struct qm_fd *)fd, vaddr,
> > + &xdp_meta_len);
> > + if (xdp_act != XDP_PASS) {
> > + percpu_stats->rx_packets++;
> > + percpu_stats->rx_bytes += qm_fd_get_length(fd);
> > + return qman_cb_dqrr_consume;
> > + }
> > skb = contig_fd_to_skb(priv, fd);
> > - else
> > + } else {
> > + /* XDP doesn't support S/G frames. Return the fragments to
> the
> > + * buffer pool and release the SGT.
> > + */
> > + if (READ_ONCE(priv->xdp_prog)) {
> > + WARN_ONCE(1, "S/G frames not supported under
> XDP\n");
> > + sgt = vaddr + qm_fd_get_offset(fd);
> > + dpaa_release_sgt_members(sgt);
> > + free_pages((unsigned long)vaddr, 0);
> > + return qman_cb_dqrr_consume;
> > + }
> > skb = sg_fd_to_skb(priv, fd);
> > + }
> > if (!skb)
> > return qman_cb_dqrr_consume;
> >
> > - if (priv->rx_tstamp) {
> > + if (xdp_meta_len)
> > + skb_metadata_set(skb, xdp_meta_len);
> > +
> > + /* Set the previously extracted timestamp */
> > + if (ts_valid) {
> > shhwtstamps = skb_hwtstamps(skb);
> > memset(shhwtstamps, 0, sizeof(*shhwtstamps));
> > -
> > - if (!fman_port_get_tstamp(priv->mac_dev->port[RX], vaddr,
> &ns))
> > - shhwtstamps->hwtstamp = ns_to_ktime(ns);
> > - else
> > - dev_warn(net_dev->dev.parent,
> "fman_port_get_tstamp failed!\n");
> > + shhwtstamps->hwtstamp = ns_to_ktime(ns);
> > }
> >
> > skb->protocol = eth_type_trans(skb, net_dev);
> >
> > - if (net_dev->features & NETIF_F_RXHASH && priv->keygen_in_use
> &&
> > - !fman_port_get_hash_result_offset(priv->mac_dev->port[RX],
> > - &hash_offset)) {
> > + /* Set the previously extracted hash */
> > + if (hash_valid) {
> > enum pkt_hash_types type;
> >
> > /* if L4 exists, it was used in the hash generation */
> > type = be32_to_cpu(fd->status) & FM_FD_STAT_L4CV ?
> > PKT_HASH_TYPE_L4 : PKT_HASH_TYPE_L3;
> > - skb_set_hash(skb, be32_to_cpu(*(u32 *)(vaddr +
> hash_offset)),
> > - type);
> > + skb_set_hash(skb, hash, type);
> > }
> >
> > skb_len = skb->len;
> > @@ -2671,6 +2756,54 @@ static int dpaa_eth_stop(struct net_device
> *net_dev)
> > return err;
> > }
> >
> > +static int dpaa_setup_xdp(struct net_device *net_dev, struct bpf_prog
> *prog)
> > +{
> > + struct dpaa_priv *priv = netdev_priv(net_dev);
> > + struct bpf_prog *old_prog;
> > + int err, max_contig_data;
> > + bool up;
> > +
> > + max_contig_data = priv->dpaa_bp->size - priv->rx_headroom;
> > +
> > + /* S/G fragments are not supported in XDP-mode */
> > + if (prog &&
> > + (net_dev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN >
> max_contig_data)) {
> > + dev_warn(net_dev->dev.parent,
> > + "The maximum MTU for XDP is %d\n",
> > + max_contig_data - VLAN_ETH_HLEN -
> ETH_FCS_LEN);
> > + return -EINVAL;
> > + }
> > +
> > + up = netif_running(net_dev);
> > +
> > + if (up)
> > + dpaa_eth_stop(net_dev);
> > +
> > + old_prog = xchg(&priv->xdp_prog, prog);
> > + if (old_prog)
> > + bpf_prog_put(old_prog);
> > +
> > + if (up) {
> > + err = dpaa_open(net_dev);
> > + if (err) {
> > + dev_err(net_dev->dev.parent, "dpaa_open()
> failed\n");
> > + return err;
>
> So you decided not to take advantage of extack messages?
Sorry, I put more emphasis on reporting the messages than on the actual means to do it. I'll wait, if you have more comments, and respin.
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int dpaa_xdp(struct net_device *net_dev, struct netdev_bpf *xdp)
> > +{
> > + switch (xdp->command) {
> > + case XDP_SETUP_PROG:
> > + return dpaa_setup_xdp(net_dev, xdp->prog);
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > static int dpaa_ts_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> > {
> > struct dpaa_priv *priv = netdev_priv(dev);
> > @@ -2737,6 +2870,7 @@ static int dpaa_ioctl(struct net_device *net_dev,
> struct ifreq *rq, int cmd)
> > .ndo_set_rx_mode = dpaa_set_rx_mode,
> > .ndo_do_ioctl = dpaa_ioctl,
> > .ndo_setup_tc = dpaa_setup_tc,
> > + .ndo_bpf = dpaa_xdp,
> > };
> >
> > static int dpaa_napi_add(struct net_device *net_dev)
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
> > index da30e5d..94e8613 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
> > @@ -196,6 +196,8 @@ struct dpaa_priv {
> >
> > bool tx_tstamp; /* Tx timestamping enabled */
> > bool rx_tstamp; /* Rx timestamping enabled */
> > +
> > + struct bpf_prog *xdp_prog;
> > };
> >
> > /* from dpaa_ethtool.c */
> > --
> > 1.9.1
> >
^ permalink raw reply
* RE: [PATCH v4 2/4] net: socket: rework SIOC?IFMAP ioctls
From: David Laight @ 2020-11-24 16:13 UTC (permalink / raw)
To: 'Arnd Bergmann', netdev@vger.kernel.org; +Cc: Arnd Bergmann
In-Reply-To: <20201124151828.169152-3-arnd@kernel.org>
From: Arnd Bergmann
> Sent: 24 November 2020 15:18
>
> SIOCGIFMAP and SIOCSIFMAP currently require compat_alloc_user_space()
> and copy_in_user() for compat mode.
>
> Move the compat handling into the location where the structures are
> actually used, to avoid using those interfaces and get a clearer
> implementation.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> changes in v3:
> - complete rewrite
...
> include/linux/compat.h | 18 ++++++------
> net/core/dev_ioctl.c | 64 +++++++++++++++++++++++++++++++++---------
> net/socket.c | 39 ++-----------------------
> 3 files changed, 62 insertions(+), 59 deletions(-)
>
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index 08dbd34bb7a5..47496c5eb5eb 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -96,6 +96,15 @@ struct compat_iovec {
> compat_size_t iov_len;
> };
>
> +struct compat_ifmap {
> + compat_ulong_t mem_start;
> + compat_ulong_t mem_end;
> + unsigned short base_addr;
> + unsigned char irq;
> + unsigned char dma;
> + unsigned char port;
> +};
Isn't the only difference the number of pad bytes at the end?
If you don't copy these (in or out) then the compat version
isn't special at all.
Not copying the pad in or out would ensure you don't leak
kernel stack to userspace.
OTOH you may want to write the padding zero.
So a CT_ASSERT(offsetof (struct ifmap, port) == offsetof (struct compat_ifmap, port))
would suffice.
Maybe a CT_ASSERT_EQ_OFFSET(struct ifmap, struct compat_ifmap, port);
Would make the code easier to read.
Although you might want the version that adds an offset
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* RE: [PATCH v4 1/4] ethtool: improve compat ioctl handling
From: David Laight @ 2020-11-24 16:19 UTC (permalink / raw)
To: 'Arnd Bergmann', netdev@vger.kernel.org
Cc: Arnd Bergmann, Christoph Hellwig
In-Reply-To: <20201124151828.169152-2-arnd@kernel.org>
From: Arnd Bergmann
> Sent: 24 November 2020 15:18
>
> The ethtool compat ioctl handling is hidden away in net/socket.c,
> which introduces a couple of minor oddities:
>
...
> +
> +static int ethtool_rxnfc_copy_from_compat(struct ethtool_rxnfc *rxnfc,
> + const struct compat_ethtool_rxnfc __user *useraddr,
> + size_t size)
> +{
I think this (and possibly others) want a 'noinline_for_stack'.
So that both the normal and compat structures aren't both on the
stack when the real code is called.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: netconsole deadlock with virtnet
From: Jakub Kicinski @ 2020-11-24 16:20 UTC (permalink / raw)
To: Jason Wang
Cc: Steven Rostedt, Leon Romanovsky, Sergey Senozhatsky,
Michael S. Tsirkin, Petr Mladek, John Ogness, virtualization,
Amit Shah, Itay Aveksis, Ran Rozenstein, netdev
In-Reply-To: <1133f1a4-6772-8aa3-41dd-edbc1ee76cee@redhat.com>
On Tue, 24 Nov 2020 11:22:03 +0800 Jason Wang wrote:
> >> Perhaps you need the trylock in virtnet_poll_tx()?
> > That could work. Best if we used normal lock if !!budget, and trylock
> > when budget is 0. But maybe that's too hairy.
>
> If we use trylock, we probably lose(or delay) tx notification that may
> have side effects to the stack.
That's why I said only trylock with budget == 0. Only netpoll calls with
budget == 0, AFAIK.
> > I'm assuming all this trickiness comes from virtqueue_get_buf() needing
> > locking vs the TX path? It's pretty unusual for the completion path to
> > need locking vs xmit path.
>
> Two reasons for doing this:
>
> 1) For some historical reason, we try to free transmitted tx packets in
> xmit (see free_old_xmit_skbs() in start_xmit()), we can probably remove
> this if we remove the non tx interrupt mode.
> 2) virtio core requires virtqueue_get_buf() to be synchronized with
> virtqueue_add(), we probably can solve this but it requires some non
> trivial refactoring in the virtio core
>
> Btw, have a quick search, there are several other drivers that uses tx
> lock in the tx NAPI.
Unless they do:
netdev->priv_flags |= IFF_DISABLE_NETPOLL;
they are all broken.
^ permalink raw reply
* Re: [PATCH bpf-next v3 01/10] net: introduce preferred busy-polling
From: Jakub Kicinski @ 2020-11-24 16:21 UTC (permalink / raw)
To: Björn Töpel
Cc: netdev, bpf, Björn Töpel, magnus.karlsson, ast, daniel,
maciej.fijalkowski, sridhar.samudrala, jesse.brandeburg,
qi.z.zhang, edumazet, jonathan.lemon, maximmi
In-Reply-To: <20201119083024.119566-2-bjorn.topel@gmail.com>
On Thu, 19 Nov 2020 09:30:15 +0100 Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> The existing busy-polling mode, enabled by the SO_BUSY_POLL socket
> option or system-wide using the /proc/sys/net/core/busy_read knob, is
> an opportunistic. That means that if the NAPI context is not
> scheduled, it will poll it. If, after busy-polling, the budget is
> exceeded the busy-polling logic will schedule the NAPI onto the
> regular softirq handling.
>
> One implication of the behavior above is that a busy/heavy loaded NAPI
> context will never enter/allow for busy-polling. Some applications
> prefer that most NAPI processing would be done by busy-polling.
>
> This series adds a new socket option, SO_PREFER_BUSY_POLL, that works
> in concert with the napi_defer_hard_irqs and gro_flush_timeout
> knobs. The napi_defer_hard_irqs and gro_flush_timeout knobs were
> introduced in commit 6f8b12d661d0 ("net: napi: add hard irqs deferral
> feature"), and allows for a user to defer interrupts to be enabled and
> instead schedule the NAPI context from a watchdog timer. When a user
> enables the SO_PREFER_BUSY_POLL, again with the other knobs enabled,
> and the NAPI context is being processed by a softirq, the softirq NAPI
> processing will exit early to allow the busy-polling to be performed.
>
> If the application stops performing busy-polling via a system call,
> the watchdog timer defined by gro_flush_timeout will timeout, and
> regular softirq handling will resume.
>
> In summary; Heavy traffic applications that prefer busy-polling over
> softirq processing should use this option.
>
> Example usage:
>
> $ echo 2 | sudo tee /sys/class/net/ens785f1/napi_defer_hard_irqs
> $ echo 200000 | sudo tee /sys/class/net/ens785f1/gro_flush_timeout
>
> Note that the timeout should be larger than the userspace processing
> window, otherwise the watchdog will timeout and fall back to regular
> softirq processing.
>
> Enable the SO_BUSY_POLL/SO_PREFER_BUSY_POLL options on your socket.
>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
^ permalink raw reply
* Re: [PATCH bpf-next v3 02/10] net: add SO_BUSY_POLL_BUDGET socket option
From: Jakub Kicinski @ 2020-11-24 16:21 UTC (permalink / raw)
To: Björn Töpel
Cc: netdev, bpf, Björn Töpel, magnus.karlsson, ast, daniel,
maciej.fijalkowski, sridhar.samudrala, jesse.brandeburg,
qi.z.zhang, edumazet, jonathan.lemon, maximmi
In-Reply-To: <20201119083024.119566-3-bjorn.topel@gmail.com>
On Thu, 19 Nov 2020 09:30:16 +0100 Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> This option lets a user set a per socket NAPI budget for
> busy-polling. If the options is not set, it will use the default of 8.
>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
^ permalink raw reply
* Re: [PATCH] hv_netvsc: Validate number of allocated sub-channels
From: Wei Liu @ 2020-11-24 16:22 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrea Parri (Microsoft), linux-kernel, K . Y . Srinivasan,
Haiyang Zhang, Stephen Hemminger, Wei Liu, linux-hyperv,
Michael Kelley, Juan Vazquez, Saruhan Karademir, David S. Miller,
netdev
In-Reply-To: <20201118173715.60b5a8f2@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
On Wed, Nov 18, 2020 at 05:37:15PM -0800, Jakub Kicinski wrote:
> On Wed, 18 Nov 2020 16:33:10 +0100 Andrea Parri (Microsoft) wrote:
> > Lack of validation could lead to out-of-bound reads and information
> > leaks (cf. usage of nvdev->chan_table[]). Check that the number of
> > allocated sub-channels fits into the expected range.
> >
> > Suggested-by: Saruhan Karademir <skarade@microsoft.com>
> > Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: netdev@vger.kernel.org
>
> Acked-by: Jakub Kicinski <kuba@kernel.org>
Applied to hyperv-next.
^ permalink raw reply
* Re: [PATCH net-next] net: sched: alias action flags with TCA_ACT_ prefix
From: Jakub Kicinski @ 2020-11-24 16:24 UTC (permalink / raw)
To: Vlad Buslov; +Cc: netdev, davem, jhs, xiyou.wangcong, jiri
In-Reply-To: <87v9dv9k8q.fsf@buslov.dev>
On Tue, 24 Nov 2020 11:28:37 +0200 Vlad Buslov wrote:
> > TCA_FLAG_TERSE_DUMP exists only in net-next, we could rename it, right?
>
> You are right. I'll send a fix.
You mean v2, not a follow up, right? :)
^ permalink raw reply
* [PATCH net-next] mptcp: put reference in mptcp timeout timer
From: Florian Westphal @ 2020-11-24 16:24 UTC (permalink / raw)
To: netdev; +Cc: mptcp, Florian Westphal, Paolo Abeni, Davide Caratti
On close this timer might be scheduled. mptcp uses sk_reset_timer for
this, so the a reference on the mptcp socket is taken.
This causes a refcount leak which can for example be reproduced
with 'mp_join_server_v4.pkt' from the mptcp-packetdrill repo.
The leak has nothing to do with join requests, v1_mp_capable_bind_no_cs.pkt
works too when replacing the last ack mpcapable to v1 instead of v0.
unreferenced object 0xffff888109bba040 (size 2744):
comm "packetdrill", [..]
backtrace:
[..] sk_prot_alloc.isra.0+0x2b/0xc0
[..] sk_clone_lock+0x2f/0x740
[..] mptcp_sk_clone+0x33/0x1a0
[..] subflow_syn_recv_sock+0x2b1/0x690 [..]
Fixes: e16163b6e2b7 ("mptcp: refactor shutdown and close")
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/mptcp/protocol.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 4b7794835fea..dc979571f561 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1710,6 +1710,7 @@ static void mptcp_timeout_timer(struct timer_list *t)
struct sock *sk = from_timer(sk, t, sk_timer);
mptcp_schedule_work(sk);
+ sock_put(sk);
}
/* Find an idle subflow. Return NULL if there is unacked data at tcp
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v2 1/1] ath10k: add option for chip-id based BDF selection
From: Doug Anderson @ 2020-11-24 16:26 UTC (permalink / raw)
To: Rakesh Pillai
Cc: Abhishek Kumar, Kalle Valo, LKML, ath10k, Brian Norris,
linux-wireless, David S. Miller, Jakub Kicinski, netdev
In-Reply-To: <002401d6c242$d78f2140$86ad63c0$@codeaurora.org>
Hi,
On Tue, Nov 24, 2020 at 1:19 AM Rakesh Pillai <pillair@codeaurora.org> wrote:
>
> > -----Original Message-----
> > From: Doug Anderson <dianders@chromium.org>
> > Sent: Tuesday, November 24, 2020 6:27 AM
> > To: Abhishek Kumar <kuabhs@chromium.org>
> > Cc: Kalle Valo <kvalo@codeaurora.org>; Rakesh Pillai
> > <pillair@codeaurora.org>; LKML <linux-kernel@vger.kernel.org>; ath10k
> > <ath10k@lists.infradead.org>; Brian Norris <briannorris@chromium.org>;
> > linux-wireless <linux-wireless@vger.kernel.org>; David S. Miller
> > <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; netdev
> > <netdev@vger.kernel.org>
> > Subject: Re: [PATCH v2 1/1] ath10k: add option for chip-id based BDF
> > selection
> >
> > Hi,
> >
> > On Thu, Nov 12, 2020 at 12:09 PM Abhishek Kumar <kuabhs@chromium.org>
> > wrote:
> > >
> > > In some devices difference in chip-id should be enough to pick
> > > the right BDF. Add another support for chip-id based BDF selection.
> > > With this new option, ath10k supports 2 fallback options.
> > >
> > > The board name with chip-id as option looks as follows
> > > board name 'bus=snoc,qmi-board-id=ff,qmi-chip-id=320'
> > >
> > > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.2.2-00696-QCAHLSWMTPL-1
> > > Tested-on: QCA6174 HW3.2 WLAN.RM.4.4.1-00157-QCARMSWPZ-1
> > > Signed-off-by: Abhishek Kumar <kuabhs@chromium.org>
> > > ---
> > >
> > > (no changes since v1)
> >
> > I think you need to work on the method you're using to generate your
> > patches. There are most definitely changes since v1. You described
> > them in your cover letter (which you don't really need for a singleton
> > patch) instead of here.
> >
> >
> > > @@ -1438,12 +1439,17 @@ static int
> > ath10k_core_create_board_name(struct ath10k *ar, char *name,
> > > }
> > >
> > > if (ar->id.qmi_ids_valid) {
> > > - if (with_variant && ar->id.bdf_ext[0] != '\0')
> > > + if (with_additional_params && ar->id.bdf_ext[0] != '\0')
> > > scnprintf(name, name_len,
> > > "bus=%s,qmi-board-id=%x,qmi-chip-id=%x%s",
> > > ath10k_bus_str(ar->hif.bus),
> > > ar->id.qmi_board_id, ar->id.qmi_chip_id,
> > > variant);
> > > + else if (with_additional_params)
> > > + scnprintf(name, name_len,
> > > + "bus=%s,qmi-board-id=%x,qmi-chip-id=%x",
> > > + ath10k_bus_str(ar->hif.bus),
> > > + ar->id.qmi_board_id, ar->id.qmi_chip_id);
> >
> > I believe this is exactly opposite of what Rakesh was requesting.
> > Specifically, he was trying to eliminate the extra scnprintf() but I
> > think he still agreed that it was a good idea to generate 3 different
> > strings. I believe the proper diff to apply to v1 is:
> >
> > https://crrev.com/c/255643
Wow, I seem to have deleted the last digit from my URL. Should have been:
https://crrev.com/c/2556437
> >
> > -Doug
>
> Hi Abhishek/Doug,
>
> I missed on reviewing this change. Also I agree with Doug that this is not the change I was looking for.
>
> The argument "with_variant" can be renamed to "with_extra_params". There is no need for any new argument to this function.
> Case 1: with_extra_params=0, ar->id.bdf_ext[0] = 0 -> The default name will be used (bus=snoc,qmi_board_id=0xab)
> Case 2: with_extra_params=1, ar->id.bdf_ext[0] = 0 -> bus=snoc,qmi_board_id=0xab,qmi_chip_id=0xcd
> Case 3: with_extra_params=1, ar->id.bdf_ext[0] = "xyz" -> bus=snoc,qmi_board_id=0xab,qmi_chip_id=0xcd,variant=xyz
>
> ar->id.bdf_ext[0] depends on the DT entry for variant field.
I'm confused about your suggestion. Maybe you can help clarify. Are
you suggesting:
a) Only two calls to ath10k_core_create_board_name()
I'm pretty sure this will fail in some cases. Specifically consider
the case where the device tree has a "variant" defined but the BRD
file only has one entry for (board-id) and one for (board-id +
chip-id) but no entry for (board-id + chip-id + variant). If you are
only making two calls then I don't think you'll pick the right one.
Said another way...
If the device tree has a variant:
1. We should prefer a BRD entry that has board-id + chip-id + variant
2. If #1 isn't there, we should prefer a BRD entry that has board-id + chip-id
3. If #1 and #2 aren't there we fall back to a BRD entry that has board-id.
...without 3 calls to ath10k_core_create_board_name() we can't handle
all 3 cases.
b) Three calls to ath10k_core_create_board_name() but the caller
manually whacks "ar->id.bdf_ext[0]" for one of the calls
This doesn't look like it's a clean solution, but maybe I'm missing something.
-Doug
^ permalink raw reply
* Re: [PATCH v5 0/9] "Task_isolation" mode
From: Tom Rix @ 2020-11-24 16:36 UTC (permalink / raw)
To: Alex Belits, nitesh@redhat.com, frederic@kernel.org
Cc: Prasun Kapoor, linux-api@vger.kernel.org, davem@davemloft.net,
mingo@kernel.org, catalin.marinas@arm.com, rostedt@goodmis.org,
linux-kernel@vger.kernel.org, peterx@redhat.com,
tglx@linutronix.de, linux-arch@vger.kernel.org,
mtosatti@redhat.com, will@kernel.org, peterz@infradead.org,
leon@sidebranch.com, linux-arm-kernel@lists.infradead.org,
pauld@redhat.com, netdev@vger.kernel.org
In-Reply-To: <8d887e59ca713726f4fcb25a316e1e932b02823e.camel@marvell.com>
On 11/23/20 9:42 AM, Alex Belits wrote:
> This is an update of task isolation work that was originally done by
> Chris Metcalf <cmetcalf@mellanox.com> and maintained by him until
> November 2017. It is adapted to the current kernel and cleaned up to
> implement its functionality in a more complete and cleaner manner.
I am having problems applying the patchset to today's linux-next.
Which kernel should I be using ?
Thanks,
Tom
^ permalink raw reply
* Re: pull-request: wireless-drivers-2020-11-23
From: Kalle Valo @ 2020-11-24 16:38 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, linux-wireless
In-Reply-To: <20201124080858.0aa8462b@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Jakub Kicinski <kuba@kernel.org> writes:
> On Tue, 24 Nov 2020 09:15:45 +0200 Kalle Valo wrote:
>> Jakub Kicinski <kuba@kernel.org> writes:
>>
>> > On Mon, 23 Nov 2020 16:10:37 +0000 (UTC) Kalle Valo wrote:
>> >> wireless-drivers fixes for v5.10
>> >>
>> >> First set of fixes for v5.10. One fix for iwlwifi kernel panic, others
>> >> less notable.
>> >>
>> >> rtw88
>> >>
>> >> * fix a bogus test found by clang
>> >>
>> >> iwlwifi
>> >>
>> >> * fix long memory reads causing soft lockup warnings
>> >>
>> >> * fix kernel panic during Channel Switch Announcement (CSA)
>> >>
>> >> * other smaller fixes
>> >>
>> >> MAINTAINERS
>> >>
>> >> * email address updates
>> >
>> > Pulled, thanks!
>> >
>> > Please watch out for missing sign-offs.
>>
>> I assume you refer to commit 97cc16943f23, sorry about that. Currently
>> I'm just manually checking sign-offs and missed this patch. My plan is
>> to implement proper checks to my patchwork script so I'll notice these
>> before I commit the patch (or pull request), just have not yet find the
>> time to do that.
>
> Check out verify_fixes and verify_signoff in Greg's repo:
>
> https://github.com/gregkh/gregkh-linux/tree/master/work
Thanks, I will.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] usbnet: ipheth: fix connectivity with iOS 14
From: Jakub Kicinski @ 2020-11-24 16:39 UTC (permalink / raw)
To: Yves-Alexis Perez
Cc: David S. Miller, Martin Habets, Luc Van Oostenryck,
Shannon Nelson, Michael S. Tsirkin, linux-usb, netdev,
linux-kernel, Matti Vuorela, stable
In-Reply-To: <02c032512dab22c1ab758d953affd94a4064fdbd.camel@corsac.net>
On Tue, 24 Nov 2020 11:41:40 +0100 Yves-Alexis Perez wrote:
> On Sat, 2020-11-21 at 14:03 -0800, Jakub Kicinski wrote:
> > Applied to net with the typo fixed, thanks!
>
> Thanks!
>
> Is there any chance it'll be in 5.10 or will it have to wait for the 5.11
> merge window?
It'll be in 5.10-rc6.
> Also it should be applied to all supported/stable kernels. I guess that'll
> have to wait until it's in Linus tree according [1] to but I'm unsure if I
> need to trigger the action myself or if Greg (or Dave, according to [2]) will
> do it.
Dave (or someone helping him, like myself) will do it, probably around
the time 5.10-rc6 is released.
> I looked at [3] and it seems that adding the CC: stable in my commit message
> maybe was an error because it's marked as a Failure, so if there's anything
> needed from me here, don't hesitate to ask.
No worries, I stripped the CC and put the patch in Dave's stable queue.
^ permalink raw reply
* Re: [PATCH net-next] net: sched: alias action flags with TCA_ACT_ prefix
From: Vlad Buslov @ 2020-11-24 16:39 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, jhs, xiyou.wangcong, jiri
In-Reply-To: <20201124082448.56a03de5@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Tue 24 Nov 2020 at 18:24, Jakub Kicinski <kuba@kernel.org> wrote:
> On Tue, 24 Nov 2020 11:28:37 +0200 Vlad Buslov wrote:
>> > TCA_FLAG_TERSE_DUMP exists only in net-next, we could rename it, right?
>>
>> You are right. I'll send a fix.
>
> You mean v2, not a follow up, right? :)
Yes. Sending the v2.
^ permalink raw reply
* [PATCH net-next v2] net: sched: alias action flags with TCA_ACT_ prefix
From: Vlad Buslov @ 2020-11-24 16:40 UTC (permalink / raw)
To: netdev, davem, kuba; +Cc: jhs, xiyou.wangcong, jiri, Vlad Buslov
Currently both filter and action flags use same "TCA_" prefix which makes
them hard to distinguish to code and confusing for users. Create aliases
for existing action flags constants with "TCA_ACT_" prefix.
Signed-off-by: Vlad Buslov <vlad@buslov.dev>
---
Notes:
Changes V1 -> V2:
- Removed old-style alias for terse dump flag.
include/uapi/linux/rtnetlink.h | 12 +++++++-----
net/sched/act_api.c | 10 +++++-----
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 2ffbef5da6c1..b841caa4657e 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -768,16 +768,18 @@ enum {
#define TA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct tcamsg))
/* tcamsg flags stored in attribute TCA_ROOT_FLAGS
*
- * TCA_FLAG_LARGE_DUMP_ON user->kernel to request for larger than TCA_ACT_MAX_PRIO
- * actions in a dump. All dump responses will contain the number of actions
- * being dumped stored in for user app's consumption in TCA_ROOT_COUNT
+ * TCA_ACT_FLAG_LARGE_DUMP_ON user->kernel to request for larger than
+ * TCA_ACT_MAX_PRIO actions in a dump. All dump responses will contain the
+ * number of actions being dumped stored in for user app's consumption in
+ * TCA_ROOT_COUNT
*
- * TCA_FLAG_TERSE_DUMP user->kernel to request terse (brief) dump that only
+ * TCA_ACT_FLAG_TERSE_DUMP user->kernel to request terse (brief) dump that only
* includes essential action info (kind, index, etc.)
*
*/
#define TCA_FLAG_LARGE_DUMP_ON (1 << 0)
-#define TCA_FLAG_TERSE_DUMP (1 << 1)
+#define TCA_ACT_FLAG_LARGE_DUMP_ON TCA_FLAG_LARGE_DUMP_ON
+#define TCA_ACT_FLAG_TERSE_DUMP (1 << 1)
/* New extended info filters for IFLA_EXT_MASK */
#define RTEXT_FILTER_VF (1 << 0)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index fc23f46a315c..99db1c77426b 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -278,7 +278,7 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
index--;
goto nla_put_failure;
}
- err = (act_flags & TCA_FLAG_TERSE_DUMP) ?
+ err = (act_flags & TCA_ACT_FLAG_TERSE_DUMP) ?
tcf_action_dump_terse(skb, p, true) :
tcf_action_dump_1(skb, p, 0, 0);
if (err < 0) {
@@ -288,7 +288,7 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
}
nla_nest_end(skb, nest);
n_i++;
- if (!(act_flags & TCA_FLAG_LARGE_DUMP_ON) &&
+ if (!(act_flags & TCA_ACT_FLAG_LARGE_DUMP_ON) &&
n_i >= TCA_ACT_MAX_PRIO)
goto done;
}
@@ -298,7 +298,7 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
mutex_unlock(&idrinfo->lock);
if (n_i) {
- if (act_flags & TCA_FLAG_LARGE_DUMP_ON)
+ if (act_flags & TCA_ACT_FLAG_LARGE_DUMP_ON)
cb->args[1] = n_i;
}
return n_i;
@@ -1473,8 +1473,8 @@ static int tcf_action_add(struct net *net, struct nlattr *nla,
}
static const struct nla_policy tcaa_policy[TCA_ROOT_MAX + 1] = {
- [TCA_ROOT_FLAGS] = NLA_POLICY_BITFIELD32(TCA_FLAG_LARGE_DUMP_ON |
- TCA_FLAG_TERSE_DUMP),
+ [TCA_ROOT_FLAGS] = NLA_POLICY_BITFIELD32(TCA_ACT_FLAG_LARGE_DUMP_ON |
+ TCA_ACT_FLAG_TERSE_DUMP),
[TCA_ROOT_TIME_DELTA] = { .type = NLA_U32 },
};
--
2.29.1
^ permalink raw reply related
* Re: [PATCH net-next] mptcp: put reference in mptcp timeout timer
From: Paolo Abeni @ 2020-11-24 16:42 UTC (permalink / raw)
To: Florian Westphal, netdev; +Cc: mptcp, Davide Caratti
In-Reply-To: <20201124162446.11448-1-fw@strlen.de>
On Tue, 2020-11-24 at 17:24 +0100, Florian Westphal wrote:
> On close this timer might be scheduled. mptcp uses sk_reset_timer for
> this, so the a reference on the mptcp socket is taken.
>
> This causes a refcount leak which can for example be reproduced
> with 'mp_join_server_v4.pkt' from the mptcp-packetdrill repo.
Whoops, my fault!
> The leak has nothing to do with join requests, v1_mp_capable_bind_no_cs.pkt
> works too when replacing the last ack mpcapable to v1 instead of v0.
>
> unreferenced object 0xffff888109bba040 (size 2744):
> comm "packetdrill", [..]
> backtrace:
> [..] sk_prot_alloc.isra.0+0x2b/0xc0
> [..] sk_clone_lock+0x2f/0x740
> [..] mptcp_sk_clone+0x33/0x1a0
> [..] subflow_syn_recv_sock+0x2b1/0x690 [..]
>
> Fixes: e16163b6e2b7 ("mptcp: refactor shutdown and close")
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Davide Caratti <dcaratti@redhat.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> net/mptcp/protocol.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 4b7794835fea..dc979571f561 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1710,6 +1710,7 @@ static void mptcp_timeout_timer(struct timer_list *t)
> struct sock *sk = from_timer(sk, t, sk_timer);
>
> mptcp_schedule_work(sk);
> + sock_put(sk);
> }
>
> /* Find an idle subflow. Return NULL if there is unacked data at tcp
LGTM, thanks!
Acked-by: Paolo Abeni <pabeni@redhat.com>
^ permalink raw reply
* Re: [PATCH net] net: stmmac: Use hrtimer for TX coalescing
From: Jakub Kicinski @ 2020-11-24 16:50 UTC (permalink / raw)
To: Vincent Whitchurch
Cc: peppe.cavallaro@st.com, alexandre.torgue@st.com,
joabreu@synopsys.com, davem@davemloft.net, kernel,
netdev@vger.kernel.org
In-Reply-To: <20201124041127.c3uagg4cguqqvgji@axis.com>
On Tue, 24 Nov 2020 05:11:27 +0100 Vincent Whitchurch wrote:
> On Tue, Nov 24, 2020 at 01:46:00AM +0100, Jakub Kicinski wrote:
> > On Fri, 20 Nov 2020 16:02:08 +0100 Vincent Whitchurch wrote:
> > > This driver uses a normal timer for TX coalescing, which means that the
> > > with the default tx-usecs of 1000 microseconds the cleanups actually
> > > happen 10 ms or more later with HZ=100. This leads to very low
> > > througput with TCP when bridged to a slow link such as a 4G modem. Fix
> > > this by using an hrtimer instead.
> > >
> > > On my ARM platform with HZ=100 and the default TX coalescing settings
> > > (tx-frames 25 tx-usecs 1000), with "tc qdisc add dev eth0 root netem
> > > delay 60ms 40ms rate 50Mbit" run on the server, netperf's TCP_STREAM
> > > improves from ~5.5 Mbps to ~100 Mbps.
> > >
> > > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> >
> > Looks perfectly reasonable, but you marked it for net. Do you consider
> > this to be a bug fix, and need it backported to stable? Otherwise I'd
> > rather apply it to net-next.
>
> No, sorry, I think a backport to stable is unnecessary. It should be
> fine to apply it to net-next. Thanks.
Applied to net-next, thank you!
^ permalink raw reply
* Re: [PATCH bpf-next V7 8/8] bpf/selftests: activating bpf_check_mtu BPF-helper
From: Yonghong Song @ 2020-11-24 16:50 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Andrii Nakryiko
Cc: bpf, Networking, Daniel Borkmann, Alexei Starovoitov,
Maciej Żenczykowski, Lorenz Bauer, shaun, Lorenzo Bianconi,
Marek Majkowski, John Fastabend, Jakub Kicinski, eyal.birger,
colrack
In-Reply-To: <20201124153301.47abc09c@carbon>
On 11/24/20 6:33 AM, Jesper Dangaard Brouer wrote:
> On Fri, 20 Nov 2020 23:41:11 -0800
> Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
>> On Fri, Nov 20, 2020 at 8:21 AM Jesper Dangaard Brouer
>> <brouer@redhat.com> wrote:
>>>
>>> Adding selftest for BPF-helper bpf_check_mtu(). Making sure
>>> it can be used from both XDP and TC.
>>>
>>> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>>> ---
>>> tools/testing/selftests/bpf/prog_tests/check_mtu.c | 37 ++++++++++++++++++++
>>> tools/testing/selftests/bpf/progs/test_check_mtu.c | 33 ++++++++++++++++++
>>> 2 files changed, 70 insertions(+)
>>> create mode 100644 tools/testing/selftests/bpf/prog_tests/check_mtu.c
>>> create mode 100644 tools/testing/selftests/bpf/progs/test_check_mtu.c
>>>
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/check_mtu.c b/tools/testing/selftests/bpf/prog_tests/check_mtu.c
>>> new file mode 100644
>>> index 000000000000..09b8f986a17b
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/bpf/prog_tests/check_mtu.c
>>> @@ -0,0 +1,37 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/* Copyright (c) 2020 Red Hat */
>>> +#include <uapi/linux/bpf.h>
>>> +#include <linux/if_link.h>
>>> +#include <test_progs.h>
>>> +
>>> +#include "test_check_mtu.skel.h"
>>> +#define IFINDEX_LO 1
>>> +
>>> +void test_check_mtu_xdp(struct test_check_mtu *skel)
>>
>> this should be static func, otherwise it's treated as an independent selftest.
>
> Ok, fixed.
>
>>> +{
>>> + int err = 0;
>>> + int fd;
>>> +
>>> + fd = bpf_program__fd(skel->progs.xdp_use_helper);
>>> + err = bpf_set_link_xdp_fd(IFINDEX_LO, fd, XDP_FLAGS_SKB_MODE);
>>> + if (CHECK_FAIL(err))
>>
>> please use CHECK() or one of ASSERT_xxx() helpers. CHECK_FAIL() should
>> be used for high-volume unlikely to fail test (i.e., very rarely).
>
> I could not get CHECK() macro working. I now realize that this is
> because I've not defined a global static variable named 'duration'.
>
> static __u32 duration;
>
> I wonder, are there any best-practice documentation or blogpost on
> howto write these bpf-selftests?
The 'duration' in old days is used to measure performance. Today most
of selftests actually do not need this. We do not have doc/blogpost
for this. The best is to look at other files under prog_tests to see
how they handle duration ...
>
>
> Below signature is the compile error for others to Google for, and
> solution above.
> -
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
>
>
> $ make
> TEST-OBJ [test_progs] check_mtu.test.o
> In file included from /home/jbrouer/git/kernel/bpf-next/tools/testing/selftests/bpf/prog_tests/check_mtu.c:6:
> /home/jbrouer/git/kernel/bpf-next/tools/testing/selftests/bpf/prog_tests/check_mtu.c: In function ‘test_check_mtu’:
> ./test_progs.h:129:25: error: ‘duration’ undeclared (first use in this function)
> 129 | _CHECK(condition, tag, duration, format)
> | ^~~~~~~~
> ./test_progs.h:111:25: note: in definition of macro ‘_CHECK’
> 111 | __func__, tag, duration); \
> | ^~~~~~~~
> /home/jbrouer/git/kernel/bpf-next/tools/testing/selftests/bpf/prog_tests/check_mtu.c:33:6: note: in expansion of macro ‘CHECK’
> 33 | if (CHECK(!skel, "open and load skel", "failed"))
> | ^~~~~
> ./test_progs.h:129:25: note: each undeclared identifier is reported only once for each function it appears in
> 129 | _CHECK(condition, tag, duration, format)
> | ^~~~~~~~
> ./test_progs.h:111:25: note: in definition of macro ‘_CHECK’
> 111 | __func__, tag, duration); \
> | ^~~~~~~~
> /home/jbrouer/git/kernel/bpf-next/tools/testing/selftests/bpf/prog_tests/check_mtu.c:33:6: note: in expansion of macro ‘CHECK’
> 33 | if (CHECK(!skel, "open and load skel", "failed"))
> | ^~~~~
> make: *** [Makefile:396: /home/jbrouer/git/kernel/bpf-next/tools/testing/selftests/bpf/check_mtu.test.o] Error 1
>
^ permalink raw reply
* [net-next v2 0/3][pull request] 40GbE Intel Wired LAN Driver Updates 2020-11-24
From: Tony Nguyen @ 2020-11-24 16:52 UTC (permalink / raw)
To: davem, kuba; +Cc: Tony Nguyen, netdev, sassmann
This series contains updates to i40e and igbvf drivers.
Marek removes a redundant assignment for i40e.
Stefan Assmann corrects reporting of VF link speed for i40e.
Karen revises a couple of error messages to warnings for igbvf as they
could be misinterpreted as issues when they are not.
v2: Dropped PTP patch as it's being updated.
The following are changes since commit 5112cf59d76d799b1c4d66af92417e2673fb1d5b:
sctp: Fix some typo
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 40GbE
Karen Sornek (1):
igbvf: Refactor traces
Marek Majtyka (1):
i40e: remove redundant assignment
Stefan Assmann (1):
i40e: report correct VF link speed when link state is set to enable
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 5 +++--
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 1 -
drivers/net/ethernet/intel/igbvf/netdev.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
--
2.26.2
^ permalink raw reply
* [net-next v2 1/3] i40e: remove redundant assignment
From: Tony Nguyen @ 2020-11-24 16:52 UTC (permalink / raw)
To: davem, kuba
Cc: Marek Majtyka, netdev, sassmann, anthony.l.nguyen,
Björn Töpel, George Kuruvinakunnel
In-Reply-To: <20201124165245.2844118-1-anthony.l.nguyen@intel.com>
From: Marek Majtyka <marekx.majtyka@intel.com>
Remove a redundant assignment of the software ring pointer in the i40e
driver. The variable is assigned twice with no use in between, so just
get rid of the first occurrence.
Fixes: 3b4f0b66c2b3 ("i40e, xsk: Migrate to new MEM_TYPE_XSK_BUFF_POOL")
Signed-off-by: Marek Majtyka <marekx.majtyka@intel.com>
Acked-by: Björn Töpel <bjorn.topel@intel.com>
Tested-by: George Kuruvinakunnel <george.kuruvinakunnel@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 567fd67e900e..67febc7b6798 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -311,7 +311,6 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
continue;
}
- bi = i40e_rx_bi(rx_ring, rx_ring->next_to_clean);
size = (qword & I40E_RXD_QW1_LENGTH_PBUF_MASK) >>
I40E_RXD_QW1_LENGTH_PBUF_SHIFT;
if (!size)
--
2.26.2
^ permalink raw reply related
* [net-next v2 2/3] i40e: report correct VF link speed when link state is set to enable
From: Tony Nguyen @ 2020-11-24 16:52 UTC (permalink / raw)
To: davem, kuba
Cc: Stefan Assmann, netdev, sassmann, anthony.l.nguyen, Aaron Brown
In-Reply-To: <20201124165245.2844118-1-anthony.l.nguyen@intel.com>
From: Stefan Assmann <sassmann@kpanic.de>
When the virtual link state was set to "enable" ethtool would report
link speed as 40000Mb/s regardless of the underlying device.
Report the correct link speed.
Example from a XXV710 NIC.
Before:
$ ip link set ens3f0 vf 0 state auto
$ ethtool enp8s2 | grep Speed
Speed: 25000Mb/s
$ ip link set ens3f0 vf 0 state enable
$ ethtool enp8s2 | grep Speed
Speed: 40000Mb/s
After:
$ ip link set ens3f0 vf 0 state auto
$ ethtool enp8s2 | grep Speed
Speed: 25000Mb/s
$ ip link set ens3f0 vf 0 state enable
$ ethtool enp8s2 | grep Speed
Speed: 25000Mb/s
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 4919d22d7b6b..2532f8eed6a8 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -63,7 +63,7 @@ static void i40e_vc_notify_vf_link_state(struct i40e_vf *vf)
} else if (vf->link_forced) {
pfe.event_data.link_event.link_status = vf->link_up;
pfe.event_data.link_event.link_speed =
- (vf->link_up ? VIRTCHNL_LINK_SPEED_40GB : 0);
+ (vf->link_up ? i40e_virtchnl_link_speed(ls->link_speed) : 0);
} else {
pfe.event_data.link_event.link_status =
ls->link_info & I40E_AQ_LINK_UP;
@@ -4437,6 +4437,7 @@ int i40e_ndo_set_vf_link_state(struct net_device *netdev, int vf_id, int link)
{
struct i40e_netdev_priv *np = netdev_priv(netdev);
struct i40e_pf *pf = np->vsi->back;
+ struct i40e_link_status *ls = &pf->hw.phy.link_info;
struct virtchnl_pf_event pfe;
struct i40e_hw *hw = &pf->hw;
struct i40e_vf *vf;
@@ -4474,7 +4475,7 @@ int i40e_ndo_set_vf_link_state(struct net_device *netdev, int vf_id, int link)
vf->link_forced = true;
vf->link_up = true;
pfe.event_data.link_event.link_status = true;
- pfe.event_data.link_event.link_speed = VIRTCHNL_LINK_SPEED_40GB;
+ pfe.event_data.link_event.link_speed = i40e_virtchnl_link_speed(ls->link_speed);
break;
case IFLA_VF_LINK_STATE_DISABLE:
vf->link_forced = true;
--
2.26.2
^ permalink raw reply related
* [net-next v2 3/3] igbvf: Refactor traces
From: Tony Nguyen @ 2020-11-24 16:52 UTC (permalink / raw)
To: davem, kuba
Cc: Karen Sornek, netdev, sassmann, anthony.l.nguyen,
Aleksandr Loktionov, Aaron Brown
In-Reply-To: <20201124165245.2844118-1-anthony.l.nguyen@intel.com>
From: Karen Sornek <karen.sornek@intel.com>
Refactoring "PF still resetting" and changing "Failed
to add vlan id" to "Vlan id is not added"
messages because previous version looked like a bug
- it informed about changes that worked as
designed but might confuse users
Signed-off-by: Karen Sornek <karen.sornek@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/igbvf/netdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index ee9f8c1dca83..30fdea24e94a 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -1236,7 +1236,7 @@ static int igbvf_vlan_rx_add_vid(struct net_device *netdev,
spin_lock_bh(&hw->mbx_lock);
if (hw->mac.ops.set_vfta(hw, vid, true)) {
- dev_err(&adapter->pdev->dev, "Failed to add vlan id %d\n", vid);
+ dev_warn(&adapter->pdev->dev, "Vlan id %d\n is not added", vid);
spin_unlock_bh(&hw->mbx_lock);
return -EINVAL;
}
@@ -1520,7 +1520,7 @@ static void igbvf_reset(struct igbvf_adapter *adapter)
/* Allow time for pending master requests to run */
if (mac->ops.reset_hw(hw))
- dev_err(&adapter->pdev->dev, "PF still resetting\n");
+ dev_warn(&adapter->pdev->dev, "PF still resetting\n");
mac->ops.init_hw(hw);
--
2.26.2
^ 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