* Re: uapi/if_ether.h: prevent redefinition of struct ethhdr
From: David Miller @ 2018-01-04 19:31 UTC (permalink / raw)
To: hauke; +Cc: netdev, linux-api, musl, felix.janda, f.fainelli
In-Reply-To: <20180103221421.8273-1-hauke@hauke-m.de>
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Wed, 3 Jan 2018 23:14:21 +0100
> Musl provides its own ethhdr struct definition. Add a guard to prevent
> its definition of the appropriate musl header has already been included.
>
> glibc does not implement this header, but when glibc will implement this
> they can just define __UAPI_DEF_ETHHDR 0 to make it work with the
> kernel.
>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net] ipv6: fix general protection fault in fib6_add()
From: David Miller @ 2018-01-04 19:30 UTC (permalink / raw)
To: weiwan; +Cc: netdev, kafai, dsahern
In-Reply-To: <20180103221159.159648-1-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
Date: Wed, 3 Jan 2018 14:11:59 -0800
> From: Wei Wang <weiwan@google.com>
>
> In fib6_add(), pn could be NULL if fib6_add_1() failed to return a fib6
> node. Checking pn != fn before accessing pn->leaf makes sure pn is not
> NULL.
> This fixes the following GPF reported by syzkaller:
...
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Fixes: 66f5d6ce53e6 ("ipv6: replace rwlock with rcu and spinlock in fib6_table")
> Signed-off-by: Wei Wang <weiwan@google.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] RDS: null pointer dereference in rds_atomic_free_op
From: David Miller @ 2018-01-04 19:20 UTC (permalink / raw)
To: simo.ghannam; +Cc: netdev, linux-rdma, santosh.shilimkar, rds-devel
In-Reply-To: <5a4d45ce.8b8a1c0a.1d072.e5e1@mx.google.com>
From: simo.ghannam@gmail.com
Date: Wed, 3 Jan 2018 21:06:06 +0000
> From: Mohamed Ghannam <simo.ghannam@gmail.com>
>
> set rm->atomic.op_active to 0 when rds_pin_pages() fails
> or the user supplied address is invalid,
> this prevents a NULL pointer usage in rds_atomic_free_op()
>
> Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] sh_eth: fix TSU resource handling
From: David Miller @ 2018-01-04 19:18 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: netdev, linux-renesas-soc, linux-sh, nobuhiro.iwamatsu.yj
In-Reply-To: <20180103170955.343662722@cogentembedded.com>
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Wed, 03 Jan 2018 20:09:49 +0300
> When switching the driver to the managed device API, I managed to break
> the case of a dual Ether devices sharing a single TSU: the 2nd Ether port
> wouldn't probe. Iwamatsu-san has tried to fix this but his patch was buggy
> and he then dropped the ball...
>
> The solution is to limit calling devm_request_mem_region() to the first
> of the two ports sharing the same TSU, so devm_ioremap_resource() can't
> be used anymore for the TSU resource...
>
> Fixes: d5e07e69218f ("sh_eth: use managed device API")
> Reported-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: Issue with commit fea23fb591cc "net: phy: convert read-modify-write to phy_modify()"
From: Heiner Kallweit @ 2018-01-04 19:16 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Andrew Lunn, David S. Miller, netdev@vger.kernel.org
In-Reply-To: <20180104114439.GL28752@n2100.armlinux.org.uk>
Am 04.01.2018 um 12:44 schrieb Russell King - ARM Linux:
> On Thu, Jan 04, 2018 at 08:00:53AM +0100, Heiner Kallweit wrote:
>> Parameter mask of phy_modify() holds the bits to be cleared.
>> In the mentioned commit parameter mask seems to be inverted in
>> few cases, what IMO is wrong (see example).
>
> I'd be grateful if you could list those that you think are wrong please.
> For function __phy_modify documentation and implementation conflict.
Documentation states "(value & mask) | set" whilst implementation is
"(value & ~mask) | set". Based on the subsequent patches I assume
that your intention is what is documented.
Personally I find "ret & ~mask" more intuitive (see also set_mask_bits
in include/linux/bitops.h) but this may be a question of personal taste.
In kernel code both flavors are used.
+ * Unlocked helper function which allows a PHY register to be modified as
+ * new register value = (old register value & mask) | set
+ */
+int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
+{
+ int ret, res;
+
+ ret = __phy_read(phydev, regnum);
+ if (ret >= 0) {
+ res = __phy_write(phydev, regnum, (ret & ~mask) | set);
+ if (res < 0)
+ ret = res;
+ }
+
+ return ret;
+}
Could you please advise whether documentation or implementation reflect
your intention? Then I'll check again which changes I'd consider to
be wrong.
Regards, Heiner
>> Maybe I miss something, could you please check?
>
> It's entirely possible that some are wrong - the patch started out as
> having the mask argument inverted, but during its evolution, that was
> corrected, and I thought all places had been updated - maybe they were
> initially wrong.
>
> I did go through the patch several times before sending it to try to
> ensure that it was correct, but must have overlooked some, because the
> one you quote is one I definitely looked at several times. It's highly
> likely that if I have another look through the patch, I still won't
> spot those that you've found.
>
>> And somehow related:
>> When adding such helpers, wouldn't it make sense to add
>> helpers for setting / clearing bits too? Something like:
>> phy_set_bits(phydev, reg, val) -> phy_modify(phydev, reg, 0, val)
>
> Maybe, but lets try and solve the problems with the existing patch
> first.
>
> Thanks for reporting this, and sorry for the hassle.
>
^ permalink raw reply
* Re: [PATCH net] net: stmmac: enable EEE in MII, GMII or RGMII only
From: David Miller @ 2018-01-04 19:02 UTC (permalink / raw)
To: jbrunet
Cc: netdev, peppe.cavallaro, alexandre.torgue, linux-kernel,
linux-amlogic
In-Reply-To: <20180103154629.1433-1-jbrunet@baylibre.com>
From: Jerome Brunet <jbrunet@baylibre.com>
Date: Wed, 3 Jan 2018 16:46:29 +0100
> Note in the databook - Section 4.4 - EEE :
> " The EEE feature is not supported when the MAC is configured to use the
> TBI, RTBI, SMII, RMII or SGMII single PHY interface. Even if the MAC
> supports multiple PHY interfaces, you should activate the EEE mode only
> when the MAC is operating with GMII, MII, or RGMII interface."
>
> Applying this restriction solves a stability issue observed on Amlogic
> gxl platforms operating with RMII interface and the internal PHY.
>
> Fixes: 83bf79b6bb64 ("stmmac: disable at run-time the EEE if not supported")
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> Tested-by: Arnaud Patard <arnaud.patard@rtp-net.org>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: bonding: Completion of error handling around bond_update_slave_arr()
From: Mahesh Bandewar (महेश बंडेवार) @ 2018-01-04 18:52 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-netdev, Andy Gospodarek, Jay Vosburgh, Veaceslav Falico,
LKML, kernel-janitors
In-Reply-To: <4af2b714-245a-0f0d-5982-818d5f81cade@users.sourceforge.net>
On Thu, Jan 4, 2018 at 12:19 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>> If you see 8 out of 9 call sites in this file ignore the return value.
>
> How do you think about to fix error detection and corresponding
> exception handling then?
>
If I understand your question correctly - not having memory is not a
correctable error and hence there are consequences. In this case, the
slave_arr is not going to be rebuilt and this might mean loosing
packets (if the interface was dropped) or not using the interface to
send packets if that was added (very unlikely case). From host's
perspective, however, this might be the last thing you want to worry
about when there is no memory left.
> Regards,
> Markus
^ permalink raw reply
* Re: [net-next 12/15] i40evf: Drop i40evf_fire_sw_int as it is prone to races
From: David Miller @ 2018-01-04 18:46 UTC (permalink / raw)
To: alexander.duyck
Cc: sergei.shtylyov, jeffrey.t.kirsher, alexander.h.duyck, netdev,
nhorman, sassmann, jogreene
In-Reply-To: <CAKgT0Uc7bQnQm_qZVK_XsPRTF2E0mZWofvyG40p_7fubHoFN8Q@mail.gmail.com>
From: Alexander Duyck <alexander.duyck@gmail.com>
Date: Thu, 4 Jan 2018 09:51:21 -0800
> I think the patches were reordered an the use of that change was
> dropped from the patch. I'll work with Jeff to make certain that the
> comment about replacing the line is removed.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next] tg3: Add Macronix NVRAM support
From: David Miller @ 2018-01-04 18:39 UTC (permalink / raw)
To: satish.baddipadige
Cc: netdev, michael.chan, prashant, siva.kallam, prashant.sreedharan
In-Reply-To: <1514971756-28420-1-git-send-email-sbaddipa@broadcom.com>
From: Satish Baddipadige <satish.baddipadige@broadcom.com>
Date: Wed, 3 Jan 2018 14:59:16 +0530
> From: Prashant Sreedharan <prashant.sreedharan@broadcom.com>
>
> This patch adds the support for Macronix NVRAM
>
> Signed-off-by: Prashant Sreedharan <prashant.sreedharan@broadcom.com>
> Signed-off-by: Satish Baddipadige <satish.baddipadige@broadcom.com>
> Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: Update my email address.
From: David Miller @ 2018-01-04 18:38 UTC (permalink / raw)
To: pshelar; +Cc: netdev
In-Reply-To: <1514952882-7040-1-git-send-email-pshelar@ovn.org>
From: Pravin B Shelar <pshelar@ovn.org>
Date: Tue, 2 Jan 2018 20:14:42 -0800
> Signed-off-by: Pravin Shelar <pshelar@ovn.org>
Applied, but please take Joe's feedback into consideration.
^ permalink raw reply
* Re: [net-next v2] ipv6: sr: export some functions of seg6local
From: David Miller @ 2018-01-04 18:37 UTC (permalink / raw)
To: amsalam20; +Cc: david.lebrun, netdev, linux-kernel
In-Reply-To: <1514588912-1292-1-git-send-email-amsalam20@gmail.com>
From: Ahmed Abdelsalam <amsalam20@gmail.com>
Date: Sat, 30 Dec 2017 00:08:32 +0100
> Some functions of seg6local are very useful to process SRv6
> encapsulated packets
>
> This patch exports some functions of seg6local that are useful and
> can be re-used at different parts of the kernel.
>
> The set of exported functions are:
> (1) seg6_get_srh()
> (2) seg6_advance_nextseg()
> (3) seg6_lookup_nexthop
>
> Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>
There is no way I am applying this as-is.
Until you can submit this alongside an in-tree user of these symbols,
these symbol exports are not going to happen.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next 0/2] net: dsa: lan9303: phy_addr_sel_strap rename and retype
From: David Miller @ 2018-01-04 18:35 UTC (permalink / raw)
To: privat; +Cc: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel
In-Reply-To: <20171229123824.28452-1-privat@egil-hjelmeland.no>
From: Egil Hjelmeland <privat@egil-hjelmeland.no>
Date: Fri, 29 Dec 2017 13:38:22 +0100
> Non functional cleanups involving chip->phy_addr_sel_strap.
> As promised in https://lkml.org/lkml/2017/11/6/273
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
From: Michael S. Tsirkin @ 2018-01-04 18:23 UTC (permalink / raw)
To: Jason Baron
Cc: davem, jasowang, netdev, virtualization, qemu-devel, virtio-dev
In-Reply-To: <20180104201934-mutt-send-email-mst@kernel.org>
On Thu, Jan 04, 2018 at 08:22:08PM +0200, Michael S. Tsirkin wrote:
> On Thu, Jan 04, 2018 at 01:12:30PM -0500, Jason Baron wrote:
> >
> >
> > On 01/04/2018 12:05 PM, Michael S. Tsirkin wrote:
> > > On Thu, Jan 04, 2018 at 12:16:44AM -0500, Jason Baron wrote:
> > >> The ability to set speed and duplex for virtio_net is useful in various
> > >> scenarios as described here:
> > >>
> > >> 16032be virtio_net: add ethtool support for set and get of settings
> > >>
> > >> However, it would be nice to be able to set this from the hypervisor,
> > >> such that virtio_net doesn't require custom guest ethtool commands.
> > >>
> > >> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
> > >> the hypervisor to export a linkspeed and duplex setting. The user can
> > >> subsequently overwrite it later if desired via: 'ethtool -s'.
> > >>
> > >> Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention
> > >> is that device feature bits are to grow down from bit 63, since the
> > >> transports are starting from bit 24 and growing up.
> > >>
> > >> Signed-off-by: Jason Baron <jbaron@akamai.com>
> > >> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > >> Cc: Jason Wang <jasowang@redhat.com>
> > >> Cc: virtio-dev@lists.oasis-open.org
> > >> ---
> > >> drivers/net/virtio_net.c | 19 ++++++++++++++++++-
> > >> include/uapi/linux/virtio_net.h | 13 +++++++++++++
> > >> 2 files changed, 31 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > >> index 6fb7b65..0b2d314 100644
> > >> --- a/drivers/net/virtio_net.c
> > >> +++ b/drivers/net/virtio_net.c
> > >> @@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work)
> > >>
> > >> vi->status = v;
> > >>
> > >> + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
> > >
> > > BTW we can avoid this read for when link goes down.
> > > Not a big deal but still.
> >
> > So you are saying that we can just set vi->speed and vi->duplex to
> > 'unknown' when the link goes down and not check for the presence of
> > VIRTIO_NET_F_SPEED_DUPLEX?
> >
> > If so, that could over-write what the user may have configured in the
> > guest via 'ethtool -s' when the link goes down, so that would be a
> > change in behavior, but perhaps that is ok?
>
> No - what I am saying is that your patch overwrites the values
> set by user when link goes down.
>
> I suggest limiting this call to when
>
> if (vi->status & VIRTIO_NET_S_LINK_UP)
>
> and then the values are overwritten when link goes up
> which seems closer to what a user might expect.
>
> >
> > I think I would prefer to have the link down event still check for
> > VIRTIO_NET_F_SPEED_DUPLEX before changing speed/duplex. That way we
> > still have 2 modes for updating the fields:
> >
> > 1) completely guest controlled. Same as we have now and host does not
> > change any values and does not set VIRTIO_NET_F_SPEED_DUPLEX flag (hence
> > don't remove above check).
> >
> > 2) if speed or duplex or speed is set in the qemu command line, then set
> > the VIRTIO_NET_F_SPEED_DUPLEX and have host control the settings of
> > speed/duplex (with ability of guest to over-write if it wanted to).
I agree - I don't see a reason to touch the speed/duplex values when
VIRTIO_NET_F_SPEED_DUPLEX has not been negotiated.
> >
> >
> > >
> > >> + u32 speed;
> > >> + u8 duplex;
> > >> +
> > >> + speed = virtio_cread32(vi->vdev,
> > >> + offsetof(struct virtio_net_config,
> > >> + speed));
> > >> + if (ethtool_validate_speed(speed))
> > >> + vi->speed = speed;
> > >> + duplex = virtio_cread8(vi->vdev,
> > >> + offsetof(struct virtio_net_config,
> > >> + duplex));
> > >> + if (ethtool_validate_duplex(duplex))
> > >> + vi->duplex = duplex;
> > >> + }
> > >> +
> > >> if (vi->status & VIRTIO_NET_S_LINK_UP) {
> > >> netif_carrier_on(vi->dev);
> > >> netif_tx_wake_all_queues(vi->dev);
> > >
> > > OK so this handles the case when VIRTIO_NET_F_STATUS is set,
> > > but when it's clear we need to call this from virtnet_probe.
> > >
> > > I propose moving this chunk to a function and calling from two places.
> > >
> >
> > good point. will update.
> >
> > Thanks,
> >
> > -Jason
> >
> > >
> > >> @@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {
> > >> VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
> > >> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> > >> VIRTIO_NET_F_CTRL_MAC_ADDR, \
> > >> - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> > >> + VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> > >> + VIRTIO_NET_F_SPEED_DUPLEX
> > >>
> > >> static unsigned int features[] = {
> > >> VIRTNET_FEATURES,
> > >> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> > >> index fc353b5..5de6ed3 100644
> > >> --- a/include/uapi/linux/virtio_net.h
> > >> +++ b/include/uapi/linux/virtio_net.h
> > >> @@ -57,6 +57,8 @@
> > >> * Steering */
> > >> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
> > >>
> > >> +#define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */
> > >> +
> > >> #ifndef VIRTIO_NET_NO_LEGACY
> > >> #define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
> > >> #endif /* VIRTIO_NET_NO_LEGACY */
> > >> @@ -76,6 +78,17 @@ struct virtio_net_config {
> > >> __u16 max_virtqueue_pairs;
> > >> /* Default maximum transmit unit advice */
> > >> __u16 mtu;
> > >> + /*
> > >> + * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
> > >> + * Any other value stands for unknown.
> > >> + */
> > >> + __u32 speed;
> > >> + /*
> > >> + * 0x00 - half duplex
> > >> + * 0x01 - full duplex
> > >> + * Any other value stands for unknown.
> > >> + */
> > >> + __u8 duplex;
> > >> } __attribute__((packed));
> > >>
> > >> /*
> > >> --
> > >> 2.6.1
^ permalink raw reply
* Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
From: Michael S. Tsirkin @ 2018-01-04 18:22 UTC (permalink / raw)
To: Jason Baron; +Cc: virtio-dev, netdev, qemu-devel, virtualization, davem
In-Reply-To: <c5e2f8d5-3961-606a-8c60-168f1465bd6e@akamai.com>
On Thu, Jan 04, 2018 at 01:12:30PM -0500, Jason Baron wrote:
>
>
> On 01/04/2018 12:05 PM, Michael S. Tsirkin wrote:
> > On Thu, Jan 04, 2018 at 12:16:44AM -0500, Jason Baron wrote:
> >> The ability to set speed and duplex for virtio_net is useful in various
> >> scenarios as described here:
> >>
> >> 16032be virtio_net: add ethtool support for set and get of settings
> >>
> >> However, it would be nice to be able to set this from the hypervisor,
> >> such that virtio_net doesn't require custom guest ethtool commands.
> >>
> >> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
> >> the hypervisor to export a linkspeed and duplex setting. The user can
> >> subsequently overwrite it later if desired via: 'ethtool -s'.
> >>
> >> Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention
> >> is that device feature bits are to grow down from bit 63, since the
> >> transports are starting from bit 24 and growing up.
> >>
> >> Signed-off-by: Jason Baron <jbaron@akamai.com>
> >> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> >> Cc: Jason Wang <jasowang@redhat.com>
> >> Cc: virtio-dev@lists.oasis-open.org
> >> ---
> >> drivers/net/virtio_net.c | 19 ++++++++++++++++++-
> >> include/uapi/linux/virtio_net.h | 13 +++++++++++++
> >> 2 files changed, 31 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> index 6fb7b65..0b2d314 100644
> >> --- a/drivers/net/virtio_net.c
> >> +++ b/drivers/net/virtio_net.c
> >> @@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work)
> >>
> >> vi->status = v;
> >>
> >> + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
> >
> > BTW we can avoid this read for when link goes down.
> > Not a big deal but still.
>
> So you are saying that we can just set vi->speed and vi->duplex to
> 'unknown' when the link goes down and not check for the presence of
> VIRTIO_NET_F_SPEED_DUPLEX?
>
> If so, that could over-write what the user may have configured in the
> guest via 'ethtool -s' when the link goes down, so that would be a
> change in behavior, but perhaps that is ok?
No - what I am saying is that your patch overwrites the values
set by user when link goes down.
I suggest limiting this call to when
if (vi->status & VIRTIO_NET_S_LINK_UP)
and then the values are overwritten when link goes up
which seems closer to what a user might expect.
>
> I think I would prefer to have the link down event still check for
> VIRTIO_NET_F_SPEED_DUPLEX before changing speed/duplex. That way we
> still have 2 modes for updating the fields:
>
> 1) completely guest controlled. Same as we have now and host does not
> change any values and does not set VIRTIO_NET_F_SPEED_DUPLEX flag (hence
> don't remove above check).
>
> 2) if speed or duplex or speed is set in the qemu command line, then set
> the VIRTIO_NET_F_SPEED_DUPLEX and have host control the settings of
> speed/duplex (with ability of guest to over-write if it wanted to).
>
>
> >
> >> + u32 speed;
> >> + u8 duplex;
> >> +
> >> + speed = virtio_cread32(vi->vdev,
> >> + offsetof(struct virtio_net_config,
> >> + speed));
> >> + if (ethtool_validate_speed(speed))
> >> + vi->speed = speed;
> >> + duplex = virtio_cread8(vi->vdev,
> >> + offsetof(struct virtio_net_config,
> >> + duplex));
> >> + if (ethtool_validate_duplex(duplex))
> >> + vi->duplex = duplex;
> >> + }
> >> +
> >> if (vi->status & VIRTIO_NET_S_LINK_UP) {
> >> netif_carrier_on(vi->dev);
> >> netif_tx_wake_all_queues(vi->dev);
> >
> > OK so this handles the case when VIRTIO_NET_F_STATUS is set,
> > but when it's clear we need to call this from virtnet_probe.
> >
> > I propose moving this chunk to a function and calling from two places.
> >
>
> good point. will update.
>
> Thanks,
>
> -Jason
>
> >
> >> @@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {
> >> VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
> >> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> >> VIRTIO_NET_F_CTRL_MAC_ADDR, \
> >> - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> >> + VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> >> + VIRTIO_NET_F_SPEED_DUPLEX
> >>
> >> static unsigned int features[] = {
> >> VIRTNET_FEATURES,
> >> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> >> index fc353b5..5de6ed3 100644
> >> --- a/include/uapi/linux/virtio_net.h
> >> +++ b/include/uapi/linux/virtio_net.h
> >> @@ -57,6 +57,8 @@
> >> * Steering */
> >> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
> >>
> >> +#define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */
> >> +
> >> #ifndef VIRTIO_NET_NO_LEGACY
> >> #define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
> >> #endif /* VIRTIO_NET_NO_LEGACY */
> >> @@ -76,6 +78,17 @@ struct virtio_net_config {
> >> __u16 max_virtqueue_pairs;
> >> /* Default maximum transmit unit advice */
> >> __u16 mtu;
> >> + /*
> >> + * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
> >> + * Any other value stands for unknown.
> >> + */
> >> + __u32 speed;
> >> + /*
> >> + * 0x00 - half duplex
> >> + * 0x01 - full duplex
> >> + * Any other value stands for unknown.
> >> + */
> >> + __u8 duplex;
> >> } __attribute__((packed));
> >>
> >> /*
> >> --
> >> 2.6.1
^ permalink raw reply
* Re: general protection fault in __netlink_ns_capable
From: Andrei Vagin @ 2018-01-04 18:14 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: syzbot, Jason, Andrew Morton, alexander.deucher, avagin,
Chris Wilson, David Miller, David Ahern, Eric Dumazet,
elena.reshetova, Greg Kroah-Hartman, Herbert Xu, Johannes Berg,
LKML, Xin Long, mchehab, netdev, syzkaller-bugs
In-Reply-To: <CACT4Y+ZNkQaJ0WWv5F6r_yFwyrniF4s9CXJ62DbKYioMieRfGw@mail.gmail.com>
On Thu, Jan 04, 2018 at 01:01:17PM +0100, Dmitry Vyukov wrote:
> On Wed, Jan 3, 2018 at 8:37 AM, Andrei Vagin <avagin@virtuozzo.com> wrote:
> >> > Hello,
> >> >
> >> > syzkaller hit the following crash on
> >> > 75aa5540627fdb3d8f86229776ea87f995275351
> >> > git://git.cmpxchg.org/linux-mmots.git/master
> >> > compiler: gcc (GCC) 7.1.1 20170620
> >> > .config is attached
> >> > Raw console output is attached.
> >> > C reproducer is attached
> >> > syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> >> > for information about syzkaller reproducers
> >> >
> >> >
> >> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> >> > Reported-by: syzbot+e432865c29eb4c48c142@syzkaller.appspotmail.com
> >> > It will help syzbot understand when the bug is fixed. See footer for
> >> > details.
> >> > If you forward the report, please keep this part and the footer.
> >> >
> >> > netlink: 3 bytes leftover after parsing attributes in process
> >> > `syzkaller140561'.
> >> > netlink: 3 bytes leftover after parsing attributes in process
> >> > `syzkaller140561'.
> >> > netlink: 3 bytes leftover after parsing attributes in process
> >> > `syzkaller140561'.
> >> > kasan: CONFIG_KASAN_INLINE enabled
> >> > kasan: GPF could be caused by NULL-ptr deref or user memory access
> >> > general protection fault: 0000 [#1] SMP KASAN
> >> > Dumping ftrace buffer:
> >> > (ftrace buffer empty)
> >> > Modules linked in:
> >> > CPU: 1 PID: 3149 Comm: syzkaller140561 Not tainted 4.15.0-rc4-mm1+ #47
> >> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> >> > Google 01/01/2011
> >> > RIP: 0010:__netlink_ns_capable+0x8b/0x120 net/netlink/af_netlink.c:868
> >>
> >> NETLINK_CB(skb).sk is NULL here. It looks like we have to use
> >> sk_ns_capable instead of netlink_ns_capable:
> >>
> >> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> >> index c688dc564b11..408c75de52ea 100644
> >> --- a/net/core/rtnetlink.c
> >> +++ b/net/core/rtnetlink.c
> >> @@ -1762,7 +1762,7 @@ static struct net *get_target_net(struct sk_buff
> >> *skb, int netnsid)
> >> /* For now, the caller is required to have CAP_NET_ADMIN in
> >> * the user namespace owning the target net ns.
> >> */
> >> - if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
> >> + if (!sk_ns_capable(skb->sk, net->user_ns, CAP_NET_ADMIN)) {
> >> put_net(net);
> >> return ERR_PTR(-EACCES);
> >> }
> >>
> >
> > get_target_net() is used twice in the code. In rtnl_getlink(), we need
> > to use netlink_ns_capable(skb, ...), but in rtnl_dump_ifinfo, we need to
> > use sk_ns_capable(skb->sk, ...).
> >
> > Pls, take a look at this patch:
> > https://patchwork.ozlabs.org/patch/854896/
> > Subject: rtnetlink: give a user socket to get_target_net()
>
>
> Please include this tag into the commit:
>
I sent v2 with this tag. Sorry for inconvenience.
https://patchwork.ozlabs.org/patch/855147/
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+e432865c29eb4c48c142@syzkaller.appspotmail.com
> > > It will help syzbot understand when the bug is fixed.
^ permalink raw reply
* Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
From: Jason Baron @ 2018-01-04 18:12 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: davem, jasowang, netdev, virtualization, qemu-devel, virtio-dev
In-Reply-To: <20180104190309-mutt-send-email-mst@kernel.org>
On 01/04/2018 12:05 PM, Michael S. Tsirkin wrote:
> On Thu, Jan 04, 2018 at 12:16:44AM -0500, Jason Baron wrote:
>> The ability to set speed and duplex for virtio_net is useful in various
>> scenarios as described here:
>>
>> 16032be virtio_net: add ethtool support for set and get of settings
>>
>> However, it would be nice to be able to set this from the hypervisor,
>> such that virtio_net doesn't require custom guest ethtool commands.
>>
>> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
>> the hypervisor to export a linkspeed and duplex setting. The user can
>> subsequently overwrite it later if desired via: 'ethtool -s'.
>>
>> Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention
>> is that device feature bits are to grow down from bit 63, since the
>> transports are starting from bit 24 and growing up.
>>
>> Signed-off-by: Jason Baron <jbaron@akamai.com>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: virtio-dev@lists.oasis-open.org
>> ---
>> drivers/net/virtio_net.c | 19 ++++++++++++++++++-
>> include/uapi/linux/virtio_net.h | 13 +++++++++++++
>> 2 files changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 6fb7b65..0b2d314 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work)
>>
>> vi->status = v;
>>
>> + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
>
> BTW we can avoid this read for when link goes down.
> Not a big deal but still.
So you are saying that we can just set vi->speed and vi->duplex to
'unknown' when the link goes down and not check for the presence of
VIRTIO_NET_F_SPEED_DUPLEX?
If so, that could over-write what the user may have configured in the
guest via 'ethtool -s' when the link goes down, so that would be a
change in behavior, but perhaps that is ok?
I think I would prefer to have the link down event still check for
VIRTIO_NET_F_SPEED_DUPLEX before changing speed/duplex. That way we
still have 2 modes for updating the fields:
1) completely guest controlled. Same as we have now and host does not
change any values and does not set VIRTIO_NET_F_SPEED_DUPLEX flag (hence
don't remove above check).
2) if speed or duplex or speed is set in the qemu command line, then set
the VIRTIO_NET_F_SPEED_DUPLEX and have host control the settings of
speed/duplex (with ability of guest to over-write if it wanted to).
>
>> + u32 speed;
>> + u8 duplex;
>> +
>> + speed = virtio_cread32(vi->vdev,
>> + offsetof(struct virtio_net_config,
>> + speed));
>> + if (ethtool_validate_speed(speed))
>> + vi->speed = speed;
>> + duplex = virtio_cread8(vi->vdev,
>> + offsetof(struct virtio_net_config,
>> + duplex));
>> + if (ethtool_validate_duplex(duplex))
>> + vi->duplex = duplex;
>> + }
>> +
>> if (vi->status & VIRTIO_NET_S_LINK_UP) {
>> netif_carrier_on(vi->dev);
>> netif_tx_wake_all_queues(vi->dev);
>
> OK so this handles the case when VIRTIO_NET_F_STATUS is set,
> but when it's clear we need to call this from virtnet_probe.
>
> I propose moving this chunk to a function and calling from two places.
>
good point. will update.
Thanks,
-Jason
>
>> @@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {
>> VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
>> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
>> VIRTIO_NET_F_CTRL_MAC_ADDR, \
>> - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
>> + VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
>> + VIRTIO_NET_F_SPEED_DUPLEX
>>
>> static unsigned int features[] = {
>> VIRTNET_FEATURES,
>> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
>> index fc353b5..5de6ed3 100644
>> --- a/include/uapi/linux/virtio_net.h
>> +++ b/include/uapi/linux/virtio_net.h
>> @@ -57,6 +57,8 @@
>> * Steering */
>> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
>>
>> +#define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */
>> +
>> #ifndef VIRTIO_NET_NO_LEGACY
>> #define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
>> #endif /* VIRTIO_NET_NO_LEGACY */
>> @@ -76,6 +78,17 @@ struct virtio_net_config {
>> __u16 max_virtqueue_pairs;
>> /* Default maximum transmit unit advice */
>> __u16 mtu;
>> + /*
>> + * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
>> + * Any other value stands for unknown.
>> + */
>> + __u32 speed;
>> + /*
>> + * 0x00 - half duplex
>> + * 0x01 - full duplex
>> + * Any other value stands for unknown.
>> + */
>> + __u8 duplex;
>> } __attribute__((packed));
>>
>> /*
>> --
>> 2.6.1
^ permalink raw reply
* [PATCH] sh_eth: fix SH7757 GEther initialization
From: Sergei Shtylyov @ 2018-01-04 18:06 UTC (permalink / raw)
To: netdev; +Cc: linux-renesas-soc, linux-sh, Sergei Shtylyov
[-- Attachment #1: sh_eth-fix-SH7757-GEther-initialization.patch --]
[-- Type: text/plain, Size: 1786 bytes --]
Renesas SH7757 has 2 Fast and 2 Gigabit Ether controllers, while the
'sh_eth' driver can only reset and initialize TSU of the first controller
pair. Shimoda-san tried to solve that adding the 'needs_init' member to the
'struct sh_eth_plat_data', however the platform code still never sets this
flag. I think that we can infer this information from the 'devno' variable
(set to 'platform_device::id') and reset/init the Ether controller pair
only for an even 'devno'; therefore 'sh_eth_plat_data::needs_init' can be
removed...
Fixes: 150647fb2c31 ("net: sh_eth: change the condition of initialization")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
The patch is against DaveM's 'net.git' repo.
drivers/net/ethernet/renesas/sh_eth.c | 4 ++--
include/linux/sh_eth.h | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)
Index: net/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- net.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net/drivers/net/ethernet/renesas/sh_eth.c
@@ -3254,8 +3254,8 @@ static int sh_eth_drv_probe(struct platf
ndev->features = NETIF_F_HW_VLAN_CTAG_FILTER;
}
- /* initialize first or needed device */
- if (!devno || pd->needs_init) {
+ /* Need to init only the first port of the two sharing a TSU */
+ if (devno % 2 == 0) {
if (mdp->cd->chip_reset)
mdp->cd->chip_reset(ndev);
Index: net/include/linux/sh_eth.h
===================================================================
--- net.orig/include/linux/sh_eth.h
+++ net/include/linux/sh_eth.h
@@ -17,7 +17,6 @@ struct sh_eth_plat_data {
unsigned char mac_addr[ETH_ALEN];
unsigned no_ether_link:1;
unsigned ether_link_active_low:1;
- unsigned needs_init:1;
};
#endif
^ permalink raw reply
* Re: [patch net-next v2 00/10] Add support for resource abstraction
From: David Ahern @ 2018-01-04 18:03 UTC (permalink / raw)
To: Arkadi Sharshevsky, Jiri Pirko, netdev, roopa
Cc: davem, mlxsw, andrew, vivien.didelot, f.fainelli, michael.chan,
ganeshgr, saeedm, matanb, leonro, idosch, jakub.kicinski, ast,
daniel, simon.horman, pieter.jansenvanvuuren, john.hurley,
alexander.h.duyck, linville, gospo, steven.lin1, yuvalm, ogerlitz
In-Reply-To: <5a294d33-acf6-7855-524f-bc9b3f9a5005@mellanox.com>
On 1/4/18 9:13 AM, Arkadi Sharshevsky wrote:
>>>> Also, it seems like the occ of 0 is wrong since we know from past
>>>> responses that if I set linear to 0 all of networking breaks.
Ok, this was a David bug. I was running ifreload after the devlink
reload command, but all of my connections to the switch are through
breakout ports and the ifreload was not running the devlink port split
command. Doing that before the ifreload and all is fine.
^ permalink raw reply
* Re: [net-next 12/15] i40evf: Drop i40evf_fire_sw_int as it is prone to races
From: Alexander Duyck @ 2018-01-04 17:51 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Jeff Kirsher, David Miller, Alexander Duyck, Netdev, Neil Horman,
sassmann, John Greene
In-Reply-To: <2d470a24-a79e-b0f4-3a91-5f2dd0be5da0@cogentembedded.com>
On Thu, Jan 4, 2018 at 1:31 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello!
>
> On 1/4/2018 12:22 AM, Jeff Kirsher wrote:
>
>> From: Alexander Duyck <alexander.h.duyck@intel.com>
>>
>> Having the interrupts firing while we are polling causes extra overhead
>> and
>> isn't needed for most systems out there. If an interrupt is lost us
>> experiencing a 2s latency spike before recovering is still not acceptable
>> and masks the issue. We are better off just identifying systems that lose
>> interrupts and instead enable workarounds for those systems.
>>
>> To that end I am dropping the code that was strobing the interrupts as
>> there is a narrow window where having them enabled can actually cause
>> race issues anyway where a few stray packets might get misses if the
>> interrupt is re-enabled and fires before we call napi_complete.
>>
>> Also replace one line where we were using bit 31 instead of the define
>> for the bit to represent masking the interrupt enable bit.
>
>
> I'm not seeing this change...
>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
> [...]
>
> MBR, Sergei
I think the change was dropped due to patches being reordered.
Basically the use of BIT(31) was in the Rx interrupt moderation
routine.
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/ethernet/intel/i40evf/i40e_txrx.c?h=v4.15-rc5#n1499
Specifically that line is dropped in the patch at:
http://patchwork.ozlabs.org/patch/854014/
I think the patches were reordered an the use of that change was
dropped from the patch. I'll work with Jeff to make certain that the
comment about replacing the line is removed.
Thanks.
- Alex
^ permalink raw reply
* Re: [PATCH][V2] wcn36xx: fix incorrect assignment to msg_body.min_ch_time
From: Bjorn Andersson @ 2018-01-04 17:40 UTC (permalink / raw)
To: Colin King
Cc: Eugene Krasnikov, Kalle Valo, wcn36xx, linux-wireless, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20171229090732.14928-1-colin.king@canonical.com>
On Fri 29 Dec 01:07 PST 2017, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The second assignment to msg_body.min_ch_time is incorrect, it
> should actually be to msg_body.max_ch_time.
>
> Thanks to Bjorn Andersson for identifying the correct way to fix
> this as my original fix was incorrect.
>
> Detected by CoverityScan, CID#1463042 ("Unused Value")
>
> Fixes: 2f3bef4b247e ("wcn36xx: Add hardware scan offload support")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Thanks Colin,
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> ---
> drivers/net/wireless/ath/wcn36xx/smd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index 2914618a0335..2a4871ca9c72 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -626,7 +626,7 @@ int wcn36xx_smd_start_hw_scan(struct wcn36xx *wcn, struct ieee80211_vif *vif,
>
> msg_body.scan_type = WCN36XX_HAL_SCAN_TYPE_ACTIVE;
> msg_body.min_ch_time = 30;
> - msg_body.min_ch_time = 100;
> + msg_body.max_ch_time = 100;
> msg_body.scan_hidden = 1;
> memcpy(msg_body.mac, vif->addr, ETH_ALEN);
> msg_body.p2p_search = vif->p2p;
> --
> 2.14.1
>
^ permalink raw reply
* Re: [patch net-next v2 00/10] Add support for resource abstraction
From: David Miller @ 2018-01-04 17:17 UTC (permalink / raw)
To: dsa
Cc: jiri, arkadis, netdev, roopa, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, saeedm, matanb, leonro,
idosch, jakub.kicinski, ast, daniel, simon.horman,
pieter.jansenvanvuuren, john.hurley, alexander.h.duyck, linville,
gospo, steven.lin1, yuvalm, ogerlitz
In-Reply-To: <5c387961-31c2-33a7-15ff-7bdc7a743b5f@cumulusnetworks.com>
From: David Ahern <dsa@cumulusnetworks.com>
Date: Thu, 4 Jan 2018 09:58:51 -0700
> This is what I am getting at. Apparently, these resource patches for
> devlink require a patched libmnl to work properly. It is wrong for
> iproute2 to accept this patch and to build a devlink command that we
> know does not work. That means iproute2 needs a dependency on a specific
> version of libmnl - a version which does not yet exist because those
> changes have not been accepted and libmnl version released. Adding that
> dependency is going to inconvenience to all current users of the
> iproute2 repo.
+1
^ permalink raw reply
* Re: general protection fault in nf_tables_dump_obj_done
From: Florian Westphal @ 2018-01-04 17:16 UTC (permalink / raw)
To: syzbot
Cc: coreteam, davem, fw, kadlec, linux-kernel, netdev,
netfilter-devel, pablo, syzkaller-bugs
In-Reply-To: <001a113fd5b81b33e40561f63c7a@google.com>
#syz fix: netfilter: nf_tables: fix potential NULL-ptr deref in nf_tables_dump_obj_done()
^ permalink raw reply
* Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
From: Michael S. Tsirkin @ 2018-01-04 17:05 UTC (permalink / raw)
To: Jason Baron
Cc: davem, jasowang, netdev, virtualization, qemu-devel, virtio-dev
In-Reply-To: <e185a03168d0ab68150d21770bf0d02c5ef537c2.1515041373.git.jbaron@akamai.com>
On Thu, Jan 04, 2018 at 12:16:44AM -0500, Jason Baron wrote:
> The ability to set speed and duplex for virtio_net is useful in various
> scenarios as described here:
>
> 16032be virtio_net: add ethtool support for set and get of settings
>
> However, it would be nice to be able to set this from the hypervisor,
> such that virtio_net doesn't require custom guest ethtool commands.
>
> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
> the hypervisor to export a linkspeed and duplex setting. The user can
> subsequently overwrite it later if desired via: 'ethtool -s'.
>
> Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention
> is that device feature bits are to grow down from bit 63, since the
> transports are starting from bit 24 and growing up.
>
> Signed-off-by: Jason Baron <jbaron@akamai.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: virtio-dev@lists.oasis-open.org
> ---
> drivers/net/virtio_net.c | 19 ++++++++++++++++++-
> include/uapi/linux/virtio_net.h | 13 +++++++++++++
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 6fb7b65..0b2d314 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work)
>
> vi->status = v;
>
> + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
BTW we can avoid this read for when link goes down.
Not a big deal but still.
> + u32 speed;
> + u8 duplex;
> +
> + speed = virtio_cread32(vi->vdev,
> + offsetof(struct virtio_net_config,
> + speed));
> + if (ethtool_validate_speed(speed))
> + vi->speed = speed;
> + duplex = virtio_cread8(vi->vdev,
> + offsetof(struct virtio_net_config,
> + duplex));
> + if (ethtool_validate_duplex(duplex))
> + vi->duplex = duplex;
> + }
> +
> if (vi->status & VIRTIO_NET_S_LINK_UP) {
> netif_carrier_on(vi->dev);
> netif_tx_wake_all_queues(vi->dev);
OK so this handles the case when VIRTIO_NET_F_STATUS is set,
but when it's clear we need to call this from virtnet_probe.
I propose moving this chunk to a function and calling from two places.
> @@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {
> VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> VIRTIO_NET_F_CTRL_MAC_ADDR, \
> - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> + VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> + VIRTIO_NET_F_SPEED_DUPLEX
>
> static unsigned int features[] = {
> VIRTNET_FEATURES,
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index fc353b5..5de6ed3 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -57,6 +57,8 @@
> * Steering */
> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
>
> +#define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */
> +
> #ifndef VIRTIO_NET_NO_LEGACY
> #define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
> #endif /* VIRTIO_NET_NO_LEGACY */
> @@ -76,6 +78,17 @@ struct virtio_net_config {
> __u16 max_virtqueue_pairs;
> /* Default maximum transmit unit advice */
> __u16 mtu;
> + /*
> + * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
> + * Any other value stands for unknown.
> + */
> + __u32 speed;
> + /*
> + * 0x00 - half duplex
> + * 0x01 - full duplex
> + * Any other value stands for unknown.
> + */
> + __u8 duplex;
> } __attribute__((packed));
>
> /*
> --
> 2.6.1
^ permalink raw reply
* Re: [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
From: Michael S. Tsirkin @ 2018-01-04 17:02 UTC (permalink / raw)
To: Jason Baron
Cc: davem, jasowang, netdev, virtualization, qemu-devel, virtio-dev
In-Reply-To: <bda195e5-70a3-aecc-260b-7979522d471a@akamai.com>
On Thu, Jan 04, 2018 at 11:57:44AM -0500, Jason Baron wrote:
>
>
> On 01/04/2018 11:27 AM, Michael S. Tsirkin wrote:
> > On Thu, Jan 04, 2018 at 12:16:44AM -0500, Jason Baron wrote:
> >> The ability to set speed and duplex for virtio_net is useful in various
> >> scenarios as described here:
> >>
> >> 16032be virtio_net: add ethtool support for set and get of settings
> >>
> >> However, it would be nice to be able to set this from the hypervisor,
> >> such that virtio_net doesn't require custom guest ethtool commands.
> >>
> >> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
> >> the hypervisor to export a linkspeed and duplex setting. The user can
> >> subsequently overwrite it later if desired via: 'ethtool -s'.
> >>
> >> Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention
> >> is that device feature bits are to grow down from bit 63, since the
> >> transports are starting from bit 24 and growing up.
> >>
> >> Signed-off-by: Jason Baron <jbaron@akamai.com>
> >> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> >> Cc: Jason Wang <jasowang@redhat.com>
> >> Cc: virtio-dev@lists.oasis-open.org
> >> ---
> >> drivers/net/virtio_net.c | 19 ++++++++++++++++++-
> >> include/uapi/linux/virtio_net.h | 13 +++++++++++++
> >> 2 files changed, 31 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> index 6fb7b65..0b2d314 100644
> >> --- a/drivers/net/virtio_net.c
> >> +++ b/drivers/net/virtio_net.c
> >> @@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct work_struct *work)
> >>
> >> vi->status = v;
> >>
> >> + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
> >> + u32 speed;
> >> + u8 duplex;
> >> +
> >> + speed = virtio_cread32(vi->vdev,
> >> + offsetof(struct virtio_net_config,
> >> + speed));
> >> + if (ethtool_validate_speed(speed))
> >> + vi->speed = speed;
> >> + duplex = virtio_cread8(vi->vdev,
> >> + offsetof(struct virtio_net_config,
> >> + duplex));
> >> + if (ethtool_validate_duplex(duplex))
> >> + vi->duplex = duplex;
> >> + }
> >> +
> >> if (vi->status & VIRTIO_NET_S_LINK_UP) {
> >> netif_carrier_on(vi->dev);
> >> netif_tx_wake_all_queues(vi->dev);
> >> @@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {
> >> VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
> >> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> >> VIRTIO_NET_F_CTRL_MAC_ADDR, \
> >> - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> >> + VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> >> + VIRTIO_NET_F_SPEED_DUPLEX
> >>
> >> static unsigned int features[] = {
> >> VIRTNET_FEATURES,
> >
> > Still missing the update from virtnet_config_changed_work, and I think
> > it's important to reflex host changes within guest when the
> > feature bit has been acked.
> >
>
> I update vi->speed and vi->duplex in virtnet_config_changed_work(). And
> I tested using the 'set_link' on/off from the qemu monitor.
> Specifically, an 'off' sets the speed and link to 'unknown', and an 'on'
> returns the speed and link to the configured speed and duplex. So they
> are being updated dynamically now. What host changes are you referring
> to that are not reflected?
>
> Thanks,
>
> -Jason
Ouch, I was reviewing an old version and replied to this one.
Sorry, will re-read now.
>
> >> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> >> index fc353b5..5de6ed3 100644
> >> --- a/include/uapi/linux/virtio_net.h
> >> +++ b/include/uapi/linux/virtio_net.h
> >> @@ -57,6 +57,8 @@
> >> * Steering */
> >> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
> >>
> >> +#define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */
> >> +
> >> #ifndef VIRTIO_NET_NO_LEGACY
> >> #define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
> >> #endif /* VIRTIO_NET_NO_LEGACY */
> >> @@ -76,6 +78,17 @@ struct virtio_net_config {
> >> __u16 max_virtqueue_pairs;
> >> /* Default maximum transmit unit advice */
> >> __u16 mtu;
> >> + /*
> >> + * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
> >> + * Any other value stands for unknown.
> >> + */
> >> + __u32 speed;
> >> + /*
> >> + * 0x00 - half duplex
> >> + * 0x01 - full duplex
> >> + * Any other value stands for unknown.
> >> + */
> >> + __u8 duplex;
> >> } __attribute__((packed));
> >>
> >> /*
> >> --
> >> 2.6.1
^ permalink raw reply
* Re: [patch net-next v2 00/10] Add support for resource abstraction
From: David Ahern @ 2018-01-04 16:58 UTC (permalink / raw)
To: Jiri Pirko
Cc: Arkadi Sharshevsky, netdev, roopa, davem, mlxsw, andrew,
vivien.didelot, f.fainelli, michael.chan, ganeshgr, saeedm,
matanb, leonro, idosch, jakub.kicinski, ast, daniel, simon.horman,
pieter.jansenvanvuuren, john.hurley, alexander.h.duyck, linville,
gospo, steven.lin1, yuvalm, ogerlitz
In-Reply-To: <20180103183636.GF2067@nanopsycho.orion>
On 1/3/18 11:36 AM, Jiri Pirko wrote:
> Wed, Jan 03, 2018 at 07:29:46PM CET, dsa@cumulusnetworks.com wrote:
>> On 1/3/18 11:17 AM, Jiri Pirko wrote:
>>> Wed, Jan 03, 2018 at 07:14:16PM CET, dsa@cumulusnetworks.com wrote:
>>>> On 1/3/18 11:05 AM, Arkadi Sharshevsky wrote:
>>>>> As I stated this is a user-space bug which I fixed, and updated my repo
>>>>> so please pull. Devlink uses mnl,and currently mnl does not support
>>>>> extended ack. I added support for this in my local ver of libmnl:
>>>>>
>>>>> https://github.com/arkadis/libmnl.git
>>>>>
>>>>> On branch master, so you can check it out. Besides this bugs, which were
>>>>> userspace, can please specify what are the pending problems from your
>>>>> point of view? Thanks!
>>>>
>>>> devlink is in iproute2 package and it has extack support. See 'git log
>>>> lib/libnetlink.c'
>>>
>>> Dave, devlink uses libmnl.
>>>
>>
>> Now I remember. You wrote it independently and but needed iproute2 be a
>> delivery vehicle. It uses none of the common infrastructure from
>> iproute2. Could we make this more difficult ....
>
> Feel free to rewrite it to use lib/libnetlink.c. Should not be that
> hard. Note that at the time I was pushing devlink userspace, tipc also
> used libmnl as a part of iproute2, so devlink was not the first one.
> That is why I decided not to rewrite.
>
> As of the rest of the "common infrastructure", what exactly do you
> have in mind?
>
This is what I am getting at. Apparently, these resource patches for
devlink require a patched libmnl to work properly. It is wrong for
iproute2 to accept this patch and to build a devlink command that we
know does not work. That means iproute2 needs a dependency on a specific
version of libmnl - a version which does not yet exist because those
changes have not been accepted and libmnl version released. Adding that
dependency is going to inconvenience to all current users of the
iproute2 repo.
^ 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