* Re: [PATCH v2] dt-bindings: net: orion-mdio: Convert to JSON schema
From: Chris Packham @ 2022-05-16 22:27 UTC (permalink / raw)
To: Rob Herring
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, krzysztof.kozlowski+dt@linaro.org,
andrew@lunn.ch, netdev@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Marek Behún
In-Reply-To: <0c67af76-df5e-1684-820c-f28aa6f50fe1@alliedtelesis.co.nz>
On 17/05/22 10:18, Chris Packham wrote:
> Hi Rob,
>
> On 17/05/22 06:16, Rob Herring wrote:
>> On Fri, May 06, 2022 at 09:06:20AM +1200, Chris Packham wrote:
>>> Convert the marvell,orion-mdio binding to JSON schema.
>>>
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> ---
>>>
>>> Notes:
>>> This does throw up the following dtbs_check warnings for
>>> turris-mox:
>>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004:
>>> switch0@10:reg: [[16], [0]] is too long
>>> From schema:
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb:
>>> mdio@32004: switch0@2:reg: [[2], [0]] is too long
>>> From schema:
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb:
>>> mdio@32004: switch1@11:reg: [[17], [0]] is too long
>>> From schema:
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb:
>>> mdio@32004: switch1@2:reg: [[2], [0]] is too long
>>> From schema:
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb:
>>> mdio@32004: switch2@12:reg: [[18], [0]] is too long
>>> From schema:
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb:
>>> mdio@32004: switch2@2:reg: [[2], [0]] is too long
>>> From schema:
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>> I think they're all genuine but I'm hesitant to leap in
>>> and fix them
>>> without being able to test them.
>>> I also need to set unevaluatedProperties: true to cater
>>> for the L2
>>> switch on turris-mox (and probably others). That might be
>>> better tackled
>>> in the core mdio.yaml schema but I wasn't planning on touching
>>> that.
>>> Changes in v2:
>>> - Add Andrew as maintainer (thanks for volunteering)
>>>
>>> .../bindings/net/marvell,orion-mdio.yaml | 60
>>> +++++++++++++++++++
>>> .../bindings/net/marvell-orion-mdio.txt | 54 -----------------
>>> 2 files changed, 60 insertions(+), 54 deletions(-)
>>> create mode 100644
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>> delete mode 100644
>>> Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>> b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>> new file mode 100644
>>> index 000000000000..fe3a3412f093
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>> @@ -0,0 +1,60 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id:
>>> http://scanmail.trustwave.com/?c=20988&d=j5WC4r_pZnDMILajH1kaKLL9oC7kQjgv_bkDWJOhEQ&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fnet%2fmarvell%2corion-mdio%2eyaml%23
>>> +$schema:
>>> http://scanmail.trustwave.com/?c=20988&d=j5WC4r_pZnDMILajH1kaKLL9oC7kQjgv_e4CDcetEw&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>>> +
>>> +title: Marvell MDIO Ethernet Controller interface
>>> +
>>> +maintainers:
>>> + - Andrew Lunn <andrew@lunn.ch>
>>> +
>>> +description: |
>>> + The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x,
>>> MV78xx0,
>>> + Armada 370, Armada XP, Armada 7k and Armada 8k have an identical
>>> unit that
>>> + provides an interface with the MDIO bus. Additionally, Armada 7k
>>> and Armada
>>> + 8k has a second unit which provides an interface with the xMDIO
>>> bus. This
>>> + driver handles these interfaces.
>>> +
>>> +allOf:
>>> + - $ref: "mdio.yaml#"
>>> +
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - marvell,orion-mdio
>>> + - marvell,xmdio
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + interrupts:
>>> + maxItems: 1
>>> +
>>> + clocks:
>>> + minItems: 1
>>> + maxItems: 4
>> Really this should be better defined, but the original was not.
>>
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> +
>>> +unevaluatedProperties: true
>> This must be false.
>
> Right now there is no way (that I have found) of dealing with non-PHY
> devices like the dsa switches so setting this to false generates
> warnings on turris-mox:
>
> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004:
> Unevaluated properties are not allowed ('#address-cells',
> '#size-cells', 'ethernet-phy@1', 'switch0@10', 'switch0@2',
> 'switch1@11', 'switch1@2', 'switch2@12', 'switch2@2' were unexpected)
> From schema:
> /home/chrisp/src/linux/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>
> There are also warnings about the size of the reg property but these
> seem to be genuine problems that Marek is looking at.
Actually it looks if I fix the reg problem the need for
unevaluatedProperties goes away. I'll whip up a small patch series on
top of net-next.
>
> This change has already been picked up for net-next but if you have
> suggestions for improvement I'm happy to submit them as additional
> changes on-top of that.
>
>>
>>> +
>>> +examples:
>>> + - |
>>> + mdio@d0072004 {
>>> + compatible = "marvell,orion-mdio";
>>> + reg = <0xd0072004 0x4>;
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + interrupts = <30>;
>>> +
>>> + phy0: ethernet-phy@0 {
>>> + reg = <0>;
>>> + };
>>> +
>>> + phy1: ethernet-phy@1 {
>>> + reg = <1>;
>>> + };
>>> + };
>>> diff --git
>>> a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>>> b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>>> deleted file mode 100644
>>> index 3f3cfc1d8d4d..000000000000
>>> --- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>>> +++ /dev/null
>>> @@ -1,54 +0,0 @@
>>> -* Marvell MDIO Ethernet Controller interface
>>> -
>>> -The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x,
>>> -MV78xx0, Armada 370, Armada XP, Armada 7k and Armada 8k have an
>>> -identical unit that provides an interface with the MDIO bus.
>>> -Additionally, Armada 7k and Armada 8k has a second unit which
>>> -provides an interface with the xMDIO bus. This driver handles
>>> -these interfaces.
>>> -
>>> -Required properties:
>>> -- compatible: "marvell,orion-mdio" or "marvell,xmdio"
>>> -- reg: address and length of the MDIO registers. When an interrupt is
>>> - not present, the length is the size of the SMI register (4 bytes)
>>> - otherwise it must be 0x84 bytes to cover the interrupt control
>>> - registers.
>>> -
>>> -Optional properties:
>>> -- interrupts: interrupt line number for the SMI error/done interrupt
>>> -- clocks: phandle for up to four required clocks for the MDIO instance
>>> -
>>> -The child nodes of the MDIO driver are the individual PHY devices
>>> -connected to this MDIO bus. They must have a "reg" property given the
>>> -PHY address on the MDIO bus.
>>> -
>>> -Example at the SoC level without an interrupt property:
>>> -
>>> -mdio {
>>> - #address-cells = <1>;
>>> - #size-cells = <0>;
>>> - compatible = "marvell,orion-mdio";
>>> - reg = <0xd0072004 0x4>;
>>> -};
>>> -
>>> -Example with an interrupt property:
>>> -
>>> -mdio {
>>> - #address-cells = <1>;
>>> - #size-cells = <0>;
>>> - compatible = "marvell,orion-mdio";
>>> - reg = <0xd0072004 0x84>;
>>> - interrupts = <30>;
>>> -};
>>> -
>>> -And at the board level:
>>> -
>>> -mdio {
>>> - phy0: ethernet-phy@0 {
>>> - reg = <0>;
>>> - };
>>> -
>>> - phy1: ethernet-phy@1 {
>>> - reg = <1>;
>>> - };
>>> -}
>>> --
>>> 2.36.0
>>>
>>>
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: net: orion-mdio: Convert to JSON schema
From: Chris Packham @ 2022-05-16 22:18 UTC (permalink / raw)
To: Rob Herring
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, krzysztof.kozlowski+dt@linaro.org,
andrew@lunn.ch, netdev@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Marek Behún
In-Reply-To: <20220516181639.GB2997786-robh@kernel.org>
Hi Rob,
On 17/05/22 06:16, Rob Herring wrote:
> On Fri, May 06, 2022 at 09:06:20AM +1200, Chris Packham wrote:
>> Convert the marvell,orion-mdio binding to JSON schema.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>
>> Notes:
>> This does throw up the following dtbs_check warnings for turris-mox:
>>
>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch0@10:reg: [[16], [0]] is too long
>> From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch0@2:reg: [[2], [0]] is too long
>> From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch1@11:reg: [[17], [0]] is too long
>> From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch1@2:reg: [[2], [0]] is too long
>> From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch2@12:reg: [[18], [0]] is too long
>> From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch2@2:reg: [[2], [0]] is too long
>> From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>
>> I think they're all genuine but I'm hesitant to leap in and fix them
>> without being able to test them.
>>
>> I also need to set unevaluatedProperties: true to cater for the L2
>> switch on turris-mox (and probably others). That might be better tackled
>> in the core mdio.yaml schema but I wasn't planning on touching that.
>>
>> Changes in v2:
>> - Add Andrew as maintainer (thanks for volunteering)
>>
>> .../bindings/net/marvell,orion-mdio.yaml | 60 +++++++++++++++++++
>> .../bindings/net/marvell-orion-mdio.txt | 54 -----------------
>> 2 files changed, 60 insertions(+), 54 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>> delete mode 100644 Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>> new file mode 100644
>> index 000000000000..fe3a3412f093
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>> @@ -0,0 +1,60 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://scanmail.trustwave.com/?c=20988&d=j5WC4r_pZnDMILajH1kaKLL9oC7kQjgv_bkDWJOhEQ&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fnet%2fmarvell%2corion-mdio%2eyaml%23
>> +$schema: http://scanmail.trustwave.com/?c=20988&d=j5WC4r_pZnDMILajH1kaKLL9oC7kQjgv_e4CDcetEw&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>> +
>> +title: Marvell MDIO Ethernet Controller interface
>> +
>> +maintainers:
>> + - Andrew Lunn <andrew@lunn.ch>
>> +
>> +description: |
>> + The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x, MV78xx0,
>> + Armada 370, Armada XP, Armada 7k and Armada 8k have an identical unit that
>> + provides an interface with the MDIO bus. Additionally, Armada 7k and Armada
>> + 8k has a second unit which provides an interface with the xMDIO bus. This
>> + driver handles these interfaces.
>> +
>> +allOf:
>> + - $ref: "mdio.yaml#"
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - marvell,orion-mdio
>> + - marvell,xmdio
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + clocks:
>> + minItems: 1
>> + maxItems: 4
> Really this should be better defined, but the original was not.
>
>> +
>> +required:
>> + - compatible
>> + - reg
>> +
>> +unevaluatedProperties: true
> This must be false.
Right now there is no way (that I have found) of dealing with non-PHY
devices like the dsa switches so setting this to false generates
warnings on turris-mox:
arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004:
Unevaluated properties are not allowed ('#address-cells', '#size-cells',
'ethernet-phy@1', 'switch0@10', 'switch0@2', 'switch1@11', 'switch1@2',
'switch2@12', 'switch2@2' were unexpected)
From schema:
/home/chrisp/src/linux/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
There are also warnings about the size of the reg property but these
seem to be genuine problems that Marek is looking at.
This change has already been picked up for net-next but if you have
suggestions for improvement I'm happy to submit them as additional
changes on-top of that.
>
>> +
>> +examples:
>> + - |
>> + mdio@d0072004 {
>> + compatible = "marvell,orion-mdio";
>> + reg = <0xd0072004 0x4>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + interrupts = <30>;
>> +
>> + phy0: ethernet-phy@0 {
>> + reg = <0>;
>> + };
>> +
>> + phy1: ethernet-phy@1 {
>> + reg = <1>;
>> + };
>> + };
>> diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>> deleted file mode 100644
>> index 3f3cfc1d8d4d..000000000000
>> --- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>> +++ /dev/null
>> @@ -1,54 +0,0 @@
>> -* Marvell MDIO Ethernet Controller interface
>> -
>> -The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x,
>> -MV78xx0, Armada 370, Armada XP, Armada 7k and Armada 8k have an
>> -identical unit that provides an interface with the MDIO bus.
>> -Additionally, Armada 7k and Armada 8k has a second unit which
>> -provides an interface with the xMDIO bus. This driver handles
>> -these interfaces.
>> -
>> -Required properties:
>> -- compatible: "marvell,orion-mdio" or "marvell,xmdio"
>> -- reg: address and length of the MDIO registers. When an interrupt is
>> - not present, the length is the size of the SMI register (4 bytes)
>> - otherwise it must be 0x84 bytes to cover the interrupt control
>> - registers.
>> -
>> -Optional properties:
>> -- interrupts: interrupt line number for the SMI error/done interrupt
>> -- clocks: phandle for up to four required clocks for the MDIO instance
>> -
>> -The child nodes of the MDIO driver are the individual PHY devices
>> -connected to this MDIO bus. They must have a "reg" property given the
>> -PHY address on the MDIO bus.
>> -
>> -Example at the SoC level without an interrupt property:
>> -
>> -mdio {
>> - #address-cells = <1>;
>> - #size-cells = <0>;
>> - compatible = "marvell,orion-mdio";
>> - reg = <0xd0072004 0x4>;
>> -};
>> -
>> -Example with an interrupt property:
>> -
>> -mdio {
>> - #address-cells = <1>;
>> - #size-cells = <0>;
>> - compatible = "marvell,orion-mdio";
>> - reg = <0xd0072004 0x84>;
>> - interrupts = <30>;
>> -};
>> -
>> -And at the board level:
>> -
>> -mdio {
>> - phy0: ethernet-phy@0 {
>> - reg = <0>;
>> - };
>> -
>> - phy1: ethernet-phy@1 {
>> - reg = <1>;
>> - };
>> -}
>> --
>> 2.36.0
>>
>>
^ permalink raw reply
* RE: [EXT] [PATCH 2/2] octeon_ep: Fix irq releasing in the error handling path of octep_request_irqs()
From: Veerasenareddy Burru @ 2022-05-16 22:14 UTC (permalink / raw)
To: Christophe JAILLET, Abhijit Ayarekar, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Satananda Burla
Cc: linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <a1b6f082fff4e68007914577961113bc452c8030.1652629833.git.christophe.jaillet@wanadoo.fr>
> -----Original Message-----
> From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Sent: Sunday, May 15, 2022 8:57 AM
> To: Veerasenareddy Burru <vburru@marvell.com>; Abhijit Ayarekar
> <aayarekar@marvell.com>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; Satananda Burla <sburla@marvell.com>
> Cc: linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org;
> Christophe JAILLET <christophe.jaillet@wanadoo.fr>;
> netdev@vger.kernel.org
> Subject: [EXT] [PATCH 2/2] octeon_ep: Fix irq releasing in the error handling
> path of octep_request_irqs()
>
> External Email
>
> ----------------------------------------------------------------------
> For the error handling to work as expected, the index in the 'oct-
> >msix_entries' array must be tweaked because, when the irq are requested
> there is:
> msix_entry = &oct->msix_entries[i + num_non_ioq_msix];
>
> So in the error handling path, 'i + num_non_ioq_msix' should be used instead
> of 'i'.
>
> The 2nd argument of free_irq() also needs to be adjusted.
>
> Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt
> support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> I think that the wording above is awful, but I'm sure you get it.
> Feel free to rephrase everything to have it more readable.
> ---
> drivers/net/ethernet/marvell/octeon_ep/octep_main.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> index 6b60a03574a0..4dcae805422b 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> @@ -257,10 +257,12 @@ static int octep_request_irqs(struct octep_device
> *oct)
>
> return 0;
> ioq_irq_err:
> + i += num_non_ioq_msix;
> while (i > num_non_ioq_msix) {
> --i;
> irq_set_affinity_hint(oct->msix_entries[i].vector, NULL);
> - free_irq(oct->msix_entries[i].vector, oct->ioq_vector[i]);
> + free_irq(oct->msix_entries[i].vector,
> + oct->ioq_vector[i - num_non_ioq_msix]);
Ack.
Thanks for the fix.
Regards,
Veerasenareddy.
> }
> non_ioq_irq_err:
> while (i) {
> --
> 2.34.1
^ permalink raw reply
* RE: [EXT] [PATCH 1/2] octeon_ep: Fix a memory leak in the error handling path of octep_request_irqs()
From: Veerasenareddy Burru @ 2022-05-16 22:06 UTC (permalink / raw)
To: Christophe JAILLET, Abhijit Ayarekar, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Satananda Burla
Cc: linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <78dcfbb5d22328bc83edbfc74af10c3625c54087.1652629833.git.christophe.jaillet@wanadoo.fr>
> -----Original Message-----
> From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Sent: Sunday, May 15, 2022 8:57 AM
> To: Veerasenareddy Burru <vburru@marvell.com>; Abhijit Ayarekar
> <aayarekar@marvell.com>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; Satananda Burla <sburla@marvell.com>
> Cc: linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org;
> Christophe JAILLET <christophe.jaillet@wanadoo.fr>;
> netdev@vger.kernel.org
> Subject: [EXT] [PATCH 1/2] octeon_ep: Fix a memory leak in the error
> handling path of octep_request_irqs()
>
> External Email
>
> ----------------------------------------------------------------------
> 'oct->non_ioq_irq_names' is not freed in the error handling path of
> octep_request_irqs().
>
> Add the missing kfree().
>
> Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt
> support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> drivers/net/ethernet/marvell/octeon_ep/octep_main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> index e020c81f3455..6b60a03574a0 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> @@ -267,6 +267,8 @@ static int octep_request_irqs(struct octep_device
> *oct)
> --i;
> free_irq(oct->msix_entries[i].vector, oct);
> }
> + kfree(oct->non_ioq_irq_names);
> + oct->non_ioq_irq_names = NULL;
Ack
Thanks for the change.
Regards,
Veerasenareddy
> alloc_err:
> return -1;
> }
> --
> 2.34.1
^ permalink raw reply
* [PATCH net-next] net: ifdefy the wireless pointers in struct net_device
From: Jakub Kicinski @ 2022-05-16 21:56 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, Jakub Kicinski, johannes, alex.aring,
stefan, mareklindner, sw, a, sven, linux-wireless, linux-wpan
Most protocol-specific pointers in struct net_device are under
a respective ifdef. Wireless is the notable exception. Since
there's a sizable number of custom-built kernels for datacenter
workloads which don't build wireless it seems reasonable to
ifdefy those pointers as well.
While at it move IPv4 and IPv6 pointers up, those are special
for obvious reasons.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: johannes@sipsolutions.net
CC: alex.aring@gmail.com
CC: stefan@datenfreihafen.org
CC: mareklindner@neomailbox.ch
CC: sw@simonwunderlich.de
CC: a@unstable.cc
CC: sven@narfation.org
CC: linux-wireless@vger.kernel.org
CC: linux-wpan@vger.kernel.org
---
include/linux/netdevice.h | 8 ++++++--
include/net/cfg80211.h | 5 +----
include/net/cfg802154.h | 2 ++
net/batman-adv/hard-interface.c | 2 ++
net/wireless/core.c | 6 ++++++
5 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 536321691c72..f0604863f18c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2119,6 +2119,8 @@ struct net_device {
/* Protocol-specific pointers */
+ struct in_device __rcu *ip_ptr;
+ struct inet6_dev __rcu *ip6_ptr;
#if IS_ENABLED(CONFIG_VLAN_8021Q)
struct vlan_info __rcu *vlan_info;
#endif
@@ -2131,16 +2133,18 @@ struct net_device {
#if IS_ENABLED(CONFIG_ATALK)
void *atalk_ptr;
#endif
- struct in_device __rcu *ip_ptr;
#if IS_ENABLED(CONFIG_DECNET)
struct dn_dev __rcu *dn_ptr;
#endif
- struct inet6_dev __rcu *ip6_ptr;
#if IS_ENABLED(CONFIG_AX25)
void *ax25_ptr;
#endif
+#if IS_ENABLED(CONFIG_WIRELESS)
struct wireless_dev *ieee80211_ptr;
+#endif
+#if IS_ENABLED(CONFIG_IEEE802154) || IS_ENABLED(CONFIG_6LOWPAN)
struct wpan_dev *ieee802154_ptr;
+#endif
#if IS_ENABLED(CONFIG_MPLS_ROUTING)
struct mpls_dev __rcu *mpls_ptr;
#endif
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 68713388b617..a4a7fc3241cf 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -8004,10 +8004,7 @@ int cfg80211_register_netdevice(struct net_device *dev);
*
* Requires the RTNL and wiphy mutex to be held.
*/
-static inline void cfg80211_unregister_netdevice(struct net_device *dev)
-{
- cfg80211_unregister_wdev(dev->ieee80211_ptr);
-}
+void cfg80211_unregister_netdevice(struct net_device *dev);
/**
* struct cfg80211_ft_event_params - FT Information Elements
diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 85f9e8417688..d8d8719315fd 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -373,6 +373,7 @@ struct wpan_dev {
#define to_phy(_dev) container_of(_dev, struct wpan_phy, dev)
+#if IS_ENABLED(CONFIG_IEEE802154) || IS_ENABLED(CONFIG_6LOWPAN)
static inline int
wpan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
const struct ieee802154_addr *daddr,
@@ -383,6 +384,7 @@ wpan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
return wpan_dev->header_ops->create(skb, dev, daddr, saddr, len);
}
+#endif
struct wpan_phy *
wpan_phy_new(const struct cfg802154_ops *ops, size_t priv_size);
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 83fb51b6e299..15d2bb4cd301 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -307,9 +307,11 @@ static bool batadv_is_cfg80211_netdev(struct net_device *net_device)
if (!net_device)
return false;
+#if IS_ENABLED(CONFIG_WIRELESS)
/* cfg80211 drivers have to set ieee80211_ptr */
if (net_device->ieee80211_ptr)
return true;
+#endif
return false;
}
diff --git a/net/wireless/core.c b/net/wireless/core.c
index f08d4b3bb148..a24944e6d01e 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1374,6 +1374,12 @@ int cfg80211_register_netdevice(struct net_device *dev)
}
EXPORT_SYMBOL(cfg80211_register_netdevice);
+void cfg80211_unregister_netdevice(struct net_device *dev)
+{
+ cfg80211_unregister_wdev(dev->ieee80211_ptr);
+}
+EXPORT_SYMBOL(cfg80211_unregister_netdevice);
+
static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
unsigned long state, void *ptr)
{
--
2.34.3
^ permalink raw reply related
* Re: [PATCH bpf 1/4] bpf_trace: check size for overflow in bpf_kprobe_multi_link_attach
From: Jiri Olsa @ 2022-05-16 21:34 UTC (permalink / raw)
To: Eugene Syromiatnikov
Cc: Masami Hiramatsu, Steven Rostedt, Ingo Molnar, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, netdev, bpf,
linux-kernel, Shuah Khan, linux-kselftest
In-Reply-To: <20220516182708.GA29437@asgard.redhat.com>
On Mon, May 16, 2022 at 08:27:08PM +0200, Eugene Syromiatnikov wrote:
> Check that size would not overflow before calculation (and return
> -EOVERFLOW if it will), to prevent potential out-of-bounds write
> with the following copy_from_user. Add the same check
> to kprobe_multi_resolve_syms in case it will be called from elsewhere
> in the future.
>
> Fixes: 0dcac272540613d4 ("bpf: Add multi kprobe link")
> Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
> ---
> kernel/trace/bpf_trace.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index d8553f4..e90c4ce7 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2358,6 +2358,8 @@ kprobe_multi_resolve_syms(const void __user *usyms, u32 cnt,
> unsigned int i;
> char *func;
>
> + if (check_mul_overflow(cnt, sizeof(*syms), &size))
> + return -EOVERFLOW;
there was an update already:
0236fec57a15 bpf: Resolve symbols with ftrace_lookup_symbols for kprobe multi link
so this won't apply anymore, could you please rebase on top of the latest bpf-next/master?
thanks,
jirka
> size = cnt * sizeof(*syms);
> syms = kvzalloc(size, GFP_KERNEL);
> if (!syms)
> @@ -2429,6 +2431,8 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> if (!cnt)
> return -EINVAL;
>
> + if (check_mul_overflow(cnt, (u32)sizeof(*addrs), &size))
> + return -EOVERFLOW;
> size = cnt * sizeof(*addrs);
> addrs = kvmalloc(size, GFP_KERNEL);
> if (!addrs)
> --
> 2.1.4
>
^ permalink raw reply
* Re: [PATCH v5 09/15] seltests/landlock: add tests for bind() hooks
From: Mickaël Salaün @ 2022-05-16 21:11 UTC (permalink / raw)
To: Konstantin Meskhidze
Cc: willemdebruijn.kernel, linux-security-module, netdev,
netfilter-devel, yusongping, anton.sirazetdinov
In-Reply-To: <20220516152038.39594-10-konstantin.meskhidze@huawei.com>
On 16/05/2022 17:20, Konstantin Meskhidze wrote:
> Adds selftests for bind socket action.
> The first is with no landlock restrictions:
> - bind_no_restrictions_ip4;
> - bind_no_restrictions_ip6;
> The second ones is with mixed landlock rules:
> - bind_with_restrictions_ip4;
> - bind_with_restrictions_ip6;
>
> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> ---
>
> Changes since v3:
> * Split commit.
> * Add helper create_socket.
> * Add FIXTURE_SETUP.
>
> Changes since v4:
> * Adds port[MAX_SOCKET_NUM], struct sockaddr_in addr4
> and struct sockaddr_in addr6 in FIXTURE.
> * Refactoring FIXTURE_SETUP:
> - initializing self->port, self->addr4 and self->addr6.
> - adding network namespace.
> * Refactoring code with self->port, self->addr4 and
> self->addr6 variables.
> * Adds selftests for IP6 family:
> - bind_no_restrictions_ip6.
> - bind_with_restrictions_ip6.
> * Refactoring selftests/landlock/config
> * Moves enforce_ruleset() into common.h
>
> ---
> tools/testing/selftests/landlock/common.h | 9 +
> tools/testing/selftests/landlock/config | 5 +-
> tools/testing/selftests/landlock/fs_test.c | 10 -
> tools/testing/selftests/landlock/net_test.c | 237 ++++++++++++++++++++
> 4 files changed, 250 insertions(+), 11 deletions(-)
> create mode 100644 tools/testing/selftests/landlock/net_test.c
>
> diff --git a/tools/testing/selftests/landlock/common.h b/tools/testing/selftests/landlock/common.h
> index 7ba18eb23783..c5381e641dfd 100644
> --- a/tools/testing/selftests/landlock/common.h
> +++ b/tools/testing/selftests/landlock/common.h
> @@ -102,6 +102,15 @@ static inline int landlock_restrict_self(const int ruleset_fd,
> }
> #endif
>
> +static void enforce_ruleset(struct __test_metadata *const _metadata,
> + const int ruleset_fd)
> +{
> + ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
> + ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0)) {
> + TH_LOG("Failed to enforce ruleset: %s", strerror(errno));
> + }
> +}
> +
Please create a commit which moves all the needed code for all network
tests. I think there is only this helper though.
> static void _init_caps(struct __test_metadata *const _metadata, bool drop_all)
> {
> cap_t cap_p;
> diff --git a/tools/testing/selftests/landlock/config b/tools/testing/selftests/landlock/config
> index 0f0a65287bac..b56f3274d3f5 100644
> --- a/tools/testing/selftests/landlock/config
> +++ b/tools/testing/selftests/landlock/config
> @@ -1,7 +1,10 @@
> +CONFIG_INET=y
> +CONFIG_IPV6=y
> +CONFIG_NET=y
> CONFIG_OVERLAY_FS=y
> CONFIG_SECURITY_LANDLOCK=y
> CONFIG_SECURITY_PATH=y
> CONFIG_SECURITY=y
> CONFIG_SHMEM=y
> CONFIG_TMPFS_XATTR=y
> -CONFIG_TMPFS=y
> +CONFIG_TMPFS=y
> \ No newline at end of file
You add whitespace changes.
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 21a2ce8fa739..036dd6f8f9ea 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -551,16 +551,6 @@ static int create_ruleset(struct __test_metadata *const _metadata,
> return ruleset_fd;
> }
>
> -static void enforce_ruleset(struct __test_metadata *const _metadata,
> - const int ruleset_fd)
> -{
> - ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
> - ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0))
> - {
> - TH_LOG("Failed to enforce ruleset: %s", strerror(errno));
> - }
> -}
> -
> TEST_F_FORK(layout1, proc_nsfs)
> {
> const struct rule rules[] = {
> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
> new file mode 100644
> index 000000000000..478ef2eff559
> --- /dev/null
> +++ b/tools/testing/selftests/landlock/net_test.c
> @@ -0,0 +1,237 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Landlock tests - Network
> + *
> + * Copyright (C) 2022 Huawei Tech. Co., Ltd.
> + */
> +
> +#define _GNU_SOURCE
> +#include <arpa/inet.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <linux/landlock.h>
> +#include <netinet/in.h>
> +#include <sched.h>
> +#include <string.h>
> +#include <sys/prctl.h>
> +#include <sys/socket.h>
> +#include <sys/types.h>
> +
> +#include "common.h"
> +
> +#define MAX_SOCKET_NUM 10
> +
> +#define SOCK_PORT_START 3470
> +#define SOCK_PORT_ADD 10
> +
> +#define IP_ADDRESS "127.0.0.1"
> +
> +/* Number pending connections queue to be hold */
> +#define BACKLOG 10
"Number of pending connection queues to be hold." maybe? This is not use
in this patch so it shouldn't be added by this patch.
> +
> +static int create_socket(struct __test_metadata *const _metadata,
> + bool ip6, bool reuse_addr)
This helper is good and I think you can improve it by leveraging test
variants. You could even factor out all the ipv4/ipv6 tests thanks to
new helpers such as bind_variant() and connect_variant(). No need to add
_metadata to those though. This would avoid duplicating all ipv4/ipv6
tests and even simplifying bind() and connect() calls. Something like this:
// rename "socket_test" to "socket" (no need to duplicate "test")
FIXTURE_VARIANT(socket)
{
const bool is_ipv4;
};
/* clang-format off */
FIXTURE_VARIANT_ADD(socket, ipv4) {
/* clang-format on */
.is_ipv4 = true,
};
/* clang-format off */
FIXTURE_VARIANT_ADD(socket, ipv6) {
/* clang-format on */
.is_ipv4 = false,
};
static int socket_variant(const FIXTURE_VARIANT(socket) *const variant,
const int type)
{
if (variant->is_ipv4)
return socket(AF_INET, type | SOCK_CLOEXEC, 0);
else
return socket(AF_INET6, type | SOCK_CLOEXEC, 0);
}
socket_variant(variant, SOCK_STREAM);
// this could be used to create UDP sockets too
static int bind_variant(const FIXTURE_VARIANT(socket) *const variant,
const int sockfd, const FIXTURE_DATA(socket) *const self, const size_t
index)
{
if (variant->is_ipv4)
return bind(sockfd, &self->addr4[index], sizeof(self->addr4[index]));
else
return bind(sockfd, &self->addr6[index], sizeof(self->addr6[index]));
}
bind_variant(variant, sockfd, self, 0);
> +{
> + int sockfd;
> + int one = 1;
> +
> + if (ip6)
> + sockfd = socket(AF_INET6, SOCK_STREAM | SOCK_CLOEXEC, 0);
> + else
> + sockfd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
> +
> + ASSERT_LE(0, sockfd);
> + /* Allows to reuse of local address */
> + if (reuse_addr)
> + ASSERT_EQ(0, setsockopt(sockfd, SOL_SOCKET,
> + SO_REUSEADDR, &one, sizeof(one)));
This reuse_addr part is not used in this patch and I think it would
simplify this helper to not add reuse_addr but to explicitely call
setsockopt() when required. This also enables to get rid of _metadata in
this helper.
> + return sockfd;
> +}
> +
> +FIXTURE(socket_test) {
> + uint port[MAX_SOCKET_NUM];
> + struct sockaddr_in addr4[MAX_SOCKET_NUM];
> + struct sockaddr_in6 addr6[MAX_SOCKET_NUM];
> +};
> +
> +FIXTURE_SETUP(socket_test)
> +{
> + int i;
> + /* Creates IP4 socket addresses */
> + for (i = 0; i < MAX_SOCKET_NUM; i++) {
Nice!
> + self->port[i] = SOCK_PORT_START + SOCK_PORT_ADD*i;
> + self->addr4[i].sin_family = AF_INET;
> + self->addr4[i].sin_port = htons(self->port[i]);
> + self->addr4[i].sin_addr.s_addr = htonl(INADDR_ANY);
Could you use the local addr (127.0.0.1) instead?
> + memset(&(self->addr4[i].sin_zero), '\0', 8);
> + }
> +
> + /* Creates IP6 socket addresses */
> + for (i = 0; i < MAX_SOCKET_NUM; i++) {
> + self->port[i] = SOCK_PORT_START + SOCK_PORT_ADD*i;
> + self->addr6[i].sin6_family = AF_INET6;
> + self->addr6[i].sin6_port = htons(self->port[i]);
> + self->addr6[i].sin6_addr = in6addr_any;
ditto
> + }
> +
> + set_cap(_metadata, CAP_SYS_ADMIN);
> + ASSERT_EQ(0, unshare(CLONE_NEWNET));
> + ASSERT_EQ(0, system("ip link set dev lo up"));
If this is really required, could you avoid calling system() but set up
the network in C? You can strace it to see what is going on underneath.
> + clear_cap(_metadata, CAP_SYS_ADMIN);
> +}
> +
> +FIXTURE_TEARDOWN(socket_test)
> +{ }
> +
> +TEST_F_FORK(socket_test, bind_no_restrictions_ip4) {
> +
> + int sockfd;
> +
> + sockfd = create_socket(_metadata, false, false);
> + ASSERT_LE(0, sockfd);
> +
> + /* Binds a socket to port[0] */
This comment is not very useful in this context considering the below
line. It will be even more clear with the bind_variant() call.
> + ASSERT_EQ(0, bind(sockfd, (struct sockaddr *)&self->addr4[0], sizeof(self->addr4[0])));
> +
> + ASSERT_EQ(0, close(sockfd));
> +}
> +
> +TEST_F_FORK(socket_test, bind_no_restrictions_ip6) {
> +
> + int sockfd;
> +
> + sockfd = create_socket(_metadata, true, false);
> + ASSERT_LE(0, sockfd);
> +
> + /* Binds a socket to port[0] */
> + ASSERT_EQ(0, bind(sockfd, (struct sockaddr *)&self->addr6[0], sizeof(self->addr6[0])));
> +
> + ASSERT_EQ(0, close(sockfd));
> +}
> +
> +TEST_F_FORK(socket_test, bind_with_restrictions_ip4) {
> +
> + int sockfd;
> +
> + struct landlock_ruleset_attr ruleset_attr = {
> + .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
> + LANDLOCK_ACCESS_NET_CONNECT_TCP,
> + };
> + struct landlock_net_service_attr net_service_1 = {
> + .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
> + LANDLOCK_ACCESS_NET_CONNECT_TCP,
> + .port = self->port[0],
> + };
> + struct landlock_net_service_attr net_service_2 = {
> + .allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
> + .port = self->port[1],
> + };
> + struct landlock_net_service_attr net_service_3 = {
> + .allowed_access = 0,
> + .port = self->port[2],
> + };
> +
> + const int ruleset_fd = landlock_create_ruleset(&ruleset_attr,
> + sizeof(ruleset_attr), 0);
> + ASSERT_LE(0, ruleset_fd);
> +
> + /* Allows connect and bind operations to the port[0] socket. */
This comment is useful though because the below call is more complex.
> + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> + &net_service_1, 0));
> + /* Allows connect and deny bind operations to the port[1] socket. */
> + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> + &net_service_2, 0));
> + /* Empty allowed_access (i.e. deny rules) are ignored in network actions
> + * for port[2] socket.
> + */
> + ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> + &net_service_3, 0));
> + ASSERT_EQ(ENOMSG, errno);
> +
> + /* Enforces the ruleset. */
> + enforce_ruleset(_metadata, ruleset_fd);
> +
> + sockfd = create_socket(_metadata, false, false);
> + ASSERT_LE(0, sockfd);
> + /* Binds a socket to port[0] */
> + ASSERT_EQ(0, bind(sockfd, (struct sockaddr *)&self->addr4[0], sizeof(self->addr4[0])));
> +
> + /* Close bounded socket*/
> + ASSERT_EQ(0, close(sockfd));
> +
> + sockfd = create_socket(_metadata, false, false);
> + ASSERT_LE(0, sockfd);
> + /* Binds a socket to port[1] */
> + ASSERT_EQ(-1, bind(sockfd, (struct sockaddr *)&self->addr4[1], sizeof(self->addr4[1])));
> + ASSERT_EQ(EACCES, errno);
> +
> + sockfd = create_socket(_metadata, false, false);
> + ASSERT_LE(0, sockfd);
> + /* Binds a socket to port[2] */
> + ASSERT_EQ(-1, bind(sockfd, (struct sockaddr *)&self->addr4[2], sizeof(self->addr4[2])));
> + ASSERT_EQ(EACCES, errno);
> +}
> +
> +TEST_F_FORK(socket_test, bind_with_restrictions_ip6) {
> +
> + int sockfd;
> +
> + struct landlock_ruleset_attr ruleset_attr = {
> + .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
> + LANDLOCK_ACCESS_NET_CONNECT_TCP,
> + };
> + struct landlock_net_service_attr net_service_1 = {
> + .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
> + LANDLOCK_ACCESS_NET_CONNECT_TCP,
> + .port = self->port[0],
> + };
> + struct landlock_net_service_attr net_service_2 = {
> + .allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
> + .port = self->port[1],
> + };
> + struct landlock_net_service_attr net_service_3 = {
> + .allowed_access = 0,
> + .port = self->port[2],
> + };
> +
> + const int ruleset_fd = landlock_create_ruleset(&ruleset_attr,
> + sizeof(ruleset_attr), 0);
> + ASSERT_LE(0, ruleset_fd);
> +
> + /* Allows connect and bind operations to the port[0] socket. */
> + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> + &net_service_1, 0));
> + /* Allows connect and deny bind operations to the port[1] socket. */
> + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> + &net_service_2, 0));
> + /* Empty allowed_access (i.e. deny rules) are ignored in network actions
> + * for port[2] socket.
> + */
> + ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> + &net_service_3, 0));
> + ASSERT_EQ(ENOMSG, errno);
> +
> + /* Enforces the ruleset. */
> + enforce_ruleset(_metadata, ruleset_fd);
> +
> + sockfd = create_socket(_metadata, true, false);
> + ASSERT_LE(0, sockfd);
> + /* Binds a socket to port[0] */
> + ASSERT_EQ(0, bind(sockfd, (struct sockaddr *)&self->addr6[0], sizeof(self->addr6[0])));
> +
> + /* Close bounded socket*/
> + ASSERT_EQ(0, close(sockfd));
> +
> + sockfd = create_socket(_metadata, false, false);
> + ASSERT_LE(0, sockfd);
> + /* Binds a socket to port[1] */
> + ASSERT_EQ(-1, bind(sockfd, (struct sockaddr *)&self->addr6[1], sizeof(self->addr6[1])));
> + ASSERT_EQ(EACCES, errno);
> +
> + sockfd = create_socket(_metadata, false, false);
> + ASSERT_LE(0, sockfd);
> + /* Binds a socket to port[2] */
> + ASSERT_EQ(-1, bind(sockfd, (struct sockaddr *)&self->addr6[2], sizeof(self->addr6[2])));
> + ASSERT_EQ(EACCES, errno);
> +}
> +TEST_HARNESS_MAIN
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH net-next] net: wwan: t7xx: fix GFP_KERNEL usage in spin_lock context
From: Martinez, Ricardo @ 2022-05-16 20:57 UTC (permalink / raw)
To: Sergey Ryazanov, Ziyang Xuan
Cc: Devegowda, Chandrashekar, Intel Corporation, chiranjeevi.rapolu,
Haijun Liu (刘海军), M Chetan Kumar,
Loic Poulain, David Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, open list
In-Reply-To: <CAHNKnsS0D8bRA5GY0xss2ZUCwY2HoLNMgeR0K4ecH-HfmdTefg@mail.gmail.com>
On 5/16/2022 1:36 PM, Sergey Ryazanov wrote:
> Hello Ziyang,
>
> On Sat, May 14, 2022 at 11:57 AM Ziyang Xuan
> <william.xuanziyang@huawei.com> wrote:
>> t7xx_cldma_clear_rxq() call t7xx_cldma_alloc_and_map_skb() in spin_lock
>> context, But __dev_alloc_skb() in t7xx_cldma_alloc_and_map_skb() uses
>> GFP_KERNEL, that will introduce scheduling factor in spin_lock context.
>>
>> Replace GFP_KERNEL with GFP_ATOMIC to fix it.
> Would not it will be more reliable to just rework
> t7xx_cldma_clear_rxq() to avoid calling t7xx_cldma_alloc_and_map_skb()
> under the spin lock instead of doing each allocation with GFP_ATOMIC?
> E.g. t7xx_cldma_gpd_rx_from_q() calls t7xx_cldma_alloc_and_map_skb()
> avoiding any lock holding.
t7xx_cldma_clear_rxq() is a helper for t7xx_cldma_clear_all_qs() which
is only called by t7xx_cldma_exception() after stopping CLDMA, so it
should be OK to remove the spin lock from t7xx_cldma_clear_rxq().
^ permalink raw reply
* Re: [PATCH net-next v3 00/10] UDP/IPv6 refactoring
From: Pavel Begunkov @ 2022-05-16 20:48 UTC (permalink / raw)
To: Paolo Abeni, netdev, David S . Miller, Jakub Kicinski
Cc: David Ahern, Eric Dumazet, linux-kernel
In-Reply-To: <b9025eb4d8a1efefbcd04013cbe8e55e98ef66e1.camel@redhat.com>
On 5/16/22 14:48, Paolo Abeni wrote:
> Hello,
>
> On Fri, 2022-05-13 at 16:26 +0100, Pavel Begunkov wrote:
>> Refactor UDP/IPv6 and especially udpv6_sendmsg() paths. The end result looks
>> cleaner than it was before and the series also removes a bunch of instructions
>> and other overhead from the hot path positively affecting performance.
>>
>> Testing over dummy netdev with 16 byte packets yields 2240481 tx/s,
>> comparing to 2203417 tx/s previously, which is around +1.6%
>
> I personally feel that some patches in this series have a relevant
> chance of introducing functional regressions and e.g. syzbot will not
> help to catch them. That risk is IMHO relevant considered that the
> performance gain here looks quite limited.
I can't say I agree with that. First, I do think the code is much
cleaner having just one block checking corking instead of a couple
of random ifs in different places. Same for sin6. Not to mention
negative line count.
Also, assuming this 1.6% translates to ~0.5-1% with fast NICs, that's
still huge, especially when we get >5GB/s in single core zc tests b/w
servers.
If maintainers are not merging it, I think I'll delay the series until
I get another batch of planned optimisations implemented on top.
> There are a few individual changes that IMHO looks like nice cleanup
> e.g. patch 5, 6, 8, 9 and possibly even patch 1.
>
> I suggest to reduce the patchset scope to them.
--
Pavel Begunkov
^ permalink raw reply
* Re: [PATCH net-next 3/4] net: add skb_defer_max sysctl
From: Eric Dumazet @ 2022-05-16 20:43 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Eric Dumazet, David S . Miller, Paolo Abeni, netdev
In-Reply-To: <20220516133941.7da6bac7@kernel.org>
On Mon, May 16, 2022 at 1:39 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sun, 15 May 2022 21:24:55 -0700 Eric Dumazet wrote:
> > @@ -6494,16 +6495,21 @@ void skb_attempt_defer_free(struct sk_buff *skb)
> > int cpu = skb->alloc_cpu;
> > struct softnet_data *sd;
> > unsigned long flags;
> > + unsigned int defer_max;
> > bool kick;
> >
> > if (WARN_ON_ONCE(cpu >= nr_cpu_ids) ||
> > !cpu_online(cpu) ||
> > cpu == raw_smp_processor_id()) {
> > - __kfree_skb(skb);
> > +nodefer: __kfree_skb(skb);
> > return;
> > }
> >
> > sd = &per_cpu(softnet_data, cpu);
> > + defer_max = READ_ONCE(sysctl_skb_defer_max);
> > + if (READ_ONCE(sd->defer_count) >= defer_max)
> > + goto nodefer;
> > +
> > /* We do not send an IPI or any signal.
> > * Remote cpu will eventually call skb_defer_free_flush()
> > */
> > @@ -6513,11 +6519,8 @@ void skb_attempt_defer_free(struct sk_buff *skb)
> > WRITE_ONCE(sd->defer_list, skb);
> > sd->defer_count++;
> >
> > - /* kick every time queue length reaches 128.
> > - * This condition should hardly be hit under normal conditions,
> > - * unless cpu suddenly stopped to receive NIC interrupts.
> > - */
> > - kick = sd->defer_count == 128;
> > + /* Send an IPI every time queue reaches half capacity. */
> > + kick = sd->defer_count == (defer_max >> 1);
>
> nit: it will behave a little strangely for defer_max == 1
> we'll let one skb get onto the list and free the subsequent
> skbs directly but we'll never kick the IPI
Yes, I was aware of this, but decided it was not a big deal.
Presumably people will be interested to disable the thing completely,
I am not sure about defer_max == 1
>
> Moving the sd->defer_count++; should fix it and have no significant
> side effects. I think.
SGTM, thanks !
^ permalink raw reply
* Re: [PATCH net-next 3/4] net: add skb_defer_max sysctl
From: Jakub Kicinski @ 2022-05-16 20:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, Paolo Abeni, netdev, Eric Dumazet
In-Reply-To: <20220516042456.3014395-4-eric.dumazet@gmail.com>
On Sun, 15 May 2022 21:24:55 -0700 Eric Dumazet wrote:
> @@ -6494,16 +6495,21 @@ void skb_attempt_defer_free(struct sk_buff *skb)
> int cpu = skb->alloc_cpu;
> struct softnet_data *sd;
> unsigned long flags;
> + unsigned int defer_max;
> bool kick;
>
> if (WARN_ON_ONCE(cpu >= nr_cpu_ids) ||
> !cpu_online(cpu) ||
> cpu == raw_smp_processor_id()) {
> - __kfree_skb(skb);
> +nodefer: __kfree_skb(skb);
> return;
> }
>
> sd = &per_cpu(softnet_data, cpu);
> + defer_max = READ_ONCE(sysctl_skb_defer_max);
> + if (READ_ONCE(sd->defer_count) >= defer_max)
> + goto nodefer;
> +
> /* We do not send an IPI or any signal.
> * Remote cpu will eventually call skb_defer_free_flush()
> */
> @@ -6513,11 +6519,8 @@ void skb_attempt_defer_free(struct sk_buff *skb)
> WRITE_ONCE(sd->defer_list, skb);
> sd->defer_count++;
>
> - /* kick every time queue length reaches 128.
> - * This condition should hardly be hit under normal conditions,
> - * unless cpu suddenly stopped to receive NIC interrupts.
> - */
> - kick = sd->defer_count == 128;
> + /* Send an IPI every time queue reaches half capacity. */
> + kick = sd->defer_count == (defer_max >> 1);
nit: it will behave a little strangely for defer_max == 1
we'll let one skb get onto the list and free the subsequent
skbs directly but we'll never kick the IPI
Moving the sd->defer_count++; should fix it and have no significant
side effects. I think.
^ permalink raw reply
* Re: [PATCH v3 1/4] can: slcan: use can_dropped_invalid_skb() instead of manual check
From: Marc Kleine-Budde @ 2022-05-16 20:40 UTC (permalink / raw)
To: Vincent Mailhol
Cc: linux-can, linux-kernel, Max Staudt, Oliver Hartkopp, netdev
In-Reply-To: <20220514141650.1109542-2-mailhol.vincent@wanadoo.fr>
[-- Attachment #1: Type: text/plain, Size: 616 bytes --]
On 14.05.2022 23:16:47, Vincent Mailhol wrote:
> slcan does a manual check in slc_xmit() to verify if the skb is
> valid. This check is incomplete, use instead
> can_dropped_invalid_skb().
>
> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
I've taken this patch into the latest pull request to net-next.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 0/2] net: skb: Remove skb_data_area_size()
From: Sergey Ryazanov @ 2022-05-16 20:39 UTC (permalink / raw)
To: Ricardo Martinez
Cc: netdev, linux-wireless, Jakub Kicinski, David Miller,
Johannes Berg, Loic Poulain, M Chetan Kumar,
Devegowda, Chandrashekar, Intel Corporation, chiranjeevi.rapolu,
Haijun Liu (刘海军), Andy Shevchenko,
Sharma, Dinesh, Ilpo Järvinen, Veleta, Moises,
Kancharla, Sreehari
In-Reply-To: <20220513173400.3848271-1-ricardo.martinez@linux.intel.com>
Hello Ricardo,
On Fri, May 13, 2022 at 8:35 PM Ricardo Martinez
<ricardo.martinez@linux.intel.com> wrote:
> This patch series removes the skb_data_area_size() helper,
> replacing it in t7xx driver with the size used during skb allocation.
>
> https://lore.kernel.org/netdev/CAHNKnsTmH-rGgWi3jtyC=ktM1DW2W1VJkYoTMJV2Z_Bt498bsg@mail.gmail.com/
>
> Ricardo Martinez (2):
> net: wwan: t7xx: Avoid calls to skb_data_area_size()
> net: skb: Remove skb_data_area_size()
>
> drivers/net/wwan/t7xx/t7xx_hif_cldma.c | 7 +++----
> drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c | 6 ++----
> include/linux/skbuff.h | 5 -----
> 3 files changed, 5 insertions(+), 13 deletions(-)
Thank you for taking care of this!
Reviewed-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
^ permalink raw reply
* Re: [PATCH bpf] bpf_trace: bail out from bpf_kprobe_multi_link_attach when in compat
From: Jiri Olsa @ 2022-05-16 20:38 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Eugene Syromiatnikov, Jiri Olsa, Masami Hiramatsu, Steven Rostedt,
Ingo Molnar, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Network Development, bpf, LKML
In-Reply-To: <CAADnVQ+2gwhcMht4PuDnDOFKY68Wsq8QFz4Y69NBX_TLaSexQQ@mail.gmail.com>
On Tue, May 10, 2022 at 03:30:10PM -0700, Alexei Starovoitov wrote:
> On Tue, May 10, 2022 at 11:42 AM Eugene Syromiatnikov <esyr@redhat.com> wrote:
> >
> > On Tue, May 10, 2022 at 11:10:35AM -0700, Alexei Starovoitov wrote:
> > > On Fri, May 6, 2022 at 7:22 AM Eugene Syromiatnikov <esyr@redhat.com> wrote:
> > > >
> > > > Since bpf_kprobe_multi_link_attach doesn't support 32-bit kernels
> > > > for whatever reason,
> > >
> > > Jiri,
> > > why did you add this restriction?
sorry, I overlooked this email
the reason for that check is that we link addrs array with cookies
which are u64 and we need to swap cookies together with addrs when
we sort them
but now when I look at that, that could perhaps work event if
unsigned long is 32 bits, will check
> > >
> > > > having it enabled for compat processes on 64-bit
> > > > kernels makes even less sense due to discrepances in the type sizes
> > > > that it does not handle.
> > >
> > > I don't follow this logic.
> > > bpf progs are always 64-bit. Even when user space is 32-bit.
> > > Jiri's check is for the kernel.
> >
> > The interface as defined (and implemented in libbpf) expects arrays of userspace
> > pointers to be passed (for example, syms points to an array of userspace
> > pointers—character strings; same goes for addrs, but with generic userspace
> > pointers) without regard to possible difference in the pointer size in case
> > of compat userspace.
>
> I see. So kprobe_multi.syms and kprobe_multi.addrs will be 'long'
> and 32-bit user space will have an issue with the 64-bit kernel.
> Let's fix it properly.
> We can remove sizeof(u64) != sizeof(void *) and keep libbpf as-is
> by keeping syms and addrs 'long' in uapi.
> As far as I can see 32-bit user space on a 32-bit kernel
> should work with existing code.
> in_compat_syscall() can be used to extend addrs/syms.
I'll check Eugene's new patchset
jirka
^ permalink raw reply
* Re: [PATCH net-next] net: wwan: t7xx: fix GFP_KERNEL usage in spin_lock context
From: Sergey Ryazanov @ 2022-05-16 20:36 UTC (permalink / raw)
To: Ziyang Xuan
Cc: Devegowda, Chandrashekar, Intel Corporation, chiranjeevi.rapolu,
Haijun Liu (刘海军), M Chetan Kumar,
Ricardo Martinez, Loic Poulain, David Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, open list
In-Reply-To: <20220514091443.4150162-1-william.xuanziyang@huawei.com>
Hello Ziyang,
On Sat, May 14, 2022 at 11:57 AM Ziyang Xuan
<william.xuanziyang@huawei.com> wrote:
> t7xx_cldma_clear_rxq() call t7xx_cldma_alloc_and_map_skb() in spin_lock
> context, But __dev_alloc_skb() in t7xx_cldma_alloc_and_map_skb() uses
> GFP_KERNEL, that will introduce scheduling factor in spin_lock context.
>
> Replace GFP_KERNEL with GFP_ATOMIC to fix it.
Would not it will be more reliable to just rework
t7xx_cldma_clear_rxq() to avoid calling t7xx_cldma_alloc_and_map_skb()
under the spin lock instead of doing each allocation with GFP_ATOMIC?
E.g. t7xx_cldma_gpd_rx_from_q() calls t7xx_cldma_alloc_and_map_skb()
avoiding any lock holding.
--
Sergey
^ permalink raw reply
* Re: [PATCH v2] net: virtio_net: support interrupt coalescing
From: Michael S. Tsirkin @ 2022-05-16 20:30 UTC (permalink / raw)
To: Alvaro Karsz
Cc: netdev, rabeeh, Jason Wang, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
In-Reply-To: <20220515064330.75604-1-alvaro.karsz@solid-run.com>
On Sun, May 15, 2022 at 09:43:30AM +0300, Alvaro Karsz wrote:
> Control a Virtio network device interrupt coalescing parameters
> using the control virtqueue.
>
> New VirtIO network feature: VIRTIO_NET_F_INTR_COAL.
>
> A device that supports this fetature can receive
> VIRTIO_NET_CTRL_INTR_COAL control commands.
>
> - VIRTIO_NET_CTRL_INTR_COAL_USECS_SET:
> change the rx-usecs and tx-usecs parameters.
>
> rx-usecs - Time to delay an RX interrupt after packet arrival in
> microseconds.
>
> tx-usecs - Time to delay a TX interrupt after a sending a packet
> in microseconds.
>
> - VIRTIO_NET_CTRL_INTR_COAL_FRAMES_SET:
> change the rx-max-frames and tx-max-frames parameters.
>
> rx-max-frames: Number of packets to delay an RX interrupt after
> packet arrival.
>
> tx-max-frames: Number of packets to delay a TX interrupt after
> sending a packet.
>
> Signed-off-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
> ---
> v2:
> - usage of __virtio vairables.
> - send commands to device only if the values have changed.
> ---
> drivers/net/virtio_net.c | 116 ++++++++++++++++++++++++++++----
> include/uapi/linux/virtio_net.h | 34 +++++++++-
This needs review of the virtio TC, and a vote. Let's wait for this
before merging please. Until then pls post RFC patches.
For patch itself, I'm going on vacation until May 25, will review
afterwards.
> 2 files changed, 136 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index cbba9d2e8f3..e40f453edb9 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -261,6 +261,12 @@ struct virtnet_info {
> u8 duplex;
> u32 speed;
>
> + /* Interrupt coalescing settings */
> + u32 tx_usecs;
> + u32 rx_usecs;
> + u32 tx_frames_max;
> + u32 rx_frames_max;
> +
> unsigned long guest_offloads;
> unsigned long guest_offloads_capable;
>
> @@ -2594,19 +2600,83 @@ static int virtnet_set_coalesce(struct net_device *dev,
> {
> struct virtnet_info *vi = netdev_priv(dev);
> int i, napi_weight;
> + struct scatterlist sgs_usecs, sgs_frames;
> + struct virtio_net_ctrl_coal_frames c_frames;
> + struct virtio_net_ctrl_coal_usec c_usecs;
> + bool update_napi,
> + intr_coal = virtio_has_feature(vi->vdev, VIRTIO_NET_F_INTR_COAL);
> +
> + /* rx_coalesce_usecs/tx_coalesce_usecs are supported only
> + * if VIRTIO_NET_F_INTR_COAL feature is set.
> + */
> + if (!intr_coal && (ec->rx_coalesce_usecs || ec->tx_coalesce_usecs))
> + return -EOPNOTSUPP;
>
> - if (ec->tx_max_coalesced_frames > 1 ||
> - ec->rx_max_coalesced_frames != 1)
> + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_INTR_COAL)) {
> + /* Send usec command only if value has changed */
> + if (ec->tx_coalesce_usecs != vi->tx_usecs ||
> + ec->rx_coalesce_usecs != vi->rx_usecs) {
> + c_usecs.tx_usecs = cpu_to_virtio32(vi->vdev, ec->tx_coalesce_usecs);
> + c_usecs.rx_usecs = cpu_to_virtio32(vi->vdev, ec->rx_coalesce_usecs);
> + sg_init_one(&sgs_usecs, &c_usecs, sizeof(c_usecs));
> +
> + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_INTR_COAL,
> + VIRTIO_NET_CTRL_INTR_COAL_USECS_SET,
> + &sgs_usecs))
> + return -EINVAL;
> +
> + /* Save parameters */
> + vi->tx_usecs = ec->tx_coalesce_usecs;
> + vi->rx_usecs = ec->rx_coalesce_usecs;
> + }
> +
> + /* Send frames command only if value has changed */
> + if (ec->tx_max_coalesced_frames != vi->tx_frames_max ||
> + ec->rx_max_coalesced_frames != vi->rx_frames_max) {
> + c_frames.tx_frames_max = cpu_to_virtio32(vi->vdev,
> + ec->tx_max_coalesced_frames);
> + c_frames.rx_frames_max = cpu_to_virtio32(vi->vdev,
> + ec->rx_max_coalesced_frames);
> + sg_init_one(&sgs_frames, &c_frames, sizeof(c_frames));
> +
> + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_INTR_COAL,
> + VIRTIO_NET_CTRL_INTR_COAL_FRAMES_SET,
> + &sgs_frames))
> + return -EINVAL;
> +
> + /* Save parameters */
> + vi->tx_frames_max = ec->tx_max_coalesced_frames;
> + vi->rx_frames_max = ec->rx_max_coalesced_frames;
> + }
> + }
> +
> + /* Should we update NAPI? */
> + update_napi = ec->tx_max_coalesced_frames <= 1 &&
> + ec->rx_max_coalesced_frames == 1;
> +
> + /* If interrupt coalesing feature is not set, and we can't update NAPI, return an error */
> + if (!intr_coal && !update_napi)
> return -EINVAL;
>
> - napi_weight = ec->tx_max_coalesced_frames ? NAPI_POLL_WEIGHT : 0;
> - if (napi_weight ^ vi->sq[0].napi.weight) {
> - if (dev->flags & IFF_UP)
> - return -EBUSY;
> - for (i = 0; i < vi->max_queue_pairs; i++)
> - vi->sq[i].napi.weight = napi_weight;
> + if (update_napi) {
> + napi_weight = ec->tx_max_coalesced_frames ? NAPI_POLL_WEIGHT : 0;
> + if (napi_weight ^ vi->sq[0].napi.weight) {
> + if (dev->flags & IFF_UP) {
> + /* If Interrupt coalescing feature is not set, return an error.
> + * Otherwise exit without changing the NAPI paremeters
> + */
> + if (!intr_coal)
> + return -EBUSY;
> +
> + goto exit;
> + }
> +
> + for (i = 0; i < vi->max_queue_pairs; i++)
> + vi->sq[i].napi.weight = napi_weight;
> + }
> }
>
> +exit:
> return 0;
> }
>
> @@ -2616,14 +2686,25 @@ static int virtnet_get_coalesce(struct net_device *dev,
> struct netlink_ext_ack *extack)
> {
> struct ethtool_coalesce ec_default = {
> - .cmd = ETHTOOL_GCOALESCE,
> - .rx_max_coalesced_frames = 1,
> + .cmd = ETHTOOL_GCOALESCE
> };
> +
> struct virtnet_info *vi = netdev_priv(dev);
> + bool intr_coal = virtio_has_feature(vi->vdev, VIRTIO_NET_F_INTR_COAL);
> +
> + /* Add Interrupt coalescing settings */
> + if (intr_coal) {
> + ec_default.rx_coalesce_usecs = vi->rx_usecs;
> + ec_default.tx_coalesce_usecs = vi->tx_usecs;
> + ec_default.tx_max_coalesced_frames = vi->tx_frames_max;
> + ec_default.rx_max_coalesced_frames = vi->rx_frames_max;
> + } else {
> + ec_default.rx_max_coalesced_frames = 1;
> + }
>
> memcpy(ec, &ec_default, sizeof(ec_default));
>
> - if (vi->sq[0].napi.weight)
> + if (!intr_coal && vi->sq[0].napi.weight)
> ec->tx_max_coalesced_frames = 1;
>
> return 0;
> @@ -2743,7 +2824,7 @@ static int virtnet_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info)
> }
>
> static const struct ethtool_ops virtnet_ethtool_ops = {
> - .supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES,
> + .supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES | ETHTOOL_COALESCE_USECS,
> .get_drvinfo = virtnet_get_drvinfo,
> .get_link = ethtool_op_get_link,
> .get_ringparam = virtnet_get_ringparam,
> @@ -3423,6 +3504,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS,
> "VIRTIO_NET_F_CTRL_VQ") ||
> VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> + "VIRTIO_NET_F_CTRL_VQ") ||
> + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_INTR_COAL,
> "VIRTIO_NET_F_CTRL_VQ"))) {
> return false;
> }
> @@ -3558,6 +3641,13 @@ static int virtnet_probe(struct virtio_device *vdev)
> if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
> vi->mergeable_rx_bufs = true;
>
> + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_INTR_COAL)) {
> + vi->rx_usecs = 0;
> + vi->tx_usecs = 0;
> + vi->tx_frames_max = 0;
> + vi->rx_frames_max = 0;
> + }
> +
> if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
> vi->has_rss_hash_report = true;
>
> @@ -3786,7 +3876,7 @@ static struct virtio_device_id id_table[] = {
> VIRTIO_NET_F_CTRL_MAC_ADDR, \
> VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> - VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT
> + VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT, VIRTIO_NET_F_INTR_COAL
>
> static unsigned int features[] = {
> VIRTNET_FEATURES,
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index 3f55a4215f1..05da9eb6ad9 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -56,7 +56,7 @@
> #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow
> * Steering */
> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
> -
> +#define VIRTIO_NET_F_INTR_COAL 24 /* Guest can handle Interrupt coalescing */
> #define VIRTIO_NET_F_HASH_REPORT 57 /* Supports hash report */
> #define VIRTIO_NET_F_RSS 60 /* Supports RSS RX steering */
> #define VIRTIO_NET_F_RSC_EXT 61 /* extended coalescing info */
> @@ -355,4 +355,36 @@ struct virtio_net_hash_config {
> #define VIRTIO_NET_CTRL_GUEST_OFFLOADS 5
> #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET 0
>
> +/*
> + * Control interrupt coalescing.
> + *
> + * Request the device to change the interrupt coalescing parameters.
> + *
> + * Available with the VIRTIO_NET_F_INTR_COAL feature bit.
> + */
> +#define VIRTIO_NET_CTRL_INTR_COAL 6
> +/*
> + * Set the rx-usecs/tx-usecs patameters.
> + * rx-usecs - Number of microseconds to delay an RX interrupt after packet arrival.
> + * tx-usecs - Number of microseconds to delay a TX interrupt after a sending a packet.
> + */
> +struct virtio_net_ctrl_coal_usec {
> + __virtio32 tx_usecs;
> + __virtio32 rx_usecs;
> +};
> +
> +#define VIRTIO_NET_CTRL_INTR_COAL_USECS_SET 0
> +
> +/*
> + * Set the rx-max-frames/tx-max-frames patameters.
> + * rx-max-frames - Number of packets to delay an RX interrupt after packet arrival.
> + * tx-max-frames - Number of packets to delay a TX interrupt after sending a packet.
> + */
> +struct virtio_net_ctrl_coal_frames {
> + __virtio32 tx_frames_max;
> + __virtio32 rx_frames_max;
> +};
> +
> +#define VIRTIO_NET_CTRL_INTR_COAL_FRAMES_SET 1
> +
> #endif /* _UAPI_LINUX_VIRTIO_NET_H */
> --
> 2.32.0
^ permalink raw reply
* [PATCH net-next 6/9] can: slcan: slc_xmit(): use can_dropped_invalid_skb() instead of manual check
From: Marc Kleine-Budde @ 2022-05-16 20:26 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel, Vincent Mailhol,
Marc Kleine-Budde
In-Reply-To: <20220516202625.1129281-1-mkl@pengutronix.de>
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
slcan does a manual check in slc_xmit() to verify if the skb is valid.
This check is incomplete, use instead can_dropped_invalid_skb().
Link: https://lore.kernel.org/all/20220514141650.1109542-2-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/slcan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index ec294d0c5722..64a3aee8a7da 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -359,8 +359,8 @@ static netdev_tx_t slc_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct slcan *sl = netdev_priv(dev);
- if (skb->len != CAN_MTU)
- goto out;
+ if (can_dropped_invalid_skb(dev, skb))
+ return NETDEV_TX_OK;
spin_lock(&sl->lock);
if (!netif_running(dev)) {
--
2.35.1
^ permalink raw reply related
* [PATCH net-next 1/9] can: raw: raw_sendmsg(): remove not needed setting of skb->sk
From: Marc Kleine-Budde @ 2022-05-16 20:26 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde,
Oliver Hartkopp
In-Reply-To: <20220516202625.1129281-1-mkl@pengutronix.de>
The skb in raw_sendmsg() is allocated with sock_alloc_send_skb(),
which subsequently calls sock_alloc_send_pskb() -> skb_set_owner_w(),
which assigns "skb->sk = sk".
This patch removes the not needed setting of skb->sk.
Link: https://lore.kernel.org/all/20220502091946.1916211-2-mkl@pengutronix.de
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/raw.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/can/raw.c b/net/can/raw.c
index b7dbb57557f3..1a68efae43c2 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -820,7 +820,6 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
skb_setup_tx_timestamp(skb, sk->sk_tsflags);
skb->dev = dev;
- skb->sk = sk;
skb->priority = sk->sk_priority;
err = can_send(skb, ro->loopback);
base-commit: d887ae3247e022183f244cb325dca1dfbd0a9ed0
--
2.35.1
^ permalink raw reply related
* [PATCH net-next 9/9] docs: ctucanfd: Use 'kernel-figure' directive instead of 'figure'
From: Marc Kleine-Budde @ 2022-05-16 20:26 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Akira Yokosawa, Pavel Pisa,
Martin Jerabek, Ondrej Ille, Marc Kleine-Budde
In-Reply-To: <20220516202625.1129281-1-mkl@pengutronix.de>
From: Akira Yokosawa <akiyks@gmail.com>
Two issues were observed in the ReST doc added by commit c3a0addefbde
("docs: ctucanfd: CTU CAN FD open-source IP core documentation.")
with Sphinx versions 2.4.4 and 4.5.0.
The plain "figure" directive broke "make pdfdocs" due to a missing
PDF figure. For conversion of SVG -> PDF to work, the "kernel-figure"
directive, which is an extension for kernel documentation, should
be used instead.
The directive of "code:: raw" causes a warning from both
"make htmldocs" and "make pdfdocs", which reads:
[...]/can/ctu/ctucanfd-driver.rst:75: WARNING: Pygments lexer name
'raw' is not known
A plain literal-block marker should suffice where no syntax
highlighting is intended.
Fix the issues by using suitable directive and marker.
Fixes: c3a0addefbde ("docs: ctucanfd: CTU CAN FD open-source IP core documentation.")
Link: https://lore.kernel.org/all/5986752a-1c2a-5d64-f91d-58b1e6decd17@gmail.com
Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
Acked-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Cc: Martin Jerabek <martin.jerabek01@gmail.com>
Cc: Ondrej Ille <ondrej.ille@gmail.com>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
.../networking/device_drivers/can/ctu/ctucanfd-driver.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst b/Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst
index 2fde5551e756..40c92ea272af 100644
--- a/Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst
+++ b/Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst
@@ -72,7 +72,7 @@ it is reachable (on which bus it resides) and its configuration –
registers address, interrupts and so on. An example of such a device
tree is given in .
-.. code:: raw
+::
/ {
/* ... */
@@ -451,7 +451,7 @@ the FIFO is maintained, together with priority rotation, is depicted in
|
-.. figure:: fsm_txt_buffer_user.svg
+.. kernel-figure:: fsm_txt_buffer_user.svg
TX Buffer states with possible transitions
--
2.35.1
^ permalink raw reply related
* [PATCH net-next 4/9] can: isotp: isotp_bind(): return -EINVAL on incorrect CAN ID formatting
From: Marc Kleine-Budde @ 2022-05-16 20:26 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp, Derek Will,
Marc Kleine-Budde
In-Reply-To: <20220516202625.1129281-1-mkl@pengutronix.de>
From: Oliver Hartkopp <socketcan@hartkopp.net>
Commit 3ea566422cbd ("can: isotp: sanitize CAN ID checks in
isotp_bind()") checks the given CAN ID address information by
sanitizing the input values.
This check (silently) removes obsolete bits by masking the given CAN
IDs.
Derek Will suggested to give a feedback to the application programmer
when the 'sanitizing' was actually needed which means the programmer
provided CAN ID content in a wrong format (e.g. SFF CAN IDs with a CAN
ID > 0x7FF).
Link: https://lore.kernel.org/all/20220515181633.76671-1-socketcan@hartkopp.net
Suggested-by: Derek Will <derekrobertwill@gmail.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/isotp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/can/isotp.c b/net/can/isotp.c
index 2caeeae8ec16..4a4007f10970 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1232,6 +1232,11 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
else
rx_id &= CAN_SFF_MASK;
+ /* give feedback on wrong CAN-ID values */
+ if (tx_id != addr->can_addr.tp.tx_id ||
+ rx_id != addr->can_addr.tp.rx_id)
+ return -EINVAL;
+
if (!addr->can_ifindex)
return -ENODEV;
--
2.35.1
^ permalink raw reply related
* [PATCH net-next 3/9] can: isotp: add support for transmission without flow control
From: Marc Kleine-Budde @ 2022-05-16 20:26 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp,
Marc Kleine-Budde
In-Reply-To: <20220516202625.1129281-1-mkl@pengutronix.de>
From: Oliver Hartkopp <socketcan@hartkopp.net>
Usually the ISO 15765-2 protocol is a point-to-point protocol to transfer
segmented PDUs to a dedicated receiver. This receiver sends a flow control
message to specify protocol options and timings (e.g. block size / STmin).
The so called functional addressing communication allows a 1:N
communication but is limited to a single frame length.
This new CAN_ISOTP_CF_BROADCAST allows an unconfirmed 1:N communication
with PDU length that would not fit into a single frame. This feature is
not covered by the ISO 15765-2 standard.
Link: https://lore.kernel.org/all/20220507115558.19065-1-socketcan@hartkopp.net
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
include/uapi/linux/can/isotp.h | 25 +++++----
net/can/isotp.c | 100 ++++++++++++++++++++++++++-------
2 files changed, 92 insertions(+), 33 deletions(-)
diff --git a/include/uapi/linux/can/isotp.h b/include/uapi/linux/can/isotp.h
index 590f8aea2b6d..439c982f7e81 100644
--- a/include/uapi/linux/can/isotp.h
+++ b/include/uapi/linux/can/isotp.h
@@ -124,18 +124,19 @@ struct can_isotp_ll_options {
/* flags for isotp behaviour */
-#define CAN_ISOTP_LISTEN_MODE 0x001 /* listen only (do not send FC) */
-#define CAN_ISOTP_EXTEND_ADDR 0x002 /* enable extended addressing */
-#define CAN_ISOTP_TX_PADDING 0x004 /* enable CAN frame padding tx path */
-#define CAN_ISOTP_RX_PADDING 0x008 /* enable CAN frame padding rx path */
-#define CAN_ISOTP_CHK_PAD_LEN 0x010 /* check received CAN frame padding */
-#define CAN_ISOTP_CHK_PAD_DATA 0x020 /* check received CAN frame padding */
-#define CAN_ISOTP_HALF_DUPLEX 0x040 /* half duplex error state handling */
-#define CAN_ISOTP_FORCE_TXSTMIN 0x080 /* ignore stmin from received FC */
-#define CAN_ISOTP_FORCE_RXSTMIN 0x100 /* ignore CFs depending on rx stmin */
-#define CAN_ISOTP_RX_EXT_ADDR 0x200 /* different rx extended addressing */
-#define CAN_ISOTP_WAIT_TX_DONE 0x400 /* wait for tx completion */
-#define CAN_ISOTP_SF_BROADCAST 0x800 /* 1-to-N functional addressing */
+#define CAN_ISOTP_LISTEN_MODE 0x0001 /* listen only (do not send FC) */
+#define CAN_ISOTP_EXTEND_ADDR 0x0002 /* enable extended addressing */
+#define CAN_ISOTP_TX_PADDING 0x0004 /* enable CAN frame padding tx path */
+#define CAN_ISOTP_RX_PADDING 0x0008 /* enable CAN frame padding rx path */
+#define CAN_ISOTP_CHK_PAD_LEN 0x0010 /* check received CAN frame padding */
+#define CAN_ISOTP_CHK_PAD_DATA 0x0020 /* check received CAN frame padding */
+#define CAN_ISOTP_HALF_DUPLEX 0x0040 /* half duplex error state handling */
+#define CAN_ISOTP_FORCE_TXSTMIN 0x0080 /* ignore stmin from received FC */
+#define CAN_ISOTP_FORCE_RXSTMIN 0x0100 /* ignore CFs depending on rx stmin */
+#define CAN_ISOTP_RX_EXT_ADDR 0x0200 /* different rx extended addressing */
+#define CAN_ISOTP_WAIT_TX_DONE 0x0400 /* wait for tx completion */
+#define CAN_ISOTP_SF_BROADCAST 0x0800 /* 1-to-N functional addressing */
+#define CAN_ISOTP_CF_BROADCAST 0x1000 /* 1-to-N transmission w/o FC */
/* protocol machine default values */
diff --git a/net/can/isotp.c b/net/can/isotp.c
index 35a1ae61744c..2caeeae8ec16 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -104,6 +104,7 @@ MODULE_ALIAS("can-proto-6");
#define FC_CONTENT_SZ 3 /* flow control content size in byte (FS/BS/STmin) */
#define ISOTP_CHECK_PADDING (CAN_ISOTP_CHK_PAD_LEN | CAN_ISOTP_CHK_PAD_DATA)
+#define ISOTP_ALL_BC_FLAGS (CAN_ISOTP_SF_BROADCAST | CAN_ISOTP_CF_BROADCAST)
/* Flow Status given in FC frame */
#define ISOTP_FC_CTS 0 /* clear to send */
@@ -159,6 +160,23 @@ static inline struct isotp_sock *isotp_sk(const struct sock *sk)
return (struct isotp_sock *)sk;
}
+static u32 isotp_bc_flags(struct isotp_sock *so)
+{
+ return so->opt.flags & ISOTP_ALL_BC_FLAGS;
+}
+
+static bool isotp_register_rxid(struct isotp_sock *so)
+{
+ /* no broadcast modes => register rx_id for FC frame reception */
+ return (isotp_bc_flags(so) == 0);
+}
+
+static bool isotp_register_txecho(struct isotp_sock *so)
+{
+ /* all modes but SF_BROADCAST register for tx echo skbs */
+ return (isotp_bc_flags(so) != CAN_ISOTP_SF_BROADCAST);
+}
+
static enum hrtimer_restart isotp_rx_timer_handler(struct hrtimer *hrtimer)
{
struct isotp_sock *so = container_of(hrtimer, struct isotp_sock,
@@ -803,7 +821,6 @@ static void isotp_create_fframe(struct canfd_frame *cf, struct isotp_sock *so,
cf->data[i] = so->tx.buf[so->tx.idx++];
so->tx.sn = 1;
- so->tx.state = ISOTP_WAIT_FIRST_FC;
}
static void isotp_rcv_echo(struct sk_buff *skb, void *data)
@@ -936,7 +953,7 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
off = (so->tx.ll_dl > CAN_MAX_DLEN) ? 1 : 0;
/* does the given data fit into a single frame for SF_BROADCAST? */
- if ((so->opt.flags & CAN_ISOTP_SF_BROADCAST) &&
+ if ((isotp_bc_flags(so) == CAN_ISOTP_SF_BROADCAST) &&
(size > so->tx.ll_dl - SF_PCI_SZ4 - ae - off)) {
err = -EINVAL;
goto err_out_drop;
@@ -1000,12 +1017,41 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
/* don't enable wait queue for a single frame transmission */
wait_tx_done = 0;
} else {
- /* send first frame and wait for FC */
+ /* send first frame */
isotp_create_fframe(cf, so, ae);
- /* start timeout for FC */
- hrtimer_sec = 1;
+ if (isotp_bc_flags(so) == CAN_ISOTP_CF_BROADCAST) {
+ /* set timer for FC-less operation (STmin = 0) */
+ if (so->opt.flags & CAN_ISOTP_FORCE_TXSTMIN)
+ so->tx_gap = ktime_set(0, so->force_tx_stmin);
+ else
+ so->tx_gap = ktime_set(0, so->frame_txtime);
+
+ /* disable wait for FCs due to activated block size */
+ so->txfc.bs = 0;
+
+ /* cfecho should have been zero'ed by init */
+ if (so->cfecho)
+ pr_notice_once("can-isotp: no fc cfecho %08X\n",
+ so->cfecho);
+
+ /* set consecutive frame echo tag */
+ so->cfecho = *(u32 *)cf->data;
+
+ /* switch directly to ISOTP_SENDING state */
+ so->tx.state = ISOTP_SENDING;
+
+ /* start timeout for unlikely lost echo skb */
+ hrtimer_sec = 2;
+ } else {
+ /* standard flow control check */
+ so->tx.state = ISOTP_WAIT_FIRST_FC;
+
+ /* start timeout for FC */
+ hrtimer_sec = 1;
+ }
+
hrtimer_start(&so->txtimer, ktime_set(hrtimer_sec, 0),
HRTIMER_MODE_REL_SOFT);
}
@@ -1025,6 +1071,9 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
if (hrtimer_sec)
hrtimer_cancel(&so->txtimer);
+ /* reset consecutive frame echo tag */
+ so->cfecho = 0;
+
goto err_out_drop;
}
@@ -1120,15 +1169,17 @@ static int isotp_release(struct socket *sock)
lock_sock(sk);
/* remove current filters & unregister */
- if (so->bound && (!(so->opt.flags & CAN_ISOTP_SF_BROADCAST))) {
+ if (so->bound && isotp_register_txecho(so)) {
if (so->ifindex) {
struct net_device *dev;
dev = dev_get_by_index(net, so->ifindex);
if (dev) {
- can_rx_unregister(net, dev, so->rxid,
- SINGLE_MASK(so->rxid),
- isotp_rcv, sk);
+ if (isotp_register_rxid(so))
+ can_rx_unregister(net, dev, so->rxid,
+ SINGLE_MASK(so->rxid),
+ isotp_rcv, sk);
+
can_rx_unregister(net, dev, so->txid,
SINGLE_MASK(so->txid),
isotp_rcv_echo, sk);
@@ -1164,7 +1215,6 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
canid_t tx_id, rx_id;
int err = 0;
int notify_enetdown = 0;
- int do_rx_reg = 1;
if (len < ISOTP_MIN_NAMELEN)
return -EINVAL;
@@ -1192,12 +1242,8 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
goto out;
}
- /* do not register frame reception for functional addressing */
- if (so->opt.flags & CAN_ISOTP_SF_BROADCAST)
- do_rx_reg = 0;
-
- /* do not validate rx address for functional addressing */
- if (do_rx_reg && rx_id == tx_id) {
+ /* ensure different CAN IDs when the rx_id is to be registered */
+ if (isotp_register_rxid(so) && rx_id == tx_id) {
err = -EADDRNOTAVAIL;
goto out;
}
@@ -1222,10 +1268,11 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
ifindex = dev->ifindex;
- if (do_rx_reg) {
+ if (isotp_register_rxid(so))
can_rx_register(net, dev, rx_id, SINGLE_MASK(rx_id),
isotp_rcv, sk, "isotp", sk);
+ if (isotp_register_txecho(so)) {
/* no consecutive frame echo skb in flight */
so->cfecho = 0;
@@ -1294,6 +1341,15 @@ static int isotp_setsockopt_locked(struct socket *sock, int level, int optname,
if (!(so->opt.flags & CAN_ISOTP_RX_EXT_ADDR))
so->opt.rx_ext_address = so->opt.ext_address;
+ /* these broadcast flags are not allowed together */
+ if (isotp_bc_flags(so) == ISOTP_ALL_BC_FLAGS) {
+ /* CAN_ISOTP_SF_BROADCAST is prioritized */
+ so->opt.flags &= ~CAN_ISOTP_CF_BROADCAST;
+
+ /* give user feedback on wrong config attempt */
+ ret = -EINVAL;
+ }
+
/* check for frame_txtime changes (0 => no changes) */
if (so->opt.frame_txtime) {
if (so->opt.frame_txtime == CAN_ISOTP_FRAME_TXTIME_ZERO)
@@ -1444,10 +1500,12 @@ static void isotp_notify(struct isotp_sock *so, unsigned long msg,
case NETDEV_UNREGISTER:
lock_sock(sk);
/* remove current filters & unregister */
- if (so->bound && (!(so->opt.flags & CAN_ISOTP_SF_BROADCAST))) {
- can_rx_unregister(dev_net(dev), dev, so->rxid,
- SINGLE_MASK(so->rxid),
- isotp_rcv, sk);
+ if (so->bound && isotp_register_txecho(so)) {
+ if (isotp_register_rxid(so))
+ can_rx_unregister(dev_net(dev), dev, so->rxid,
+ SINGLE_MASK(so->rxid),
+ isotp_rcv, sk);
+
can_rx_unregister(dev_net(dev), dev, so->txid,
SINGLE_MASK(so->txid),
isotp_rcv_echo, sk);
--
2.35.1
^ permalink raw reply related
* [PATCH net-next 5/9] can: ctucanfd: Let users select instead of depend on CAN_CTUCANFD
From: Marc Kleine-Budde @ 2022-05-16 20:26 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Geert Uytterhoeven, Pavel Pisa,
Marc Kleine-Budde
In-Reply-To: <20220516202625.1129281-1-mkl@pengutronix.de>
From: Geert Uytterhoeven <geert+renesas@glider.be>
The CTU CAN-FD IP core is only useful when used with one of the
corresponding PCI/PCIe or platform (FPGA, SoC) drivers, which depend on
PCI resp. OF.
Hence make the users select the core driver code, instead of letting
then depend on it. Keep the core code config option visible when
compile-testing, to maintain compile-coverage.
Link: https://lore.kernel.org/all/887b7440446b6244a20a503cc6e8dc9258846706.1652104941.git.geert+renesas@glider.be
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/ctucanfd/Kconfig | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/ctucanfd/Kconfig b/drivers/net/can/ctucanfd/Kconfig
index 48963efc7f19..3c383612eb17 100644
--- a/drivers/net/can/ctucanfd/Kconfig
+++ b/drivers/net/can/ctucanfd/Kconfig
@@ -1,5 +1,5 @@
config CAN_CTUCANFD
- tristate "CTU CAN-FD IP core"
+ tristate "CTU CAN-FD IP core" if COMPILE_TEST
help
This driver adds support for the CTU CAN FD open-source IP core.
More documentation and core sources at project page
@@ -13,8 +13,8 @@ config CAN_CTUCANFD
config CAN_CTUCANFD_PCI
tristate "CTU CAN-FD IP core PCI/PCIe driver"
- depends on CAN_CTUCANFD
depends on PCI
+ select CAN_CTUCANFD
help
This driver adds PCI/PCIe support for CTU CAN-FD IP core.
The project providing FPGA design for Intel EP4CGX15 based DB4CGX15
@@ -23,8 +23,8 @@ config CAN_CTUCANFD_PCI
config CAN_CTUCANFD_PLATFORM
tristate "CTU CAN-FD IP core platform (FPGA, SoC) driver"
- depends on CAN_CTUCANFD
depends on OF || COMPILE_TEST
+ select CAN_CTUCANFD
help
The core has been tested together with OpenCores SJA1000
modified to be CAN FD frames tolerant on MicroZed Zynq based
--
2.35.1
^ permalink raw reply related
* [PATCH net-next 0/9] pull-request: can-next 2022-05-16
From: Marc Kleine-Budde @ 2022-05-16 20:26 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel
Hello Jakub, hello David,
this is a pull request of 9 patches for net-next/master.
the first 2 patches are by me and target the CAN raw protocol. The 1st
removes an unneeded assignment, the other one adds support for
SO_TXTIME/SCM_TXTIME.
Oliver Hartkopp contributes 2 patches for the ISOTP protocol. The 1st
adds support for transmission without flow control, the other let's
bind() return an error on incorrect CAN ID formatting.
Geert Uytterhoeven contributes a patch to clean up ctucanfd's Kconfig
file.
Vincent Mailhol's patch for the slcan driver uses the proper function
to check for invalid CAN frames in the xmit callback.
The next patch is by Geert Uytterhoeven and makes the interrupt-names
of the renesas,rcar-canfd dt bindings mandatory.
A patch by my update the ctucanfd dt bindings to include the common
CAN controller bindings.
The last patch is by Akira Yokosawa and fixes a breakage the
ctucanfd's documentation.
regards,
Marc
---
The following changes since commit d887ae3247e022183f244cb325dca1dfbd0a9ed0:
octeontx2-pf: Remove unnecessary synchronize_irq() before free_irq() (2022-05-16 11:47:22 +0100)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git tags/linux-can-next-for-5.19-20220516
for you to fetch changes up to ba3e2eaef1ae5019775989aeec3be8e9df83baa5:
docs: ctucanfd: Use 'kernel-figure' directive instead of 'figure' (2022-05-16 22:11:11 +0200)
----------------------------------------------------------------
linux-can-next-for-5.19-20220516
----------------------------------------------------------------
Akira Yokosawa (1):
docs: ctucanfd: Use 'kernel-figure' directive instead of 'figure'
Geert Uytterhoeven (2):
can: ctucanfd: Let users select instead of depend on CAN_CTUCANFD
dt-bindings: can: renesas,rcar-canfd: Make interrupt-names required
Marc Kleine-Budde (3):
can: raw: raw_sendmsg(): remove not needed setting of skb->sk
can: raw: add support for SO_TXTIME/SCM_TXTIME
dt-bindings: can: ctucanfd: include common CAN controller bindings
Oliver Hartkopp (2):
can: isotp: add support for transmission without flow control
can: isotp: isotp_bind(): return -EINVAL on incorrect CAN ID formatting
Vincent Mailhol (1):
can: slcan: slc_xmit(): use can_dropped_invalid_skb() instead of manual check
.../devicetree/bindings/net/can/ctu,ctucanfd.yaml | 3 +
.../bindings/net/can/renesas,rcar-canfd.yaml | 3 +-
.../device_drivers/can/ctu/ctucanfd-driver.rst | 4 +-
drivers/net/can/ctucanfd/Kconfig | 6 +-
drivers/net/can/slcan.c | 4 +-
include/uapi/linux/can/isotp.h | 25 ++---
net/can/isotp.c | 105 ++++++++++++++++-----
net/can/raw.c | 12 ++-
8 files changed, 119 insertions(+), 43 deletions(-)
^ permalink raw reply
* [PATCH net-next 2/9] can: raw: add support for SO_TXTIME/SCM_TXTIME
From: Marc Kleine-Budde @ 2022-05-16 20:26 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde,
Oliver Hartkopp
In-Reply-To: <20220516202625.1129281-1-mkl@pengutronix.de>
This patch calls into sock_cmsg_send() to parse the user supplied
control information into a struct sockcm_cookie. Then assign the
requested transmit time to the skb.
This makes it possible to use the Earliest TXTIME First (ETF) packet
scheduler with the CAN_RAW protocol. The user can send a CAN_RAW frame
with a TXTIME and the kernel (with the ETF scheduler) will take care
of sending it to the network interface.
Link: https://lore.kernel.org/all/20220502091946.1916211-3-mkl@pengutronix.de
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/raw.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/net/can/raw.c b/net/can/raw.c
index 1a68efae43c2..d1bd9cc51ebe 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -772,6 +772,7 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
{
struct sock *sk = sock->sk;
struct raw_sock *ro = raw_sk(sk);
+ struct sockcm_cookie sockc;
struct sk_buff *skb;
struct net_device *dev;
int ifindex;
@@ -817,10 +818,18 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
if (err < 0)
goto free_skb;
- skb_setup_tx_timestamp(skb, sk->sk_tsflags);
+ sockcm_init(&sockc, sk);
+ if (msg->msg_controllen) {
+ err = sock_cmsg_send(sk, msg, &sockc);
+ if (unlikely(err))
+ goto free_skb;
+ }
skb->dev = dev;
skb->priority = sk->sk_priority;
+ skb->tstamp = sockc.transmit_time;
+
+ skb_setup_tx_timestamp(skb, sockc.tsflags);
err = can_send(skb, ro->loopback);
--
2.35.1
^ permalink raw reply related
* [PATCH net-next 8/9] dt-bindings: can: ctucanfd: include common CAN controller bindings
From: Marc Kleine-Budde @ 2022-05-16 20:26 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde, Ondrej Ille,
Pavel Pisa, Rob Herring
In-Reply-To: <20220516202625.1129281-1-mkl@pengutronix.de>
Since commit
| 1f9234401ce0 ("dt-bindings: can: add can-controller.yaml")
there is a common CAN controller binding. Add this to the ctucanfd
binding.
Cc: Ondrej Ille <ondrej.ille@gmail.com>
Acked-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml b/Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml
index fb34d971dcb3..4635cb96fc64 100644
--- a/Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml
+++ b/Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml
@@ -25,6 +25,9 @@ maintainers:
- Ondrej Ille <ondrej.ille@gmail.com>
- Martin Jerabek <martin.jerabek01@gmail.com>
+allOf:
+ - $ref: can-controller.yaml#
+
properties:
compatible:
oneOf:
--
2.35.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