* Re: [PATCH 2/3] samples: pktgen: add helper functions for IP(v4/v6) CIDR parsing
From: Jesper Dangaard Brouer @ 2019-08-30 13:27 UTC (permalink / raw)
To: Daniel T. Lee; +Cc: David S . Miller, netdev, brouer
In-Reply-To: <20190828204243.16666-2-danieltimlee@gmail.com>
On Thu, 29 Aug 2019 05:42:42 +0900
"Daniel T. Lee" <danieltimlee@gmail.com> wrote:
> This commit adds CIDR parsing and IP validate helper function to parse
> single IP or range of IP with CIDR. (e.g. 198.18.0.0/15)
>
> Helpers will be used in prior to set target address in samples/pktgen.
>
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> ---
> samples/pktgen/functions.sh | 134 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 134 insertions(+)
>
> diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh
> index 4af4046d71be..eb1c52e25018 100644
> --- a/samples/pktgen/functions.sh
> +++ b/samples/pktgen/functions.sh
> @@ -163,6 +163,140 @@ function get_node_cpus()
> echo $node_cpu_list
> }
>
> +# Extend shrunken IPv6 address.
> +# fe80::42:bcff:fe84:e10a => fe80:0:0:0:42:bcff:fe84:e10a
> +function extend_addr6()
> +{
> + local addr=$1
> + local sep=:
> + local sep2=::
> + local sep_cnt=$(tr -cd $sep <<< $1 | wc -c)
> + local shrink
> +
> + # separator count : should be between 2, 7.
> + if [[ $sep_cnt -lt 2 || $sep_cnt -gt 7 ]]; then
> + err 5 "Invalid IP6 address sep: $1"
> + fi
> +
> + # if shrink '::' occurs multiple, it's malformed.
> + shrink=( $(egrep -o "$sep{2,}" <<< $addr) )
> + if [[ ${#shrink[@]} -ne 0 ]]; then
> + if [[ ${#shrink[@]} -gt 1 || ( ${shrink[0]} != $sep2 ) ]]; then
> + err 5 "Invalid IP$IP6 address shr: $1"
> + fi
> + fi
> +
> + # add 0 at begin & end, and extend addr by adding :0
> + [[ ${addr:0:1} == $sep ]] && addr=0${addr}
> + [[ ${addr: -1} == $sep ]] && addr=${addr}0
> + echo "${addr/$sep2/$(printf ':0%.s' $(seq $[8-sep_cnt])):}"
> +}
> +
> +
> +# Given a single IP(v4/v6) address, whether it is valid.
> +function validate_addr()
> +{
> + # check function is called with (funcname)6
> + [[ ${FUNCNAME[1]: -1} == 6 ]] && local IP6=6
> + local len=$[ IP6 ? 8 : 4 ]
> + local max=$[ 2**(len*2)-1 ]
> + local addr
> + local sep
> +
> + # set separator for each IP(v4/v6)
> + [[ $IP6 ]] && sep=: || sep=.
> + IFS=$sep read -a addr <<< $1
> +
> + # array length
> + if [[ ${#addr[@]} != $len ]]; then
> + err 5 "Invalid IP$IP6 address: $1"
> + fi
> +
> + # check each digit between 0, $max
> + for digit in "${addr[@]}"; do
> + [[ $IP6 ]] && digit=$[ 16#$digit ]
> + if [[ $digit -lt 0 || $digit -gt $max ]]; then
> + err 5 "Invalid IP$IP6 address: $1"
> + fi
> + done
> +
> + return 0
> +}
> +
> +function validate_addr6() { validate_addr $@ ; }
> +
> +# Given a single IP(v4/v6) or CIDR, return minimum and maximum IP addr.
> +function parse_addr()
I must say that I'm impressed by your bash-shell skills, BUT below
function does look too complicated for doing this... I were expecting
that you would use the regular & (AND) operation to do the prefix
masking.
> +{
> + # check function is called with (funcname)6
> + [[ ${FUNCNAME[1]: -1} == 6 ]] && local IP6=6
> + local bitlen=$[ IP6 ? 128 : 32 ]
> +
> + local addr=$1
> + local net
> + local prefix
> + local min_ip
> + local max_ip
> +
> + IFS='/' read net prefix <<< $addr
> + [[ $IP6 ]] && net=$(extend_addr6 $net)
> + validate_addr$IP6 $net
> +
> + if [[ $prefix -gt $bitlen ]]; then
> + err 5 "Invalid prefix: $prefix"
> + elif [[ -z $prefix ]]; then
> + min_ip=$net
> + max_ip=$net
> + else
> + # defining array for converting Decimal 2 Binary
> + # 00000000 00000001 00000010 00000011 00000100 ...
> + local d2b='{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}'
> + [[ $IP6 ]] && d2b+=$d2b
> + eval local D2B=($d2b)
> +
> + local shift=$[ bitlen-prefix ]
> + local ip_bit
> + local ip
> + local sep
> +
> + # set separator for each IP(v4/v6)
> + [[ $IP6 ]] && sep=: || sep=.
> + IFS=$sep read -ra ip <<< $net
> +
> + # build full size bit
> + for digit in "${ip[@]}"; do
> + [[ $IP6 ]] && digit=$[ 16#$digit ]
> + ip_bit+=${D2B[$digit]}
> + done
> +
> + # fill 0 or 1 by $shift
> + base_bit=${ip_bit::$prefix}
> + min_bit="$base_bit$(printf '0%.s' $(seq $shift))"
> + max_bit="$base_bit$(printf '1%.s' $(seq $shift))"
> +
> + bit2addr() {
> + local step=$[ IP6 ? 16 : 8 ]
> + local max=$[ bitlen-step ]
> + local result
> + local fmt
> + [[ $IP6 ]] && fmt='%X' || fmt='%d'
> +
> + for i in $(seq 0 $step $max); do
> + result+=$(printf $fmt $[ 2#${1:$i:$step} ])
> + [[ $i != $max ]] && result+=$sep
> + done
> + echo $result
> + }
> +
> + min_ip=$(bit2addr $min_bit)
> + max_ip=$(bit2addr $max_bit)
> + fi
> +
> + echo $min_ip $max_ip
> +}
> +
> +function parse_addr6() { parse_addr $@ ; }
> +
> # Given a single or range of port(s), return minimum and maximum port number.
> function parse_ports()
> {
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [RESEND PATCH 0/5] Add bluetooth support for Orange Pi 3
From: Maxime Ripard @ 2019-08-30 13:20 UTC (permalink / raw)
To: Marcel Holtmann
Cc: megous, Chen-Yu Tsai, Rob Herring, Johan Hedberg, Mark Rutland,
David S. Miller, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-bluetooth
In-Reply-To: <D02B89FB-F8C0-40AD-A99A-6C1B4FEB72A0@holtmann.org>
On Fri, Aug 30, 2019 at 02:43:48PM +0200, Marcel Holtmann wrote:
> >>> (Resend to add missing lists, sorry for the noise.)
> >>>
> >>> This series implements bluetooth support for Xunlong Orange Pi 3 board.
> >>>
> >>> The board uses AP6256 WiFi/BT 5.0 chip.
> >>>
> >>> Summary of changes:
> >>>
> >>> - add more delay to let initialize the chip
> >>> - let the kernel detect firmware file path
> >>> - add new compatible and update dt-bindings
> >>> - update Orange Pi 3 / H6 DTS
> >>>
> >>> Please take a look.
> >>>
> >>> thank you and regards,
> >>> Ondrej Jirman
> >>>
> >>> Ondrej Jirman (5):
> >>> dt-bindings: net: Add compatible for BCM4345C5 bluetooth device
> >>> bluetooth: bcm: Add support for loading firmware for BCM4345C5
> >>> bluetooth: hci_bcm: Give more time to come out of reset
> >>> arm64: dts: allwinner: h6: Add pin configs for uart1
> >>> arm64: dts: allwinner: orange-pi-3: Enable UART1 / Bluetooth
> >>>
> >>> .../bindings/net/broadcom-bluetooth.txt | 1 +
> >>> .../dts/allwinner/sun50i-h6-orangepi-3.dts | 19 +++++++++++++++++++
> >>> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 ++++++++++
> >>> drivers/bluetooth/btbcm.c | 3 +++
> >>> drivers/bluetooth/hci_bcm.c | 3 ++-
> >>> 5 files changed, 35 insertions(+), 1 deletion(-)
> >>
> >> all 5 patches have been applied to bluetooth-next tree.
> >
> > The DTS patches (last 2) should go through the arm-soc tree, can you
> > drop them?
>
> why is that? We have included DTS changes for Bluetooth devices
> directly all the time. What is different with this hardware?
I guess some maintainers are more relaxed with it than we are then,
but for the why, well, it's the usual reasons, the most immediate one
being that it reduces to a minimum the conflicts between trees.
The other being that it's not really usual to merge patches supposed
to be handled by another maintainer without (at least) his
consent. I'm pretty sure you would have asked the same request if I
would have merged the bluetooth patches through my tree without
notice.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* RE: [PATCH v2 5/6] mdev: Update sysfs documentation
From: Parav Pandit @ 2019-08-30 13:10 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190830144927.7961193e.cohuck@redhat.com>
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Friday, August 30, 2019 6:19 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH v2 5/6] mdev: Update sysfs documentation
>
> On Thu, 29 Aug 2019 06:19:03 -0500
> Parav Pandit <parav@mellanox.com> wrote:
>
> > Updated documentation for optional read only sysfs attribute.
>
> I'd probably merge this into the patch introducing the attribute.
>
Ok. I will spin v3.
> >
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > ---
> > Documentation/driver-api/vfio-mediated-device.rst | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/driver-api/vfio-mediated-device.rst
> > b/Documentation/driver-api/vfio-mediated-device.rst
> > index 25eb7d5b834b..0ab03d3f5629 100644
> > --- a/Documentation/driver-api/vfio-mediated-device.rst
> > +++ b/Documentation/driver-api/vfio-mediated-device.rst
> > @@ -270,6 +270,7 @@ Directories and Files Under the sysfs for Each mdev
> Device
> > |--- remove
> > |--- mdev_type {link to its type}
> > |--- vendor-specific-attributes [optional]
> > + |--- alias [optional]
>
> "optional" implies "not always present" to me, not "might return a read error if
> not available". Don't know if there's a better way to tag this? Or make it really
> optional? :)
May be write it as,
alias [ optional when requested by parent ]
>
> >
> > * remove (write only)
> >
> > @@ -281,6 +282,10 @@ Example::
> >
> > # echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
> >
> > +* alias (read only)
> > +Whenever a parent requested to generate an alias, each mdev is
> > +assigned a unique alias by the mdev core. This file shows the alias of the
> mdev device.
>
> It's not really the parent, but the vendor driver requesting this, right? Also,
At mdev level, it only knows parent->ops structure, whether parent is registered by vendor driver or something else.
> "each mdev" is a bit ambiguous,
It is in context of the parent. Sentence is not starting with "each mdev".
But may be more verbosely written as,
Whenever a parent requested to generate an alias, Each mdev device of such parent is assigned
unique alias by the mdev core. This file shows the alias of the mdev device.
> created via that driver. Lastly, if we stick with the "returns an error if not
> implemented" approach, that should also be mentioned here.
Ok. Will spin v3 to describe it.
>
> > +
> > Mediated device Hot plug
> > ------------------------
> >
^ permalink raw reply
* RE: [PATCH v2 2/6] mdev: Make mdev alias unique among all mdevs
From: Parav Pandit @ 2019-08-30 12:59 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190830144052.11d23ec3.cohuck@redhat.com>
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Friday, August 30, 2019 6:11 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH v2 2/6] mdev: Make mdev alias unique among all mdevs
>
> On Thu, 29 Aug 2019 06:19:00 -0500
> Parav Pandit <parav@mellanox.com> wrote:
>
> > Mdev alias should be unique among all the mdevs, so that when such
> > alias is used by the mdev users to derive other objects, there is no
> > collision in a given system.
> >
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> >
> > ---
> > Changelog:
> > v1->v2:
> > - Moved alias NULL check at beginning
> > v0->v1:
> > - Fixed inclusiong of alias for NULL check
> > - Added ratelimited debug print for sha1 hash collision error
> > ---
> > drivers/vfio/mdev/mdev_core.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/vfio/mdev/mdev_core.c
> > b/drivers/vfio/mdev/mdev_core.c index 3bdff0469607..c9bf2ac362b9
> > 100644
> > --- a/drivers/vfio/mdev/mdev_core.c
> > +++ b/drivers/vfio/mdev/mdev_core.c
> > @@ -388,6 +388,13 @@ int mdev_device_create(struct kobject *kobj, struct
> device *dev,
> > ret = -EEXIST;
> > goto mdev_fail;
> > }
> > + if (alias && tmp->alias && strcmp(alias, tmp->alias) == 0) {
> > + mutex_unlock(&mdev_list_lock);
> > + ret = -EEXIST;
> > + dev_dbg_ratelimited(dev, "Hash collision in alias
> creation for UUID %pUl\n",
> > + uuid);
> > + goto mdev_fail;
> > + }
> > }
> >
> > mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
>
> Any reason not to merge this into the first patch?
No. It surely can be merged. Its easy to start with smaller patches instead of splitting. :-)
Doing uniqueness comparison was easy to split as independent functionality, so did as 2nd patch.
But either way is ok.
^ permalink raw reply
* RE: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
From: Parav Pandit @ 2019-08-30 12:58 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190830143927.163d13a7.cohuck@redhat.com>
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Friday, August 30, 2019 6:09 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
>
> On Fri, 30 Aug 2019 12:33:22 +0000
> Parav Pandit <parav@mellanox.com> wrote:
>
> > > -----Original Message-----
> > > From: Cornelia Huck <cohuck@redhat.com>
> > > Sent: Friday, August 30, 2019 2:47 PM
> > > To: Parav Pandit <parav@mellanox.com>
> > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > Subject: Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
> > >
> > > On Thu, 29 Aug 2019 06:18:59 -0500
> > > Parav Pandit <parav@mellanox.com> wrote:
> > >
> > > > Some vendor drivers want an identifier for an mdev device that is
> > > > shorter than the UUID, due to length restrictions in the consumers
> > > > of that identifier.
> > > >
> > > > Add a callback that allows a vendor driver to request an alias of
> > > > a specified length to be generated for an mdev device. If
> > > > generated, that alias is checked for collisions.
> > > >
> > > > It is an optional attribute.
> > > > mdev alias is generated using sha1 from the mdev name.
> > > >
> > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > >
> > > > ---
> > > > Changelog:
> > > > v1->v2:
> > > > - Kept mdev_device naturally aligned
> > > > - Added error checking for crypt_*() calls
> > > > - Corrected a typo from 'and' to 'an'
> > > > - Changed return type of generate_alias() from int to char*
> > > > v0->v1:
> > > > - Moved alias length check outside of the parent lock
> > > > - Moved alias and digest allocation from kvzalloc to kzalloc
> > > > - &alias[0] changed to alias
> > > > - alias_length check is nested under get_alias_length callback
> > > > check
> > > > - Changed comments to start with an empty line
> > > > - Fixed cleaunup of hash if mdev_bus_register() fails
> > > > - Added comment where alias memory ownership is handed over to
> > > > mdev device
> > > > - Updated commit log to indicate motivation for this feature
> > > > ---
> > > > drivers/vfio/mdev/mdev_core.c | 123
> > > ++++++++++++++++++++++++++++++-
> > > > drivers/vfio/mdev/mdev_private.h | 5 +-
> > > > drivers/vfio/mdev/mdev_sysfs.c | 13 ++--
> > > > include/linux/mdev.h | 4 +
> > > > 4 files changed, 135 insertions(+), 10 deletions(-)
>
> > > ...and detached from the local variable here. Who is freeing it? The
> > > comment states that it is done by the mdev, but I don't see it?
> > >
> > mdev_device_free() frees it.
>
> Ah yes, I overlooked the kfree().
>
> > once its assigned to mdev, mdev is the owner of it.
> >
> > > This detour via the local variable looks weird to me. Can you either
> > > create the alias directly in the mdev (would need to happen later in
> > > the function, but I'm not sure why you generate the alias before
> > > checking for duplicates anyway), or do an explicit copy?
> > Alias duplicate check is done after generating it, because duplicate alias are
> not allowed.
> > The probability of collision is rare.
> > So it is speculatively generated without hold the lock, because there is no
> need to hold the lock.
> > It is compared along with guid while mutex lock is held in single loop.
> > And if it is duplicate, there is no need to allocate mdev.
> >
> > It will be sub optimal to run through the mdev list 2nd time after mdev
> creation and after generating alias for duplicate check.
>
> Ok, but what about copying it? I find this "set local variable to NULL after
> ownership is transferred" pattern a bit unintuitive. Copying it to the mdev (and
> then unconditionally freeing it) looks more obvious to me.
Its not unconditionally freed. Its freed in the error unwinding path.
I think its ok along with the comment that describes this error path area.
^ permalink raw reply
* Re: [PATCH 1/3] samples: pktgen: make variable consistent with option
From: Jesper Dangaard Brouer @ 2019-08-30 12:57 UTC (permalink / raw)
To: Daniel T. Lee; +Cc: David S . Miller, netdev, brouer
In-Reply-To: <20190828204243.16666-1-danieltimlee@gmail.com>
On Thu, 29 Aug 2019 05:42:41 +0900
"Daniel T. Lee" <danieltimlee@gmail.com> wrote:
> This commit changes variable names that can cause confusion.
>
> For example, variable DST_MIN is quite confusing since the
> keyword 'udp_dst_min' and keyword 'dst_min' is used with pg_ctrl.
>
> On the following commit, 'dst_min' will be used to set destination IP,
> and the existing variable name DST_MIN should be changed.
>
> Variable names are matched to the exact keyword used with pg_ctrl.
>
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH v2 5/6] mdev: Update sysfs documentation
From: Cornelia Huck @ 2019-08-30 12:49 UTC (permalink / raw)
To: Parav Pandit
Cc: alex.williamson, jiri, kwankhede, davem, kvm, linux-kernel,
netdev
In-Reply-To: <20190829111904.16042-6-parav@mellanox.com>
On Thu, 29 Aug 2019 06:19:03 -0500
Parav Pandit <parav@mellanox.com> wrote:
> Updated documentation for optional read only sysfs attribute.
I'd probably merge this into the patch introducing the attribute.
>
> Signed-off-by: Parav Pandit <parav@mellanox.com>
> ---
> Documentation/driver-api/vfio-mediated-device.rst | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst
> index 25eb7d5b834b..0ab03d3f5629 100644
> --- a/Documentation/driver-api/vfio-mediated-device.rst
> +++ b/Documentation/driver-api/vfio-mediated-device.rst
> @@ -270,6 +270,7 @@ Directories and Files Under the sysfs for Each mdev Device
> |--- remove
> |--- mdev_type {link to its type}
> |--- vendor-specific-attributes [optional]
> + |--- alias [optional]
"optional" implies "not always present" to me, not "might return a read
error if not available". Don't know if there's a better way to tag
this? Or make it really optional? :)
>
> * remove (write only)
>
> @@ -281,6 +282,10 @@ Example::
>
> # echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
>
> +* alias (read only)
> +Whenever a parent requested to generate an alias, each mdev is assigned a unique
> +alias by the mdev core. This file shows the alias of the mdev device.
It's not really the parent, but the vendor driver requesting this,
right? Also, "each mdev" is a bit ambiguous, as this is only true for
the subset of mdevs created via that driver. Lastly, if we stick with
the "returns an error if not implemented" approach, that should also be
mentioned here.
> +
> Mediated device Hot plug
> ------------------------
>
^ permalink raw reply
* Re: [RESEND PATCH 0/5] Add bluetooth support for Orange Pi 3
From: Marcel Holtmann @ 2019-08-30 12:43 UTC (permalink / raw)
To: Maxime Ripard
Cc: megous, Chen-Yu Tsai, Rob Herring, Johan Hedberg, Mark Rutland,
David S. Miller, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-bluetooth
In-Reply-To: <20190830092104.odipmbflounqpffo@flea>
Hi Maxime,
>>> (Resend to add missing lists, sorry for the noise.)
>>>
>>> This series implements bluetooth support for Xunlong Orange Pi 3 board.
>>>
>>> The board uses AP6256 WiFi/BT 5.0 chip.
>>>
>>> Summary of changes:
>>>
>>> - add more delay to let initialize the chip
>>> - let the kernel detect firmware file path
>>> - add new compatible and update dt-bindings
>>> - update Orange Pi 3 / H6 DTS
>>>
>>> Please take a look.
>>>
>>> thank you and regards,
>>> Ondrej Jirman
>>>
>>> Ondrej Jirman (5):
>>> dt-bindings: net: Add compatible for BCM4345C5 bluetooth device
>>> bluetooth: bcm: Add support for loading firmware for BCM4345C5
>>> bluetooth: hci_bcm: Give more time to come out of reset
>>> arm64: dts: allwinner: h6: Add pin configs for uart1
>>> arm64: dts: allwinner: orange-pi-3: Enable UART1 / Bluetooth
>>>
>>> .../bindings/net/broadcom-bluetooth.txt | 1 +
>>> .../dts/allwinner/sun50i-h6-orangepi-3.dts | 19 +++++++++++++++++++
>>> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 ++++++++++
>>> drivers/bluetooth/btbcm.c | 3 +++
>>> drivers/bluetooth/hci_bcm.c | 3 ++-
>>> 5 files changed, 35 insertions(+), 1 deletion(-)
>>
>> all 5 patches have been applied to bluetooth-next tree.
>
> The DTS patches (last 2) should go through the arm-soc tree, can you
> drop them?
why is that? We have included DTS changes for Bluetooth devices directly all the time. What is different with this hardware?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v2 2/6] mdev: Make mdev alias unique among all mdevs
From: Cornelia Huck @ 2019-08-30 12:40 UTC (permalink / raw)
To: Parav Pandit
Cc: alex.williamson, jiri, kwankhede, davem, kvm, linux-kernel,
netdev
In-Reply-To: <20190829111904.16042-3-parav@mellanox.com>
On Thu, 29 Aug 2019 06:19:00 -0500
Parav Pandit <parav@mellanox.com> wrote:
> Mdev alias should be unique among all the mdevs, so that when such alias
> is used by the mdev users to derive other objects, there is no
> collision in a given system.
>
> Signed-off-by: Parav Pandit <parav@mellanox.com>
>
> ---
> Changelog:
> v1->v2:
> - Moved alias NULL check at beginning
> v0->v1:
> - Fixed inclusiong of alias for NULL check
> - Added ratelimited debug print for sha1 hash collision error
> ---
> drivers/vfio/mdev/mdev_core.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index 3bdff0469607..c9bf2ac362b9 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -388,6 +388,13 @@ int mdev_device_create(struct kobject *kobj, struct device *dev,
> ret = -EEXIST;
> goto mdev_fail;
> }
> + if (alias && tmp->alias && strcmp(alias, tmp->alias) == 0) {
> + mutex_unlock(&mdev_list_lock);
> + ret = -EEXIST;
> + dev_dbg_ratelimited(dev, "Hash collision in alias creation for UUID %pUl\n",
> + uuid);
> + goto mdev_fail;
> + }
> }
>
> mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
Any reason not to merge this into the first patch?
^ permalink raw reply
* Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
From: Cornelia Huck @ 2019-08-30 12:39 UTC (permalink / raw)
To: Parav Pandit
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB48660877881F7A2D757A9C82D1BD0@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Fri, 30 Aug 2019 12:33:22 +0000
Parav Pandit <parav@mellanox.com> wrote:
> > -----Original Message-----
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Friday, August 30, 2019 2:47 PM
> > To: Parav Pandit <parav@mellanox.com>
> > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Subject: Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
> >
> > On Thu, 29 Aug 2019 06:18:59 -0500
> > Parav Pandit <parav@mellanox.com> wrote:
> >
> > > Some vendor drivers want an identifier for an mdev device that is
> > > shorter than the UUID, due to length restrictions in the consumers of
> > > that identifier.
> > >
> > > Add a callback that allows a vendor driver to request an alias of a
> > > specified length to be generated for an mdev device. If generated,
> > > that alias is checked for collisions.
> > >
> > > It is an optional attribute.
> > > mdev alias is generated using sha1 from the mdev name.
> > >
> > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > >
> > > ---
> > > Changelog:
> > > v1->v2:
> > > - Kept mdev_device naturally aligned
> > > - Added error checking for crypt_*() calls
> > > - Corrected a typo from 'and' to 'an'
> > > - Changed return type of generate_alias() from int to char*
> > > v0->v1:
> > > - Moved alias length check outside of the parent lock
> > > - Moved alias and digest allocation from kvzalloc to kzalloc
> > > - &alias[0] changed to alias
> > > - alias_length check is nested under get_alias_length callback check
> > > - Changed comments to start with an empty line
> > > - Fixed cleaunup of hash if mdev_bus_register() fails
> > > - Added comment where alias memory ownership is handed over to mdev
> > > device
> > > - Updated commit log to indicate motivation for this feature
> > > ---
> > > drivers/vfio/mdev/mdev_core.c | 123
> > ++++++++++++++++++++++++++++++-
> > > drivers/vfio/mdev/mdev_private.h | 5 +-
> > > drivers/vfio/mdev/mdev_sysfs.c | 13 ++--
> > > include/linux/mdev.h | 4 +
> > > 4 files changed, 135 insertions(+), 10 deletions(-)
> > ...and detached from the local variable here. Who is freeing it? The comment
> > states that it is done by the mdev, but I don't see it?
> >
> mdev_device_free() frees it.
Ah yes, I overlooked the kfree().
> once its assigned to mdev, mdev is the owner of it.
>
> > This detour via the local variable looks weird to me. Can you either create the
> > alias directly in the mdev (would need to happen later in the function, but I'm
> > not sure why you generate the alias before checking for duplicates anyway), or
> > do an explicit copy?
> Alias duplicate check is done after generating it, because duplicate alias are not allowed.
> The probability of collision is rare.
> So it is speculatively generated without hold the lock, because there is no need to hold the lock.
> It is compared along with guid while mutex lock is held in single loop.
> And if it is duplicate, there is no need to allocate mdev.
>
> It will be sub optimal to run through the mdev list 2nd time after mdev creation and after generating alias for duplicate check.
Ok, but what about copying it? I find this "set local variable to NULL
after ownership is transferred" pattern a bit unintuitive. Copying it
to the mdev (and then unconditionally freeing it) looks more obvious to
me.
^ permalink raw reply
* RE: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
From: Parav Pandit @ 2019-08-30 12:33 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190830111720.04aa54e9.cohuck@redhat.com>
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Friday, August 30, 2019 2:47 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
>
> On Thu, 29 Aug 2019 06:18:59 -0500
> Parav Pandit <parav@mellanox.com> wrote:
>
> > Some vendor drivers want an identifier for an mdev device that is
> > shorter than the UUID, due to length restrictions in the consumers of
> > that identifier.
> >
> > Add a callback that allows a vendor driver to request an alias of a
> > specified length to be generated for an mdev device. If generated,
> > that alias is checked for collisions.
> >
> > It is an optional attribute.
> > mdev alias is generated using sha1 from the mdev name.
> >
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> >
> > ---
> > Changelog:
> > v1->v2:
> > - Kept mdev_device naturally aligned
> > - Added error checking for crypt_*() calls
> > - Corrected a typo from 'and' to 'an'
> > - Changed return type of generate_alias() from int to char*
> > v0->v1:
> > - Moved alias length check outside of the parent lock
> > - Moved alias and digest allocation from kvzalloc to kzalloc
> > - &alias[0] changed to alias
> > - alias_length check is nested under get_alias_length callback check
> > - Changed comments to start with an empty line
> > - Fixed cleaunup of hash if mdev_bus_register() fails
> > - Added comment where alias memory ownership is handed over to mdev
> > device
> > - Updated commit log to indicate motivation for this feature
> > ---
> > drivers/vfio/mdev/mdev_core.c | 123
> ++++++++++++++++++++++++++++++-
> > drivers/vfio/mdev/mdev_private.h | 5 +-
> > drivers/vfio/mdev/mdev_sysfs.c | 13 ++--
> > include/linux/mdev.h | 4 +
> > 4 files changed, 135 insertions(+), 10 deletions(-)
> >
>
> (...)
>
> > +static const char *
> > +generate_alias(const char *uuid, unsigned int max_alias_len) {
> > + struct shash_desc *hash_desc;
> > + unsigned int digest_size;
> > + unsigned char *digest;
> > + unsigned int alias_len;
> > + char *alias;
> > + int ret;
> > +
> > + /*
> > + * Align to multiple of 2 as bin2hex will generate
> > + * even number of bytes.
> > + */
> > + alias_len = roundup(max_alias_len, 2);
> > + alias = kzalloc(alias_len + 1, GFP_KERNEL);
>
> This function allocates alias...
>
> > + if (!alias)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + /* Allocate and init descriptor */
> > + hash_desc = kvzalloc(sizeof(*hash_desc) +
> > + crypto_shash_descsize(alias_hash),
> > + GFP_KERNEL);
> > + if (!hash_desc) {
> > + ret = -ENOMEM;
> > + goto desc_err;
> > + }
> > +
> > + hash_desc->tfm = alias_hash;
> > +
> > + digest_size = crypto_shash_digestsize(alias_hash);
> > +
> > + digest = kzalloc(digest_size, GFP_KERNEL);
> > + if (!digest) {
> > + ret = -ENOMEM;
> > + goto digest_err;
> > + }
> > + ret = crypto_shash_init(hash_desc);
> > + if (ret)
> > + goto hash_err;
> > +
> > + ret = crypto_shash_update(hash_desc, uuid, UUID_STRING_LEN);
> > + if (ret)
> > + goto hash_err;
> > +
> > + ret = crypto_shash_final(hash_desc, digest);
> > + if (ret)
> > + goto hash_err;
> > +
> > + bin2hex(alias, digest, min_t(unsigned int, digest_size, alias_len / 2));
> > + /*
> > + * When alias length is odd, zero out an additional last byte
> > + * that bin2hex has copied.
> > + */
> > + if (max_alias_len % 2)
> > + alias[max_alias_len] = 0;
> > +
> > + kfree(digest);
> > + kvfree(hash_desc);
> > + return alias;
>
> ...and returns it here on success...
>
> > +
> > +hash_err:
> > + kfree(digest);
> > +digest_err:
> > + kvfree(hash_desc);
> > +desc_err:
> > + kfree(alias);
> > + return ERR_PTR(ret);
> > +}
> > +
> > +int mdev_device_create(struct kobject *kobj, struct device *dev,
> > + const char *uuid_str, const guid_t *uuid)
> > {
> > int ret;
> > struct mdev_device *mdev, *tmp;
> > struct mdev_parent *parent;
> > struct mdev_type *type = to_mdev_type(kobj);
> > + const char *alias = NULL;
> >
> > parent = mdev_get_parent(type->parent);
> > if (!parent)
> > return -EINVAL;
> >
> > + if (parent->ops->get_alias_length) {
> > + unsigned int alias_len;
> > +
> > + alias_len = parent->ops->get_alias_length();
> > + if (alias_len) {
> > + alias = generate_alias(uuid_str, alias_len);
>
> ...to be saved into a local variable here...
>
> > + if (IS_ERR(alias)) {
> > + ret = PTR_ERR(alias);
> > + goto alias_fail;
> > + }
> > + }
> > + }
> > mutex_lock(&mdev_list_lock);
> >
> > /* Check for duplicate */
> > @@ -300,6 +398,12 @@ int mdev_device_create(struct kobject *kobj,
> > }
> >
> > guid_copy(&mdev->uuid, uuid);
> > + mdev->alias = alias;
>
> ...and reassigned to the mdev member here...
>
> > + /*
> > + * At this point alias memory is owned by the mdev.
> > + * Mark it NULL, so that only mdev can free it.
> > + */
> > + alias = NULL;
>
> ...and detached from the local variable here. Who is freeing it? The comment
> states that it is done by the mdev, but I don't see it?
>
mdev_device_free() frees it.
once its assigned to mdev, mdev is the owner of it.
> This detour via the local variable looks weird to me. Can you either create the
> alias directly in the mdev (would need to happen later in the function, but I'm
> not sure why you generate the alias before checking for duplicates anyway), or
> do an explicit copy?
Alias duplicate check is done after generating it, because duplicate alias are not allowed.
The probability of collision is rare.
So it is speculatively generated without hold the lock, because there is no need to hold the lock.
It is compared along with guid while mutex lock is held in single loop.
And if it is duplicate, there is no need to allocate mdev.
It will be sub optimal to run through the mdev list 2nd time after mdev creation and after generating alias for duplicate check.
>
> > list_add(&mdev->next, &mdev_list);
> > mutex_unlock(&mdev_list_lock);
> >
> > @@ -346,6 +450,8 @@ int mdev_device_create(struct kobject *kobj,
> > up_read(&parent->unreg_sem);
> > put_device(&mdev->dev);
> > mdev_fail:
> > + kfree(alias);
> > +alias_fail:
> > mdev_put_parent(parent);
> > return ret;
> > }
>
> (...)
^ permalink raw reply
* [PATCH net 5/5] net: aquantia: fix out of memory condition on rx side
From: Igor Russkikh @ 2019-08-30 12:08 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Dmitry Bogdanov
In-Reply-To: <cover.1567163402.git.igor.russkikh@aquantia.com>
From: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
On embedded environments with hard memory limits it is a normal although
rare case when skb can't be allocated on rx part under high traffic.
In such OOM cases napi_complete_done() was not called.
So the napi object became in an invalid state like it is "scheduled".
Kernel do not re-schedules the poll of that napi object.
Consequently, kernel can not remove that object the system hangs on
`ifconfig down` waiting for a poll.
We are fixing this by gracefully closing napi poll routine with correct
invocation of napi_complete_done.
This was reproduced with artificially failing the allocation of skb to
simulate an "out of memory" error case and check that traffic does
not get stuck.
Fixes: 970a2e9864b0 ("net: ethernet: aquantia: Vector operations")
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Signed-off-by: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_vec.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_vec.c b/drivers/net/ethernet/aquantia/atlantic/aq_vec.c
index 715685aa48c3..28892b8acd0e 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_vec.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_vec.c
@@ -86,6 +86,7 @@ static int aq_vec_poll(struct napi_struct *napi, int budget)
}
}
+err_exit:
if (!was_tx_cleaned)
work_done = budget;
@@ -95,7 +96,7 @@ static int aq_vec_poll(struct napi_struct *napi, int budget)
1U << self->aq_ring_param.vec_idx);
}
}
-err_exit:
+
return work_done;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net 4/5] net: aquantia: linkstate irq should be oneshot
From: Igor Russkikh @ 2019-08-30 12:08 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh
In-Reply-To: <cover.1567163402.git.igor.russkikh@aquantia.com>
Declaring threaded irq handler should also indicate the irq is
oneshot. It is oneshot indeed, because HW implements irq automasking
on trigger.
Not declaring this causes some kernel configurations to fail
on interface up, because request_threaded_irq returned an err code.
The issue was originally hidden on normal x86_64 configuration with
latest kernel, because depending on interrupt controller, irq driver
added ONESHOT flag on its own.
Issue was observed on older kernels (4.14) where no such logic exists.
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Reported-by: Michael Symolkin <Michael.Symolkin@aquantia.com>
Fixes: 4c83f170b3ac ("net: aquantia: link status irq handling")
---
drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index e1392766e21e..8f66e7817811 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -393,7 +393,7 @@ int aq_nic_start(struct aq_nic_s *self)
self->aq_nic_cfg.link_irq_vec);
err = request_threaded_irq(irqvec, NULL,
aq_linkstate_threaded_isr,
- IRQF_SHARED,
+ IRQF_SHARED | IRQF_ONESHOT,
self->ndev->name, self);
if (err < 0)
goto err_exit;
--
2.17.1
^ permalink raw reply related
* [PATCH net 3/5] net: aquantia: reapply vlan filters on up
From: Igor Russkikh @ 2019-08-30 12:08 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Dmitry Bogdanov
In-Reply-To: <cover.1567163402.git.igor.russkikh@aquantia.com>
From: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
In case of device reconfiguration the driver may reset the device invisible
for other modules, vlan module in particular. So vlans will not be
removed&created and vlan filters will not be configured in the device.
The patch reapplies the vlan filters at device start.
Fixes: 7975d2aff5afb ("net: aquantia: add support of rx-vlan-filter offload")
Signed-off-by: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
index 100722ad5c2d..b4a0fb281e69 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
@@ -61,6 +61,10 @@ static int aq_ndev_open(struct net_device *ndev)
if (err < 0)
goto err_exit;
+ err = aq_filters_vlans_update(aq_nic);
+ if (err < 0)
+ goto err_exit;
+
err = aq_nic_start(aq_nic);
if (err < 0)
goto err_exit;
--
2.17.1
^ permalink raw reply related
* [PATCH net 2/5] net: aquantia: fix limit of vlan filters
From: Igor Russkikh @ 2019-08-30 12:08 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Dmitry Bogdanov
In-Reply-To: <cover.1567163402.git.igor.russkikh@aquantia.com>
From: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
Fix a limit condition of vlans on the interface before setting vlan
promiscuous mode
Fixes: 48dd73d08d4dd ("net: aquantia: fix vlans not working over bridged network")
Signed-off-by: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_filters.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_filters.c b/drivers/net/ethernet/aquantia/atlantic/aq_filters.c
index b13704544a23..aee827f07c16 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_filters.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_filters.c
@@ -844,7 +844,7 @@ int aq_filters_vlans_update(struct aq_nic_s *aq_nic)
return err;
if (aq_nic->ndev->features & NETIF_F_HW_VLAN_CTAG_FILTER) {
- if (hweight < AQ_VLAN_MAX_FILTERS && hweight > 0) {
+ if (hweight <= AQ_VLAN_MAX_FILTERS && hweight > 0) {
err = aq_hw_ops->hw_filter_vlan_ctrl(aq_hw,
!(aq_nic->packet_filter & IFF_PROMISC));
aq_nic->aq_nic_cfg.is_vlan_force_promisc = false;
--
2.17.1
^ permalink raw reply related
* [PATCH net 1/5] net: aquantia: fix removal of vlan 0
From: Igor Russkikh @ 2019-08-30 12:08 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Dmitry Bogdanov
In-Reply-To: <cover.1567163402.git.igor.russkikh@aquantia.com>
From: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
Due to absence of checking against the rx flow rule when vlan 0 is being
removed, the other rule could be removed instead of the rule with vlan 0
Fixes: 7975d2aff5afb ("net: aquantia: add support of rx-vlan-filter offload")
Signed-off-by: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_filters.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_filters.c b/drivers/net/ethernet/aquantia/atlantic/aq_filters.c
index 440690b18734..b13704544a23 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_filters.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_filters.c
@@ -431,7 +431,8 @@ int aq_del_fvlan_by_vlan(struct aq_nic_s *aq_nic, u16 vlan_id)
if (be16_to_cpu(rule->aq_fsp.h_ext.vlan_tci) == vlan_id)
break;
}
- if (rule && be16_to_cpu(rule->aq_fsp.h_ext.vlan_tci) == vlan_id) {
+ if (rule && rule->type == aq_rx_filter_vlan &&
+ be16_to_cpu(rule->aq_fsp.h_ext.vlan_tci) == vlan_id) {
struct ethtool_rxnfc cmd;
cmd.fs.location = rule->aq_fsp.location;
--
2.17.1
^ permalink raw reply related
* [PATCH net 0/5] net: aquantia: fixes on vlan filters and other conditions
From: Igor Russkikh @ 2019-08-30 12:08 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh
Here is a set of various bug fixes related to vlan filter offload and
two other rare cases.
Dmitry Bogdanov (4):
net: aquantia: fix removal of vlan 0
net: aquantia: fix limit of vlan filters
net: aquantia: reapply vlan filters on up
net: aquantia: fix out of memory condition on rx side
Igor Russkikh (1):
net: aquantia: linkstate irq should be oneshot
drivers/net/ethernet/aquantia/atlantic/aq_filters.c | 5 +++--
drivers/net/ethernet/aquantia/atlantic/aq_main.c | 4 ++++
drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 2 +-
drivers/net/ethernet/aquantia/atlantic/aq_vec.c | 3 ++-
4 files changed, 10 insertions(+), 4 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH 1/5] netfilter: xt_physdev: Fix spurious error message in physdev_mt_check
From: Pablo Neira Ayuso @ 2019-08-30 12:07 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190830120704.6147-1-pablo@netfilter.org>
From: Todd Seidelmann <tseidelmann@linode.com>
Simplify the check in physdev_mt_check() to emit an error message
only when passed an invalid chain (ie, NF_INET_LOCAL_OUT).
This avoids cluttering up the log with errors against valid rules.
For large/heavily modified rulesets, current behavior can quickly
overwhelm the ring buffer, because this function gets called on
every change, regardless of the rule that was changed.
Signed-off-by: Todd Seidelmann <tseidelmann@linode.com>
Acked-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/xt_physdev.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/xt_physdev.c b/net/netfilter/xt_physdev.c
index ead7c6022208..b92b22ce8abd 100644
--- a/net/netfilter/xt_physdev.c
+++ b/net/netfilter/xt_physdev.c
@@ -101,11 +101,9 @@ static int physdev_mt_check(const struct xt_mtchk_param *par)
if (info->bitmask & (XT_PHYSDEV_OP_OUT | XT_PHYSDEV_OP_ISOUT) &&
(!(info->bitmask & XT_PHYSDEV_OP_BRIDGED) ||
info->invert & XT_PHYSDEV_OP_BRIDGED) &&
- par->hook_mask & ((1 << NF_INET_LOCAL_OUT) |
- (1 << NF_INET_FORWARD) | (1 << NF_INET_POST_ROUTING))) {
+ par->hook_mask & (1 << NF_INET_LOCAL_OUT)) {
pr_info_ratelimited("--physdev-out and --physdev-is-out only supported in the FORWARD and POSTROUTING chains with bridged traffic\n");
- if (par->hook_mask & (1 << NF_INET_LOCAL_OUT))
- return -EINVAL;
+ return -EINVAL;
}
if (!brnf_probed) {
--
2.11.0
^ permalink raw reply related
* [PATCH 3/5] netfilter: conntrack: make sysctls per-namespace again
From: Pablo Neira Ayuso @ 2019-08-30 12:07 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190830120704.6147-1-pablo@netfilter.org>
From: Florian Westphal <fw@strlen.de>
When I merged the extension sysctl tables with the main one I forgot to
reset them on netns creation. They currently read/write init_net settings.
Fixes: d912dec12428 ("netfilter: conntrack: merge acct and helper sysctl table with main one")
Fixes: cb2833ed0044 ("netfilter: conntrack: merge ecache and timestamp sysctl tables with main one")
Reported-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_conntrack_standalone.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index e0d392cb3075..0006503d2da9 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -1037,9 +1037,14 @@ static int nf_conntrack_standalone_init_sysctl(struct net *net)
table[NF_SYSCTL_CT_COUNT].data = &net->ct.count;
table[NF_SYSCTL_CT_CHECKSUM].data = &net->ct.sysctl_checksum;
table[NF_SYSCTL_CT_LOG_INVALID].data = &net->ct.sysctl_log_invalid;
+ table[NF_SYSCTL_CT_ACCT].data = &net->ct.sysctl_acct;
+ table[NF_SYSCTL_CT_HELPER].data = &net->ct.sysctl_auto_assign_helper;
#ifdef CONFIG_NF_CONNTRACK_EVENTS
table[NF_SYSCTL_CT_EVENTS].data = &net->ct.sysctl_events;
#endif
+#ifdef CONFIG_NF_CONNTRACK_TIMESTAMP
+ table[NF_SYSCTL_CT_TIMESTAMP].data = &net->ct.sysctl_tstamp;
+#endif
table[NF_SYSCTL_CT_PROTO_TIMEOUT_GENERIC].data = &nf_generic_pernet(net)->timeout;
table[NF_SYSCTL_CT_PROTO_TIMEOUT_ICMP].data = &nf_icmp_pernet(net)->timeout;
table[NF_SYSCTL_CT_PROTO_TIMEOUT_ICMPV6].data = &nf_icmpv6_pernet(net)->timeout;
--
2.11.0
^ permalink raw reply related
* [PATCH 4/5] netfilter: nf_flow_table: clear skb tstamp before xmit
From: Pablo Neira Ayuso @ 2019-08-30 12:07 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190830120704.6147-1-pablo@netfilter.org>
From: Florian Westphal <fw@strlen.de>
If 'fq' qdisc is used and a program has requested timestamps,
skb->tstamp needs to be cleared, else fq will treat these as
'transmit time'.
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_flow_table_ip.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index d68c801dd614..b9e7dd6e60ce 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -228,7 +228,6 @@ static unsigned int nf_flow_xmit_xfrm(struct sk_buff *skb,
{
skb_orphan(skb);
skb_dst_set_noref(skb, dst);
- skb->tstamp = 0;
dst_output(state->net, state->sk, skb);
return NF_STOLEN;
}
@@ -284,6 +283,7 @@ nf_flow_offload_ip_hook(void *priv, struct sk_buff *skb,
flow->timeout = (u32)jiffies + NF_FLOW_TIMEOUT;
iph = ip_hdr(skb);
ip_decrease_ttl(iph);
+ skb->tstamp = 0;
if (unlikely(dst_xfrm(&rt->dst))) {
memset(skb->cb, 0, sizeof(struct inet_skb_parm));
@@ -512,6 +512,7 @@ nf_flow_offload_ipv6_hook(void *priv, struct sk_buff *skb,
flow->timeout = (u32)jiffies + NF_FLOW_TIMEOUT;
ip6h = ipv6_hdr(skb);
ip6h->hop_limit--;
+ skb->tstamp = 0;
if (unlikely(dst_xfrm(&rt->dst))) {
memset(skb->cb, 0, sizeof(struct inet6_skb_parm));
--
2.11.0
^ permalink raw reply related
* [PATCH 5/5] netfilter: nft_meta_bridge: Fix get NFT_META_BRI_IIFVPROTO in network byteorder
From: Pablo Neira Ayuso @ 2019-08-30 12:07 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190830120704.6147-1-pablo@netfilter.org>
From: wenxu <wenxu@ucloud.cn>
Get the vlan_proto of ingress bridge in network byteorder as userspace
expects. Otherwise this is inconsistent with NFT_META_PROTOCOL.
Fixes: 2a3a93ef0ba5 ("netfilter: nft_meta_bridge: Add NFT_META_BRI_IIFVPROTO support")
Signed-off-by: wenxu <wenxu@ucloud.cn>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/bridge/netfilter/nft_meta_bridge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/netfilter/nft_meta_bridge.c b/net/bridge/netfilter/nft_meta_bridge.c
index 1804e867f715..7c9e92b2f806 100644
--- a/net/bridge/netfilter/nft_meta_bridge.c
+++ b/net/bridge/netfilter/nft_meta_bridge.c
@@ -53,7 +53,7 @@ static void nft_meta_bridge_get_eval(const struct nft_expr *expr,
goto err;
br_vlan_get_proto(br_dev, &p_proto);
- nft_reg_store16(dest, p_proto);
+ nft_reg_store16(dest, htons(p_proto));
return;
}
default:
--
2.11.0
^ permalink raw reply related
* [PATCH 2/5] netfilter: nf_conntrack_ftp: Fix debug output
From: Pablo Neira Ayuso @ 2019-08-30 12:07 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190830120704.6147-1-pablo@netfilter.org>
From: Thomas Jarosch <thomas.jarosch@intra2net.com>
The find_pattern() debug output was printing the 'skip' character.
This can be a NULL-byte and messes up further pr_debug() output.
Output without the fix:
kernel: nf_conntrack_ftp: Pattern matches!
kernel: nf_conntrack_ftp: Skipped up to `<7>nf_conntrack_ftp: find_pattern `PORT': dlen = 8
kernel: nf_conntrack_ftp: find_pattern `EPRT': dlen = 8
Output with the fix:
kernel: nf_conntrack_ftp: Pattern matches!
kernel: nf_conntrack_ftp: Skipped up to 0x0 delimiter!
kernel: nf_conntrack_ftp: Match succeeded!
kernel: nf_conntrack_ftp: conntrack_ftp: match `172,17,0,100,200,207' (20 bytes at 4150681645)
kernel: nf_conntrack_ftp: find_pattern `PORT': dlen = 8
Signed-off-by: Thomas Jarosch <thomas.jarosch@intra2net.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_conntrack_ftp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
index 0ecb3e289ef2..8d96738b7dfd 100644
--- a/net/netfilter/nf_conntrack_ftp.c
+++ b/net/netfilter/nf_conntrack_ftp.c
@@ -322,7 +322,7 @@ static int find_pattern(const char *data, size_t dlen,
i++;
}
- pr_debug("Skipped up to `%c'!\n", skip);
+ pr_debug("Skipped up to 0x%hhx delimiter!\n", skip);
*numoff = i;
*numlen = getnum(data + i, dlen - i, cmd, term, numoff);
--
2.11.0
^ permalink raw reply related
* [PATCH 0/5] Netfilter fixes for net
From: Pablo Neira Ayuso @ 2019-08-30 12:06 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
Hi,
The following patchset contains Netfilter fixes for net:
1) Spurious warning when loading rules using the physdev match,
from Todd Seidelmann.
2) Fix FTP conntrack helper debugging output, from Thomas Jarosch.
3) Restore per-netns nf_conntrack_{acct,helper,timeout} sysctl knobs,
from Florian Westphal.
4) Clear skbuff timestamp from the flowtable datapath, also from Florian.
5) Fix incorrect byteorder of NFT_META_BRI_IIFVPROTO, from wenxu.
You can pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
Thanks.
----------------------------------------------------------------
The following changes since commit f53a7ad189594a112167efaf17ea8d0242b5ac00:
r8152: Set memory to all 0xFFs on failed reg reads (2019-08-25 19:52:59 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git HEAD
for you to fetch changes up to daf1de9078792a4d60e36aa7ecf3aadca65277c2:
netfilter: nft_meta_bridge: Fix get NFT_META_BRI_IIFVPROTO in network byteorder (2019-08-30 02:49:04 +0200)
----------------------------------------------------------------
Florian Westphal (2):
netfilter: conntrack: make sysctls per-namespace again
netfilter: nf_flow_table: clear skb tstamp before xmit
Thomas Jarosch (1):
netfilter: nf_conntrack_ftp: Fix debug output
Todd Seidelmann (1):
netfilter: xt_physdev: Fix spurious error message in physdev_mt_check
wenxu (1):
netfilter: nft_meta_bridge: Fix get NFT_META_BRI_IIFVPROTO in network byteorder
net/bridge/netfilter/nft_meta_bridge.c | 2 +-
net/netfilter/nf_conntrack_ftp.c | 2 +-
net/netfilter/nf_conntrack_standalone.c | 5 +++++
net/netfilter/nf_flow_table_ip.c | 3 ++-
net/netfilter/xt_physdev.c | 6 ++----
5 files changed, 11 insertions(+), 7 deletions(-)
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/4] tools: bpftool: improve bpftool build experience
From: Ilya Leoshkevich @ 2019-08-30 11:42 UTC (permalink / raw)
To: Quentin Monnet
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, netdev, oss-drivers,
Lorenz Bauer, Jakub Kicinski
In-Reply-To: <20190830110040.31257-1-quentin.monnet@netronome.com>
> Am 30.08.2019 um 13:00 schrieb Quentin Monnet <quentin.monnet@netronome.com>:
>
> Hi,
> This set attempts to make it easier to build bpftool, in particular when
> passing a specific output directory. This is a follow-up to the
> conversation held last month by Lorenz, Ilya and Jakub [0].
>
> The first patch is a minor fix to bpftool's Makefile, regarding the
> retrieval of kernel version (which currently prints a non-relevant make
> warning on some invocations).
>
> Second patch improves the Makefile commands to support more "make"
> invocations, or to fix building with custom output directory. On Jakub's
> suggestion, a script is also added to BPF selftests in order to keep track
> of the supported build variants.
>
> Building bpftool with "make tools/bpf" from the top of the repository
> generates files in "libbpf/" and "feature/" directories under tools/bpf/
> and tools/bpf/bpftool/. The third patch ensures such directories are taken
> care of on "make clean", and add them to the relevant .gitignore files.
>
> At last, fourth patch is a sligthly modified version of Ilya's fix
> regarding libbpf.a appearing twice on the linking command for bpftool.
>
> [0] https://lore.kernel.org/bpf/CACAyw9-CWRHVH3TJ=Tke2x8YiLsH47sLCijdp=V+5M836R9aAA@mail.gmail.com/
>
> v2:
> - Return error from check script if one of the make invocations returns
> non-zero (even if binary is successfully produced).
> - Run "make clean" from bpf/ and not only bpf/bpftool/ in that same script,
> when relevant.
> - Add a patch to clean up generated "feature/" and "libbpf/" directories.
>
> Cc: Lorenz Bauer <lmb@cloudflare.com>
> Cc: Ilya Leoshkevich <iii@linux.ibm.com>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
>
> Quentin Monnet (4):
> tools: bpftool: ignore make built-in rules for getting kernel version
> tools: bpftool: improve and check builds for different make
> invocations
> tools: bpf: account for generated feature/ and libbpf/ directories
> tools: bpftool: do not link twice against libbpf.a in Makefile
>
> tools/bpf/.gitignore | 1 +
> tools/bpf/Makefile | 5 +-
> tools/bpf/bpftool/.gitignore | 2 +
> tools/bpf/bpftool/Makefile | 28 ++--
> tools/testing/selftests/bpf/Makefile | 3 +-
> .../selftests/bpf/test_bpftool_build.sh | 143 ++++++++++++++++++
> 6 files changed, 167 insertions(+), 15 deletions(-)
> create mode 100755 tools/testing/selftests/bpf/test_bpftool_build.sh
>
> --
> 2.17.1
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com>
for the whole series.
^ permalink raw reply
* Re: [PATCH][V2] wimax/i2400m: remove debug containing bogus calculation of index
From: Dan Carpenter @ 2019-08-30 11:40 UTC (permalink / raw)
To: Colin King
Cc: Inaky Perez-Gonzalez, linux-wimax, David S . Miller, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20190830090711.15300-1-colin.king@canonical.com>
On Fri, Aug 30, 2019 at 10:07:11AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The subtraction of the two pointers is automatically scaled by the
> size of the size of the object the pointers point to, so the division
> by sizeof(*i2400m->barker) is incorrect. This has been broken since
> day one of the driver and is only debug, so remove the debug completely.
>
> Also move && in condition to clean up a checkpatch warning.
>
> Addresses-Coverity: ("Extra sizeof expression")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
regards,
dan carpenter
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox