* Re: [PATCH bpf-next] bpf: add skb->queue_mapping write access from tc clsact
From: Daniel Borkmann @ 2019-02-19 11:46 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <155057184028.20935.11848804617158437103.stgit@firesoul>
On 02/19/2019 11:24 AM, Jesper Dangaard Brouer wrote:
> The skb->queue_mapping already have read access, via __sk_buff->queue_mapping.
>
> This patch allow BPF tc qdisc clsact write access to the queue_mapping via
> tc_cls_act_is_valid_access.
>
> It is already possible to change this via TC filter action skbedit
> tc-skbedit(8). Due to the lack of TC examples, lets show one:
>
> # tc qdisc add dev ixgbe1 handle ffff: ingress
> # tc filter add dev ixgbe1 parent ffff: matchall action skbedit queue_mapping 5
> # tc filter list dev ixgbe1 parent ffff:
Using handles was in the old days, if we add examples, then lets do
something more user friendly ;)
# tc qdisc add dev ixgbe1 clsact
# tc filter replace dev ixgbe1 ingress matchall action skbedit queue_mapping 5
# tc filter list dev ixgbe1 ingress
> The most common mistake is that XPS (Transmit Packet Steering) takes
> precedence over setting skb->queue_mapping. XPS is configured per DEVICE
> via /sys/class/net/DEVICE/queues/tx-*/xps_cpus via a CPU hex mask. To
> disable set mask=00.
>
> The purpose of changing skb->queue_mapping is to influence the selection of
> the net_device "txq" (struct netdev_queue), which influence selection of
> the qdisc "root_lock" (via txq->qdisc->q.lock) and txq->_xmit_lock. When
> using the MQ qdisc the txq->qdisc points to different qdiscs and associated
> locks, and HARD_TX_LOCK (txq->_xmit_lock), allowing for CPU scalability.
>
> Due to lack of TC examples, lets show howto attach clsact BPF programs:
>
> # tc qdisc add dev ixgbe2 clsact
> # tc filter replace dev ixgbe2 egress bpf da obj XXX_kern.o sec tc_qmap2cpu
> # tc filter list dev ixgbe2 egress
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> net/core/filter.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 353735575204..d05ae8d05397 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -6238,6 +6238,7 @@ static bool tc_cls_act_is_valid_access(int off, int size,
> case bpf_ctx_range(struct __sk_buff, tc_classid):
> case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
> case bpf_ctx_range(struct __sk_buff, tstamp):
> + case bpf_ctx_range(struct __sk_buff, queue_mapping):
> break;
> default:
> return false;
> @@ -6642,9 +6643,16 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
> break;
>
> case offsetof(struct __sk_buff, queue_mapping):
> - *insn++ = BPF_LDX_MEM(BPF_H, si->dst_reg, si->src_reg,
> - bpf_target_off(struct sk_buff, queue_mapping, 2,
> - target_size));
> + if (type == BPF_WRITE)
> + *insn++ = BPF_STX_MEM(BPF_H, si->dst_reg, si->src_reg,
> + bpf_target_off(struct sk_buff,
> + queue_mapping,
> + 2, target_size));
> + else
> + *insn++ = BPF_LDX_MEM(BPF_H, si->dst_reg, si->src_reg,
> + bpf_target_off(struct sk_buff,
> + queue_mapping,
> + 2, target_size));
One thing we should avoid would be to allow user to write NO_QUEUE_MAPPING
into skb->queue_mapping so we don't hit the warn in sk_tx_queue_set(), I'd
add this into the ctx rewrite here.
> break;
>
> case offsetof(struct __sk_buff, vlan_present):
>
^ permalink raw reply
* Re: [RFC PATCH net-next v3 00/21] ethtool netlink interface, part 1
From: Michal Kubecek @ 2019-02-19 11:57 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Andrew Lunn, Jakub Kicinski, linux-kernel
In-Reply-To: <20190219103508.GD3080@nanopsycho>
On Tue, Feb 19, 2019 at 11:35:08AM +0100, Jiri Pirko wrote:
> >- some features provided by ethtool would rather belong to devlink (and
> > some are already superseded by devlink); however, only few drivers
> > provide devlink interface at the moment and as recent discussion on
> > flashing revealed, we cannot rely on devlink's presence
>
> Could you explain why please?
What I mean is the problem discussed under Jakub's devlink flash
patchset: that he couldn't implement only the devlink callback in nfp
and rely on the generic fallback to devlink because it wouldn't work if
devlink is built as a module.
But I think this should be addressed. If we agree that flashing (and
other features provided by ethtool at the moment) rather belongs to
devlink (which nobody seems to oppose), we should rather try to make it
possible for drivers to provide only the devlink callback and gradually
move all in-tree drivers to doing so. (And one day, remove it from
ethtool_ops.) It doesn't seem to make much sense to have devlink as
a module in such scenario.
Michal
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net: phy: at803x: disable delay only for RGMII mode
From: Marc Gonzalez @ 2019-02-19 11:57 UTC (permalink / raw)
To: Vinod Koul, David S Miller, netdev
Cc: MSM, Bjorn Andersson, Niklas Cassel, Andrew Lunn,
Florian Fainelli, Nori Sekhar, Peter Ujfalusi
In-Reply-To: <20190219061800.31025-3-vkoul@kernel.org>
On 19/02/2019 07:18, Vinod Koul wrote:
> Per "Documentation/devicetree/bindings/net/ethernet.txt" RGMII mode
> should not have delay in PHY whereas RGMII_ID and RGMII_RXID/RGMII_TXID
> can have delay in phy.
PHY or phy? :-)
> So disable the delay only for RGMII mode and enable for other modes.
> Also treat the default case as disabled delays.
>
> Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode")
> Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Reviewed-by: Niklas Cassel <niklas.cassel@linaro.org>
> Tested-by: Peter Ujfalusi <peter.ujflausi@ti.com>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> drivers/net/phy/at803x.c | 47 ++++++++++++++++++++++++++++++----------
> 1 file changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index c6e7d800fd7a..dc1b13f7fc12 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -110,6 +110,18 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
> return phy_write(phydev, AT803X_DEBUG_DATA, val);
> }
>
> +static int at803x_enable_rx_delay(struct phy_device *phydev)
> +{
> + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
> + AT803X_DEBUG_RX_CLK_DLY_EN);
> +}
> +
> +static int at803x_enable_tx_delay(struct phy_device *phydev)
> +{
> + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
> + AT803X_DEBUG_TX_CLK_DLY_EN);
> +}
> +
> static int at803x_disable_rx_delay(struct phy_device *phydev)
> {
> return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
> @@ -255,23 +267,36 @@ static int at803x_config_init(struct phy_device *phydev)
> if (ret < 0)
> return ret;
>
> - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> - ret = at803x_disable_rx_delay(phydev);
> + /* The hardware register default is RX and TX delay enabled, so lets
> + * first disable the RX and TX delays in phy and enable them based
> + * on the mode selected
> + */
"let's" (let us)
For the record, AFAIR, the default is not *quite* RX and TX enabled:
https://www.spinics.net/lists/netdev/msg444527.html
RX: enabled at HW reset
TX: disabled at HW reset, but retains value after SW reset
> + ret = at803x_disable_rx_delay(phydev);
> + if (ret < 0)
> + return ret;
> + ret = at803x_disable_tx_delay(phydev);
> + if (ret < 0)
> + return ret;
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> + /* If RGMII_ID or RGMII_RXID are specified enable RX delay,
> + * otherwise keep it disabled
> + */
> + ret = at803x_enable_rx_delay(phydev);
> if (ret < 0)
> return ret;
> }
>
> - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> - ret = at803x_disable_tx_delay(phydev);
> - if (ret < 0)
> - return ret;
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> + /* If RGMII_ID or RGMII_TXID are specified enable TX delay,
> + * otherwise keep it disabled
> + */
> + ret = at803x_enable_tx_delay(phydev);
> }
>
> - return 0;
> + return ret;
> }
IMO, the asymmetry in error handling for RX and TX is unfortunate.
Didn't you like the way it was done in my old patch? :-)
https://www.spinics.net/lists/netdev/msg445053.html
Regards.
^ permalink raw reply
* Re: [PATCH net-next v4 07/17] net: sched: protect filter_chain list with filter_chain_lock mutex
From: Vlad Buslov @ 2019-02-19 12:31 UTC (permalink / raw)
To: Cong Wang
Cc: Ido Schimmel, netdev@vger.kernel.org, jhs@mojatatu.com,
jiri@resnulli.us, davem@davemloft.net, ast@kernel.org,
daniel@iogearbox.net
In-Reply-To: <CAM_iQpWXF1Yq2Yic0J3KTxo+h5UQTc1vwEcR-cs1424dnxd9-g@mail.gmail.com>
On Tue 19 Feb 2019 at 05:26, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Feb 15, 2019 at 7:35 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>> Another problem that I found in cls_fw and cls_route is that they set
>> arg->stop when empty. Both of them have code unchanged since it was
>> committed initially in 2005 so I assume this convention is no longer
>> relevant because all other classifiers don't do that (they only set
>> arg->stop when arg->fn returns negative value).
>>
>
> The question is why do you want to use arg->stop==0 as
> an indication for emptiness? Isn't what arg->count==0
> supposed to be?
Good question! I initially wanted to implement it like that, but
reconsidered because iterating through all filters on classifier to
count them is O(N), and terminating on first filter and relying on
arg->stop==1 is constant time. Making function that is called
"tcf_proto_is_empty" linear on number of filters seemed sloppy to me...
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net: phy: at803x: disable delay only for RGMII mode
From: Niklas Cassel @ 2019-02-19 12:33 UTC (permalink / raw)
To: Marc Gonzalez
Cc: Vinod Koul, David S Miller, netdev, MSM, Bjorn Andersson,
Andrew Lunn, Florian Fainelli, Nori Sekhar, Peter Ujfalusi
In-Reply-To: <1dddc455-309d-5e7d-ca26-479fe800a013@free.fr>
On Tue, Feb 19, 2019 at 12:57:18PM +0100, Marc Gonzalez wrote:
> On 19/02/2019 07:18, Vinod Koul wrote:
>
> > Per "Documentation/devicetree/bindings/net/ethernet.txt" RGMII mode
> > should not have delay in PHY whereas RGMII_ID and RGMII_RXID/RGMII_TXID
> > can have delay in phy.
>
> PHY or phy? :-)
>
> > So disable the delay only for RGMII mode and enable for other modes.
> > Also treat the default case as disabled delays.
> >
> > Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode")
> > Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > Reviewed-by: Niklas Cassel <niklas.cassel@linaro.org>
> > Tested-by: Peter Ujfalusi <peter.ujflausi@ti.com>
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> > drivers/net/phy/at803x.c | 47 ++++++++++++++++++++++++++++++----------
> > 1 file changed, 36 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> > index c6e7d800fd7a..dc1b13f7fc12 100644
> > --- a/drivers/net/phy/at803x.c
> > +++ b/drivers/net/phy/at803x.c
> > @@ -110,6 +110,18 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
> > return phy_write(phydev, AT803X_DEBUG_DATA, val);
> > }
> >
> > +static int at803x_enable_rx_delay(struct phy_device *phydev)
> > +{
> > + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
> > + AT803X_DEBUG_RX_CLK_DLY_EN);
> > +}
> > +
> > +static int at803x_enable_tx_delay(struct phy_device *phydev)
> > +{
> > + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
> > + AT803X_DEBUG_TX_CLK_DLY_EN);
> > +}
> > +
> > static int at803x_disable_rx_delay(struct phy_device *phydev)
> > {
> > return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
> > @@ -255,23 +267,36 @@ static int at803x_config_init(struct phy_device *phydev)
> > if (ret < 0)
> > return ret;
> >
> > - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> > - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > - phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> > - ret = at803x_disable_rx_delay(phydev);
> > + /* The hardware register default is RX and TX delay enabled, so lets
> > + * first disable the RX and TX delays in phy and enable them based
> > + * on the mode selected
> > + */
>
> "let's" (let us)
>
> For the record, AFAIR, the default is not *quite* RX and TX enabled:
>
> https://www.spinics.net/lists/netdev/msg444527.html
Hello Marc,
>
> RX: enabled at HW reset
> TX: disabled at HW reset, but retains value after SW reset
You are correct of course.
However, since the bootloader might have enabled delay on
TX, I still think that this patch does the right thing by
starting out with disabling delays for both RX and TX.
But we should probably make the comment more elaborate:
The value after HW reset is RX delay enabled and TX delay disabled.
The value after SW reset is RX delay enabled, while TX delay retains
the value before reset.
In order to not depend on reset values, start off by disabling both
delays.
Kind regards,
Niklas
>
> > + ret = at803x_disable_rx_delay(phydev);
> > + if (ret < 0)
> > + return ret;
> > + ret = at803x_disable_tx_delay(phydev);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > + phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> > + /* If RGMII_ID or RGMII_RXID are specified enable RX delay,
> > + * otherwise keep it disabled
> > + */
> > + ret = at803x_enable_rx_delay(phydev);
> > if (ret < 0)
> > return ret;
> > }
> >
> > - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> > - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > - phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> > - ret = at803x_disable_tx_delay(phydev);
> > - if (ret < 0)
> > - return ret;
> > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> > + /* If RGMII_ID or RGMII_TXID are specified enable TX delay,
> > + * otherwise keep it disabled
> > + */
> > + ret = at803x_enable_tx_delay(phydev);
> > }
> >
> > - return 0;
> > + return ret;
> > }
>
> IMO, the asymmetry in error handling for RX and TX is unfortunate.
>
> Didn't you like the way it was done in my old patch? :-)
>
> https://www.spinics.net/lists/netdev/msg445053.html
>
> Regards.
^ permalink raw reply
* Re: [RFC PATCH net-next v3 00/21] ethtool netlink interface, part 1
From: Jiri Pirko @ 2019-02-19 12:27 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, David Miller, Andrew Lunn, Jakub Kicinski, linux-kernel
In-Reply-To: <20190219115727.GE23151@unicorn.suse.cz>
Tue, Feb 19, 2019 at 12:57:27PM CET, mkubecek@suse.cz wrote:
>On Tue, Feb 19, 2019 at 11:35:08AM +0100, Jiri Pirko wrote:
>> >- some features provided by ethtool would rather belong to devlink (and
>> > some are already superseded by devlink); however, only few drivers
>> > provide devlink interface at the moment and as recent discussion on
>> > flashing revealed, we cannot rely on devlink's presence
>>
>> Could you explain why please?
>
>What I mean is the problem discussed under Jakub's devlink flash
>patchset: that he couldn't implement only the devlink callback in nfp
>and rely on the generic fallback to devlink because it wouldn't work if
>devlink is built as a module.
So let's fix that.
>
>But I think this should be addressed. If we agree that flashing (and
>other features provided by ethtool at the moment) rather belongs to
>devlink (which nobody seems to oppose), we should rather try to make it
>possible for drivers to provide only the devlink callback and gradually
>move all in-tree drivers to doing so. (And one day, remove it from
>ethtool_ops.) It doesn't seem to make much sense to have devlink as
>a module in such scenario.
Agreed.
>
>Michal
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net: phy: at803x: disable delay only for RGMII mode
From: Marc Gonzalez @ 2019-02-19 12:41 UTC (permalink / raw)
To: Niklas Cassel
Cc: Vinod Koul, David Miller, netdev, MSM, Bjorn Andersson,
Andrew Lunn, Florian Fainelli, Nori Sekhar, Peter Ujfalusi
In-Reply-To: <20190219123339.GA13172@centauri.lan>
On 19/02/2019 13:33, Niklas Cassel wrote:
> However, since the bootloader might have enabled delay on
> TX, I still think that this patch does the right thing by
> starting out with disabling delays for both RX and TX.
>
> But we should probably make the comment more elaborate:
>
> The value after HW reset is RX delay enabled and TX delay disabled.
> The value after SW reset is RX delay enabled, while TX delay retains
> the value before reset.
> In order to not depend on reset values, start off by disabling both
> delays.
Ultimately, the two patches do the same thing, AFAICT ;-)
I was just arguing that my way was better because... errr... because
it was my way! :-)
Regards.
^ permalink raw reply
* Re: [PATCH mlx5-next v1] net/mlx5: Factor out HCA capabilities functions
From: Leon Romanovsky @ 2019-02-19 12:42 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: RDMA mailing list, Saeed Mahameed, linux-netdev
In-Reply-To: <20190217111102.13608-1-leon@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]
On Sun, Feb 17, 2019 at 01:11:02PM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Combine all HCA capabilities setters under one function
> and compile out the ODP related function in case kernel
> was compiled without ODP support.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> Changelog v0->v1:
> * Embedded config option check into ODP capability function flow
> ---
> .../net/ethernet/mellanox/mlx5/core/main.c | 46 +++++++++++++------
> 1 file changed, 31 insertions(+), 15 deletions(-)
>
Applied to mlx5-next branch,
37b6bb77c6fd net/mlx5: Factor out HCA capabilities functions
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: L2TPv3 offset
From: James Chapman @ 2019-02-19 12:40 UTC (permalink / raw)
To: t.martitz; +Cc: davem, netdev
In-Reply-To: <OF967393CB.A0BF4494-ONC12583A6.00323DFC-C12583A6.00330730@avm.de>
On 19/02/2019 09:17, t.martitz@avm.de wrote:
>
> Hello,
>
> I saw that you removed the offset option from l2tp sessions in Linux
> 4.16 (commit 900631ee6a2651dc4fbaecb8ef9fa5f1e3378853 l2tp: remove
> configurable payload offset). Since we need something like that I'm
> reaching out to you.
>
Adding netdev.
>
>
> Our use case is pseudo-wire over l2tp, so we encapsulate ethernet
> within l2tpv3. This has lots of benifits for us, but one remarkable
> drawback is that the receiver, who's removing the encapsulation, will
> see a misaligned inner IP header.
>
> So we planned to use the offset to correct that. But I saw that the
> offset was removed not too long ago. Now I see that there is still
> "l2specific_len" / L2TP_ATTR_L2SPEC_LEN that we can use for pretty
> much the same purpose but I get the feeling that this should only be 0
> or 4 while we would need 2 or 6. I get this feeling because "ip l2tp"
> of the iproute2 package does not allow setting this at all and
> hardcodes 0 or 4 based on the type.
>
In L2TPv3, any payload offset would need to be implemented by defining a
new L2SpecificSublayer type since the L2TPv3 header has no offset field.
There are two standard L2TPv3 sublayers supported - None and Default.
These are fixed size (0 and 4 bytes respectively), hence the kernel now
ignores L2TP_ATTR_L2SPEC_LEN since the length can be derived from
L2TP_ATTR_L2SPEC_TYPE. For reference, the set of defined types are
listed at
https://www.iana.org/assignments/l2tp-parameters/l2tp-parameters.xhtml#l2tp-parameters-37.
No L2SpecificSublayer type was ever defined for L2TPv3 to allow a
configurable payload offset in ethernet pseudowires.
>
> So my question is what should we do know? Being based on removed
> functionality is kind of bad, but we must fix the misaligned inner ip
> header.
> We control the sender and receiver so we could apply all kinds of
> hacks, but we would rather find a solution compliant with the Internet
> community.
>
If you really need to insert padding in transmitted L2TPv3 packets
between the L2TPv3 header and its payload, one option is to define a new
L2SpecificHeaderType and patch the kernel to accept it.
If you control both the sender and receiver, is using FOU an option?
James
^ permalink raw reply
* Re: [PATCH v3] net: ns83820: code cleanup for ns83820_probe_phy()
From: Walter Harms @ 2019-02-19 12:42 UTC (permalink / raw)
To: Mao Wenan
Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
daniel, ast, julia.lawall
Am 19.02.2019 10:06, schrieb Mao Wenan:
> This patch is to do code cleanup for ns83820_probe_phy().
> It deletes unused variable 'first' and commented out code.
>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
> v2->v3: delte unused variable 'first'; change subject from
> "net: ns83820: drop pointless static qualifier in ns83820_probe_phy()" to
> "net: ns83820: code cleanup for ns83820_probe_phy()".
> drivers/net/ethernet/natsemi/ns83820.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/natsemi/ns83820.c
> b/drivers/net/ethernet/natsemi/ns83820.c
> index 958fced4dacf..955d34a6f0d8 100644
> --- a/drivers/net/ethernet/natsemi/ns83820.c
> +++ b/drivers/net/ethernet/natsemi/ns83820.c
> @@ -1869,34 +1869,16 @@ static unsigned ns83820_mii_write_reg(struct ns83820
> *dev, unsigned phy, unsigne
> static void ns83820_probe_phy(struct net_device *ndev)
> {
> struct ns83820 *dev = PRIV(ndev);
> - static int first;
> int i;
> #define MII_PHYIDR1 0x02
> #define MII_PHYIDR2 0x03
>
> -#if 0
> - if (!first) {
> - unsigned tmp;
> - ns83820_mii_read_reg(dev, 1, 0x09);
> - ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
> -
> - tmp = ns83820_mii_read_reg(dev, 1, 0x00);
> - ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
> - udelay(1300);
> - ns83820_mii_read_reg(dev, 1, 0x09);
> - }
> -#endif
> - first = 1;
> -
> for (i=1; i<2; i++) {
the loop here seems also pointless, so you can eliminate i.
(or did i muss something ?)
just my 2 cents,
re,
wh
> int j;
> unsigned a, b;
> a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
> b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
>
> - //printk("%s: phy %d: 0x%04x 0x%04x\n",
> - // ndev->name, i, a, b);
> -
> for (j=0; j<0x16; j+=4) {
> dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
> ndev->name, j,
^ permalink raw reply
* Re: [PATCH v3] net: ns83820: code cleanup for ns83820_probe_phy()
From: walter harms @ 2019-02-19 12:40 UTC (permalink / raw)
To: Mao Wenan
Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
daniel, ast, julia.lawall
In-Reply-To: <20190219090635.134457-1-maowenan@huawei.com>
Am 19.02.2019 10:06, schrieb Mao Wenan:
> This patch is to do code cleanup for ns83820_probe_phy().
> It deletes unused variable 'first' and commented out code.
>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
> v2->v3: delte unused variable 'first'; change subject from
> "net: ns83820: drop pointless static qualifier in ns83820_probe_phy()" to
> "net: ns83820: code cleanup for ns83820_probe_phy()".
> drivers/net/ethernet/natsemi/ns83820.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
> index 958fced4dacf..955d34a6f0d8 100644
> --- a/drivers/net/ethernet/natsemi/ns83820.c
> +++ b/drivers/net/ethernet/natsemi/ns83820.c
> @@ -1869,34 +1869,16 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
> static void ns83820_probe_phy(struct net_device *ndev)
> {
> struct ns83820 *dev = PRIV(ndev);
> - static int first;
> int i;
> #define MII_PHYIDR1 0x02
> #define MII_PHYIDR2 0x03
>
> -#if 0
> - if (!first) {
> - unsigned tmp;
> - ns83820_mii_read_reg(dev, 1, 0x09);
> - ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
> -
> - tmp = ns83820_mii_read_reg(dev, 1, 0x00);
> - ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
> - udelay(1300);
> - ns83820_mii_read_reg(dev, 1, 0x09);
> - }
> -#endif
> - first = 1;
> -
> for (i=1; i<2; i++) {
> int j;
> unsigned a, b;
> a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
> b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
>
> - //printk("%s: phy %d: 0x%04x 0x%04x\n",
> - // ndev->name, i, a, b);
> -
> for (j=0; j<0x16; j+=4) {
> dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
> ndev->name, j,
^ permalink raw reply
* Re: [PATCH v3] net: ns83820: code cleanup for ns83820_probe_phy()
From: maowenan @ 2019-02-19 12:52 UTC (permalink / raw)
To: Walter Harms
Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
daniel, ast, julia.lawall
In-Reply-To: <715767253.151509.1550580124264@ox-groupware.bfs.de>
On 2019/2/19 20:42, Walter Harms wrote:
>
> Am 19.02.2019 10:06, schrieb Mao Wenan:
>> This patch is to do code cleanup for ns83820_probe_phy().
>> It deletes unused variable 'first' and commented out code.
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>> v2->v3: delte unused variable 'first'; change subject from
>> "net: ns83820: drop pointless static qualifier in ns83820_probe_phy()" to
>> "net: ns83820: code cleanup for ns83820_probe_phy()".
>> drivers/net/ethernet/natsemi/ns83820.c | 18 ------------------
>> 1 file changed, 18 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/natsemi/ns83820.c
>> b/drivers/net/ethernet/natsemi/ns83820.c
>> index 958fced4dacf..955d34a6f0d8 100644
>> --- a/drivers/net/ethernet/natsemi/ns83820.c
>> +++ b/drivers/net/ethernet/natsemi/ns83820.c
>> @@ -1869,34 +1869,16 @@ static unsigned ns83820_mii_write_reg(struct ns83820
>> *dev, unsigned phy, unsigne
>> static void ns83820_probe_phy(struct net_device *ndev)
>> {
>> struct ns83820 *dev = PRIV(ndev);
>> - static int first;
>> int i;
>> #define MII_PHYIDR1 0x02
>> #define MII_PHYIDR2 0x03
>>
>> -#if 0
>> - if (!first) {
>> - unsigned tmp;
>> - ns83820_mii_read_reg(dev, 1, 0x09);
>> - ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
>> -
>> - tmp = ns83820_mii_read_reg(dev, 1, 0x00);
>> - ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
>> - udelay(1300);
>> - ns83820_mii_read_reg(dev, 1, 0x09);
>> - }
>> -#endif
>> - first = 1;
>> -
>> for (i=1; i<2; i++) {
>
>
> the loop here seems also pointless, so you can eliminate i.
> (or did i muss something ?)
>
good point.Thank you.
> just my 2 cents,
> re,
> wh
>> int j;
>> unsigned a, b;
>> a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
>> b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
>>
>> - //printk("%s: phy %d: 0x%04x 0x%04x\n",
>> - // ndev->name, i, a, b);
>> -
>> for (j=0; j<0x16; j+=4) {
>> dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
>> ndev->name, j,
>
> .
>
^ permalink raw reply
* Re: [PATCH net-next v2] net: dsa: qca8k: Enable delay for RGMII_ID mode
From: Andrew Lunn @ 2019-02-19 12:58 UTC (permalink / raw)
To: Vinod Koul
Cc: David S Miller, linux-arm-msm, Bjorn Andersson, netdev,
Niklas Cassel, Florian Fainelli, Michal Vokáč
In-Reply-To: <20190219065943.22765-1-vkoul@kernel.org>
On Tue, Feb 19, 2019 at 12:29:43PM +0530, Vinod Koul wrote:
> RGMII_ID specifies that we should have internal delay, so resurrect the
> delay addition routine but under the RGMII_ID mode.
>
> Fixes: 40269aa9f40a ("net: dsa: qca8k: disable delay for RGMII mode")
> Tested-by: Michal Vokáč <michal.vokac@ysoft.com>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: Handling an Extra Signal at PHY Reset
From: Paul Kocialkowski @ 2019-02-19 12:53 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, netdev,
Mylène Josserand
In-Reply-To: <20190219103608.56d390db@windsurf.home>
Hi,
On Tue, 2019-02-19 at 10:36 +0100, Thomas Petazzoni wrote:
> Hello Paul,
>
> On Tue, 19 Feb 2019 10:14:20 +0100
> Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote:
>
> > We are dealing with an Ethernet PHY (Marvell 88E1512) that comes with a
> > CONFIG pin that must be connected to one of the other pins of the PHY
> > to configure the LSB of the PHY address as well as I/O voltages (see
> > section 2.18.1 Hardware Configuration of the datasheet). It must be
> > connected "soon after reset" for the PHY to be correctly configured.
> >
> > We have a switch for connecting the CONFIG pin to the other pin (LED0),
> > which needs to be controlled by Linux. The CONFIG pin seems to be used
> > for a PTP clock the rest of the time.
> >
> > So we are wondering how to properly represent this case, especially on
> > the device-tree side.
> >
> > The trick here is that this step is necessary before the PHY can be
> > discovered on the MDIO bus (and thus the PHY driver selected) so we
> > can't rely on the PHY driver to do this. Basically, it looks like we
> > need to handle this like the reset pin and describe it at the MDIO bus
> > level.
> >
> > Here are some ideas for potential solutions:
> > - Allowing more than a single GPIO to be passed to the MDIO bus' reset-
> > gpios via device-tree and toggling all the passed GPIOs at once;
> >
> > - Adding a new optional GPIO for the MDIO bus dedicated to controlling
> > switches for such config switching, perhaps called "config-gpios"
> > (quite a narrow solution);
> >
> > - Adding a broader power sequence description to the MDIO bus (a bit
> > like it's done with the mmc pwrseq descriptions) which would allow
> > specifying the toggle order/delays of various GPIOs (would probably be
> > the most extensive solution);
> >
> > - Adding the extra GPIO control to the MAC description and toggling it
> > through bus->reset (probably the less invasive solution for the core
> > but not very satisfying from the description perspective, since this is
> > definitely not MAC-specific).
> >
> > What do you think about how we could solve this issue?
> > Do you see other options that I missed here?
>
> I think it's important to mention the sequence that is needed:
>
> 1. assert reset
> 2. wait 10 us
> 3. switch config signal
> 4. deassert reset
> 5. wait 100us
> 6. de-switch config signal
>
> I.e, the config signal needs to be switched properly before deasserting
> reset, and then switched back to its original state so that the config
> pin can be used for its normal (non-reset) purpose.
>
> So the manipulation of the config signal is intertwined with the
> assert/de-assert of the reset. So I don't see how your fourth option
> would work for example. Am I missing something here ?
So we discussed and checked this with Thomas and the sequence that he
mentionned is indeed the right way to go.
However, we also found that our board circuitry switches the signal
when the GPIO is not driven low. So assuming that it's either high-Z or
high when the kernel boots, we can manage with the final step (de-
switch config signal) alone (although it's safer to ensure this is the
case before reset).
Cheers,
Paul
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply
* [PATCH mlx5-next] net/mlx5: Separate ODP capabilities
From: Leon Romanovsky @ 2019-02-19 13:06 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Moni Shoua, RDMA mailing list, Saeed Mahameed, linux-netdev,
Leon Romanovsky
From: Moni Shoua <monis@mellanox.com>
ODP support for XRC transport is not enabled by default in FW,
so we need separate ODP checks to enable/disable it.
While that, rewrite the set of ODP SRQ support capabilities in way
that tests each field separately for clearness, which is not needed
for current FW, but better to have it separated.
Signed-off-by: Moni Shoua <monis@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
.../net/ethernet/mellanox/mlx5/core/main.c | 29 ++++++++++---------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 025dfaf3466b..5cf557a77f0f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -474,11 +474,6 @@ static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
if (err)
return err;
- if (!(MLX5_CAP_ODP_MAX(dev, ud_odp_caps.srq_receive) ||
- MLX5_CAP_ODP_MAX(dev, rc_odp_caps.srq_receive) ||
- MLX5_CAP_ODP_MAX(dev, xrc_odp_caps.srq_receive)))
- return 0;
-
set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in);
set_ctx = kzalloc(set_sz, GFP_KERNEL);
if (!set_ctx)
@@ -488,15 +483,21 @@ static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
memcpy(set_hca_cap, dev->caps.hca_cur[MLX5_CAP_ODP],
MLX5_ST_SZ_BYTES(odp_cap));
- /* set ODP SRQ support for RC/UD and XRC transports */
- MLX5_SET(odp_cap, set_hca_cap, ud_odp_caps.srq_receive,
- MLX5_CAP_ODP_MAX(dev, ud_odp_caps.srq_receive));
-
- MLX5_SET(odp_cap, set_hca_cap, rc_odp_caps.srq_receive,
- MLX5_CAP_ODP_MAX(dev, rc_odp_caps.srq_receive));
-
- MLX5_SET(odp_cap, set_hca_cap, xrc_odp_caps.srq_receive,
- MLX5_CAP_ODP_MAX(dev, xrc_odp_caps.srq_receive));
+#define ODP_CAP_SET_MAX(dev, field) \
+ do { \
+ u32 _res = MLX5_CAP_ODP_MAX(dev, field); \
+ if (_res) \
+ MLX5_SET(odp_cap, set_hca_cap, field, _res); \
+ } while (0)
+
+ ODP_CAP_SET_MAX(dev, ud_odp_caps.srq_receive);
+ ODP_CAP_SET_MAX(dev, rc_odp_caps.srq_receive);
+ ODP_CAP_SET_MAX(dev, xrc_odp_caps.srq_receive);
+ ODP_CAP_SET_MAX(dev, xrc_odp_caps.send);
+ ODP_CAP_SET_MAX(dev, xrc_odp_caps.receive);
+ ODP_CAP_SET_MAX(dev, xrc_odp_caps.write);
+ ODP_CAP_SET_MAX(dev, xrc_odp_caps.read);
+ ODP_CAP_SET_MAX(dev, xrc_odp_caps.atomic);
err = set_caps(dev, set_ctx, set_sz, MLX5_SET_HCA_CAP_OP_MOD_ODP);
--
2.19.1
^ permalink raw reply related
* Re: [RFC PATCH net-next v3 04/21] ethtool: helper functions for netlink interface
From: Michal Kubecek @ 2019-02-19 13:07 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, David Miller, Andrew Lunn, Jiri Pirko, linux-kernel
In-Reply-To: <20190218121517.4d299f6e@cakuba.netronome.com>
On Mon, Feb 18, 2019 at 12:15:17PM -0800, Jakub Kicinski wrote:
> On Mon, 18 Feb 2019 19:21:44 +0100 (CET), Michal Kubecek wrote:
> > +/* create skb for a reply and fill device identification
> > + * payload: payload length (without netlink and genetlink header)
> > + * dev: device the reply is about (may be null)
> > + * cmd: ETHNL_CMD_* command for reply
> > + * info: info for the received packet we respond to
> > + * ehdrp: place to store payload pointer returned by genlmsg_new()
> > + * returns: skb or null on error
> > + */
>
> nit: why not correct kdoc?
Good point. I'll fix that, together with the other things.
Michal
>
> > +struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
> > + u16 dev_attrtype, struct genl_info *info,
> > + void **ehdrp)
> > +{
> > + void *ehdr;
> > + struct sk_buff *rskb;
>
> nit: reverse xmas tree
^ permalink raw reply
* Re: [PATCH] phy/marvell: prevent unneeded page switching
From: Andrew Lunn @ 2019-02-19 13:19 UTC (permalink / raw)
To: Frank de Brabander; +Cc: Florian Fainelli, Heiner Kallweit, netdev
In-Reply-To: <1550560893-2445-1-git-send-email-debrabander@gmail.com>
On Tue, Feb 19, 2019 at 08:21:33AM +0100, Frank de Brabander wrote:
> marvell_read_status() no longer first switches to the fiber page, instead
> looks at the already active register page first. In case the link is down,
> switches to the other (fiber/copper) register page.
>
> Unneeded register page switching of the Marvell 88E1510 PHY can cause
> the ethernet driver to register a link down & up event.
Hi Frank
I think the page change itself is unlikely to cause a link down/up.
It seems more likely there is something else going on here. The wrong
page being read, fibre page being read when there is no support for
fibre, etc.
Do you get link down/up reported when you read the temperature sensor?
That also causes a page change?
I think i would first start by adding locking to
marvell_read_status().
phy_select_page() takes a lock before changing the page.
phy_restore_page() releases the lock. Use those around reading the
status.
Andrew
^ permalink raw reply
* [PATCH v4] net: ns83820: code cleanup for ns83820_probe_phy()
From: Mao Wenan @ 2019-02-19 13:45 UTC (permalink / raw)
To: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
daniel, ast, julia.lawall, wharms
This patch is to do code cleanup for ns83820_probe_phy().
It deletes unused variable 'first', commented out code,
and the pointless 'for' loop.
Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
---
v3->v4: delete pointless 'for' loop, variable 'i', and so on.
drivers/net/ethernet/natsemi/ns83820.c | 70 ++++++++------------------
1 file changed, 21 insertions(+), 49 deletions(-)
diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
index 958fced4dacf..959c0fa37d75 100644
--- a/drivers/net/ethernet/natsemi/ns83820.c
+++ b/drivers/net/ethernet/natsemi/ns83820.c
@@ -1869,56 +1869,28 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
static void ns83820_probe_phy(struct net_device *ndev)
{
struct ns83820 *dev = PRIV(ndev);
- static int first;
- int i;
-#define MII_PHYIDR1 0x02
-#define MII_PHYIDR2 0x03
-
-#if 0
- if (!first) {
- unsigned tmp;
- ns83820_mii_read_reg(dev, 1, 0x09);
- ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
-
- tmp = ns83820_mii_read_reg(dev, 1, 0x00);
- ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
- udelay(1300);
- ns83820_mii_read_reg(dev, 1, 0x09);
- }
-#endif
- first = 1;
-
- for (i=1; i<2; i++) {
- int j;
- unsigned a, b;
- a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
- b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
-
- //printk("%s: phy %d: 0x%04x 0x%04x\n",
- // ndev->name, i, a, b);
-
- for (j=0; j<0x16; j+=4) {
- dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
- ndev->name, j,
- ns83820_mii_read_reg(dev, i, 0 + j),
- ns83820_mii_read_reg(dev, i, 1 + j),
- ns83820_mii_read_reg(dev, i, 2 + j),
- ns83820_mii_read_reg(dev, i, 3 + j)
- );
- }
- }
- {
- unsigned a, b;
- /* read firmware version: memory addr is 0x8402 and 0x8403 */
- ns83820_mii_write_reg(dev, 1, 0x16, 0x000d);
- ns83820_mii_write_reg(dev, 1, 0x1e, 0x810e);
- a = ns83820_mii_read_reg(dev, 1, 0x1d);
-
- ns83820_mii_write_reg(dev, 1, 0x16, 0x000d);
- ns83820_mii_write_reg(dev, 1, 0x1e, 0x810e);
- b = ns83820_mii_read_reg(dev, 1, 0x1d);
- dprintk("version: 0x%04x 0x%04x\n", a, b);
+ int j;
+ unsigned a, b;
+
+ for (j = 0; j < 0x16; j += 4) {
+ dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
+ ndev->name, j,
+ ns83820_mii_read_reg(dev, 1, 0 + j),
+ ns83820_mii_read_reg(dev, 1, 1 + j),
+ ns83820_mii_read_reg(dev, 1, 2 + j),
+ ns83820_mii_read_reg(dev, 1, 3 + j)
+ );
}
+
+ /* read firmware version: memory addr is 0x8402 and 0x8403 */
+ ns83820_mii_write_reg(dev, 1, 0x16, 0x000d);
+ ns83820_mii_write_reg(dev, 1, 0x1e, 0x810e);
+ a = ns83820_mii_read_reg(dev, 1, 0x1d);
+
+ ns83820_mii_write_reg(dev, 1, 0x16, 0x000d);
+ ns83820_mii_write_reg(dev, 1, 0x1e, 0x810e);
+ b = ns83820_mii_read_reg(dev, 1, 0x1d);
+ dprintk("version: 0x%04x 0x%04x\n", a, b);
}
#endif
--
2.20.1
^ permalink raw reply related
* Re: [Bridge] [RFC v2] net: bridge: don't flood known multicast traffic when snooping is enabled
From: Nikolay Aleksandrov @ 2019-02-19 13:31 UTC (permalink / raw)
To: Linus Lüssing; +Cc: bridge, netdev, roopa, f.fainelli, idosch
In-Reply-To: <20190219092125.GE10191@otheros>
On 19/02/2019 11:21, Linus Lüssing wrote:
> On Tue, Feb 19, 2019 at 09:57:16AM +0100, Linus Lüssing wrote:
>> On Mon, Feb 18, 2019 at 02:21:07PM +0200, Nikolay Aleksandrov wrote:
>>> This is v2 of the RFC patch which aims to forward packets to known
>>> mdsts' ports only (the no querier case). After v1 I've kept
>>> the previous behaviour when it comes to unregistered traffic or when
>>> a querier is present. All of this is of course only with snooping
>>> enabled. So with this patch the following changes should occur:
>>> - No querier: forward known mdst traffic to its registered ports,
>>> no change about unknown mcast (flood)
>>> - Querier present: no change
>>>
>>> The reason to do this is simple - we want to respect the user's mdb
>>> configuration in both cases, that is if the user adds static mdb entries
>>> manually then we should use that information about forwarding traffic.
>>>
>>> What do you think ?
>>>
>>> * Notes
>>> Traffic that is currently marked as mrouters_only:
>>> - IPv4: non-local mcast traffic, igmp reports
>>> - IPv6: non-all-nodes-dst mcast traffic, mldv1 reports
>>>
>>> Simple use case:
>>> $ echo 1 > /sys/class/net/bridge/bridge/multicast_snooping
>>> $ bridge mdb add dev bridge port swp1 grp 239.0.0.1
>>> - without a querier currently traffic for 239.0.0.1 will still be flooded,
>>> with this change it will be forwarded only to swp1
>>
>> There is still the issue with unsolicited reports adding mdst
>> entries here, too. Leading to unwanted packet loss and connectivity issues.
>
> Or in other words, an unsolicited report will turn a previously
> unregistered multicast group into a registered one. However in the
> absence of a querier the knowledge about this newly registered multicast group
> will be incomplete. And therefore still needs to be flooded to avoid packet
> loss.
>
Right, this is expected. If the user has enabled igmp snooping and doesn't have
a querier present then such behaviour is to be expected. What is surprising is
the user explicitly enabling igmp snooping, adding an mdst and then still
getting it flooded. :)
An alternative is to drop all unregistered traffic when a querier is not present.
But that will surely break setups and at best should be a configurable option that
is disabled by default.
So in effect and to try and make everybody happy we can add an option to control
this behaviour with keeping the current as default and adding the following options:
- no querier: flood all (default, current)
- no querier: flood unregistered, forward registered
- no querier: drop unregistered, forward registered
^ permalink raw reply
* Re: Handling an Extra Signal at PHY Reset
From: Andrew Lunn @ 2019-02-19 13:36 UTC (permalink / raw)
To: Paul Kocialkowski
Cc: Florian Fainelli, Heiner Kallweit, netdev, Thomas Petazzoni,
Mylène Josserand
In-Reply-To: <e647050a0fbadb8445cf6a7a5a2ccfbfd0865592.camel@bootlin.com>
On Tue, Feb 19, 2019 at 10:14:20AM +0100, Paul Kocialkowski wrote:
> Hi,
>
> We are dealing with an Ethernet PHY (Marvell 88E1512) that comes with a
> CONFIG pin that must be connected to one of the other pins of the PHY
> to configure the LSB of the PHY address as well as I/O voltages (see
> section 2.18.1 Hardware Configuration of the datasheet). It must be
> connected "soon after reset" for the PHY to be correctly configured.
Hi Paul
I assume there are two PHYs on the MDIO bus, and you need to ensure
they have different addresses? Do we have an Ethernet switch involved
here, or are they two SoC Ethernet networks with one shared MDIO bus?
This seems like an odd design. I've normally seen weak pull up/down
resistors, not a switch, so i'm wondering why it is designed like
this.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH] bpf: bpftool, fix documentation for attach types
From: Alban Crequy @ 2019-02-19 13:48 UTC (permalink / raw)
To: Quentin Monnet
Cc: Alban Crequy, Alexei Starovoitov, Daniel Borkmann, netdev, LKML,
John Fastabend, Iago López Galeiras
In-Reply-To: <208650eb-37bf-228b-43f8-de5027747db9@netronome.com>
On Mon, Feb 11, 2019 at 2:26 PM Quentin Monnet
<quentin.monnet@netronome.com> wrote:
>
> 2019-02-11 13:54 UTC+0100 ~ Alban Crequy <alban.crequy@gmail.com>
> > From: Alban Crequy <alban@kinvolk.io>
> >
> > bpftool has support for attach types "stream_verdict" and
> > "stream_parser" but the documentation was referring to them with
> > "skb_verdict" and "skb_parse". The inconsistency comes from commit
> > b7d3826c2ed6 ("bpf: bpftool, add support for attaching programs to
> > maps").
> >
> > This patch changes the documentation to match the implementation.
> >
> > Signed-off-by: Alban Crequy <alban@kinvolk.io>
> > ---
> > tools/bpf/bpftool/prog.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> > index 0640e9bc0ada..dfaa019a60f0 100644
> > --- a/tools/bpf/bpftool/prog.c
> > +++ b/tools/bpf/bpftool/prog.c
> > @@ -1198,7 +1198,7 @@ static int do_help(int argc, char **argv)
> > " cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |\n"
> > " cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |\n"
> > " cgroup/sendmsg4 | cgroup/sendmsg6 }\n"
> > - " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse |\n"
> > + " ATTACH_TYPE := { msg_verdict | stream_verdict | stream_parser |\n"
> > " flow_dissector }\n"
> > " " HELP_SPEC_OPTIONS "\n"
> > "",
> >
>
> Thanks Alban!
>
> Could you please fix the man page and the bash completion file at the
> same time?
Yes, I will do that soon. Sorry for the delay in replying.
Thanks,
Alban
^ permalink raw reply
* [PATCH][next] vhost: only return early if ret indicates an error or no iovecs have been processed
From: Colin King @ 2019-02-19 13:57 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, kvm, virtualization, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently the loop that calls log_write_hva on each iovec is never
executed because of an incorrect error check on the return from the
call to translate_desc. The check should be checking for a -ve error
return and because it makes no sense to iterate over zero items, the
checks should also check for zero too.
Detected by CoverityScan, CID#1476969 ("Logically dead code")
Fixes: cc5e71075947 ("vhost: log dirty page correctly")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/vhost/vhost.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 24a129fcdd61..a9a1709a859a 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1788,7 +1788,7 @@ static int log_used(struct vhost_virtqueue *vq, u64 used_offset, u64 len)
ret = translate_desc(vq, (uintptr_t)vq->used + used_offset,
len, iov, 64, VHOST_ACCESS_WO);
- if (ret)
+ if (ret <= 0)
return ret;
for (i = 0; i < ret; i++) {
--
2.20.1
^ permalink raw reply related
* [PATCH iproute2] ip-rule/trivial: add comment about json key "to_tbl" for unspecific rule action
From: Thomas Haller @ 2019-02-19 14:03 UTC (permalink / raw)
To: netdev; +Cc: stephen, Thomas Haller
The key should not be called "to_tbl" because it is exactly
not a FR_ACT_TO_TBL action.
# ip rule add blackhole
# ip -j rule | python -m json.tool
...
{
"priority": 0,
"src": "all",
"to_tbl": "blackhole"
},
Still, as this is already released API from v4.17.0, stick to it.
Only add a comment for this oddity.
Related: 0dd4ccc56c0e ("iprule: add json support")
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
If it is still permissible to change the API, I can also send a patch to
rename "to_tbl" key to "action".
ip/iprule.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ip/iprule.c b/ip/iprule.c
index 2f58d8c2..aea175aa 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -459,6 +459,8 @@ int print_rule(struct nlmsghdr *n, void *arg)
} else if (frh->action == FR_ACT_NOP) {
print_null(PRINT_ANY, "nop", "nop", NULL);
} else if (frh->action != FR_ACT_TO_TBL) {
+ /* The action is not(!) to-tbl, however for historic
+ * reasons, this JSON key is called "to_tbl". */
print_string(PRINT_ANY, "to_tbl", "%s",
rtnl_rtntype_n2a(frh->action, b1, sizeof(b1)));
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next 09/12] net: sched: flower: handle concurrent tcf proto deletion
From: Vlad Buslov @ 2019-02-19 14:08 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <CAM_iQpWdKR1JJsqarzLSuzaUDfPeTzPZMjCUb-ENj8aG62v_nA@mail.gmail.com>
On Mon 18 Feb 2019 at 20:47, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Wed, Feb 13, 2019 at 11:47 PM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>> Without rtnl lock protection tcf proto can be deleted concurrently. Check
>> tcf proto 'deleting' flag after taking tcf spinlock to verify that no
>> concurrent deletion is in progress. Return EAGAIN error if concurrent
>> deletion detected, which will cause caller to retry and possibly create new
>> instance of tcf proto.
>>
>
> Please state the reason why you prefer retry over locking the whole
> tp without retrying, that is why and how it is better?
>
> Personally I always prefer non-retry logic, because it is very easy
> to understand and justify its correctness.
>
> As you prefer otherwise, please share your reasoning in changelog.
>
> Thanks!
At the moment filter removal code is implemented by cls API in following
fashion:
1) tc_del_tfilter() obtains opaque void pointer to filter by calling
tp->ops->get()
2) Pass filter pointer to tfilter_del_notify() which prepares skb with
all necessary info about filter that is being removed and...
3) ... calls tp->ops->delete() to actually delete filter.
Between 1) and 3) filter can be removed concurrently and there is
nothing we can do about it in flower, besides account for that with some
kind of retry logic. I will explain why I prefer cls API to not just
lock whole classifier instance when modifying it in any way in reply to
cls API patch "net: sched: protect filter_chain list with
filter_chain_lock mutex" discussion.
^ permalink raw reply
* [PATCH bpf-next v2] bpf: bpftool, fix documentation for attach types
From: Alban Crequy @ 2019-02-19 14:13 UTC (permalink / raw)
To: ast, daniel, quentin.monnet
Cc: netdev, linux-kernel, john.fastabend, alban, iago
From: Alban Crequy <alban@kinvolk.io>
bpftool has support for attach types "stream_verdict" and
"stream_parser" but the documentation was referring to them as
"skb_verdict" and "skb_parse". The inconsistency comes from commit
b7d3826c2ed6 ("bpf: bpftool, add support for attaching programs to
maps").
This patch changes the documentation to match the implementation:
- "bpftool prog help"
- man pages
- bash completion
Signed-off-by: Alban Crequy <alban@kinvolk.io>
---
Changes v1 to v2:
- fix man pages & bash completion (from Quentin's review)
---
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 2 +-
tools/bpf/bpftool/bash-completion/bpftool | 4 ++--
tools/bpf/bpftool/prog.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 7e59495cb028..12bc1e2d4b46 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -42,7 +42,7 @@ PROG COMMANDS
| **cgroup/connect4** | **cgroup/connect6** | **cgroup/sendmsg4** | **cgroup/sendmsg6**
| }
| *ATTACH_TYPE* := {
-| **msg_verdict** | **skb_verdict** | **skb_parse** | **flow_dissector**
+| **msg_verdict** | **stream_verdict** | **stream_parser** | **flow_dissector**
| }
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 763dd12482aa..b803827d01e8 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -311,8 +311,8 @@ _bpftool()
return 0
;;
5)
- COMPREPLY=( $( compgen -W 'msg_verdict skb_verdict \
- skb_parse flow_dissector' -- "$cur" ) )
+ COMPREPLY=( $( compgen -W 'msg_verdict stream_verdict \
+ stream_parser flow_dissector' -- "$cur" ) )
return 0
;;
6)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 33ed0806ccc0..db978c8d76a8 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -1199,7 +1199,7 @@ static int do_help(int argc, char **argv)
" cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |\n"
" cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |\n"
" cgroup/sendmsg4 | cgroup/sendmsg6 }\n"
- " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse |\n"
+ " ATTACH_TYPE := { msg_verdict | stream_verdict | stream_parser |\n"
" flow_dissector }\n"
" " HELP_SPEC_OPTIONS "\n"
"",
--
2.20.1
^ 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