From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH v7 net-next 2/4] net: Introduce generic failover module Date: Fri, 20 Apr 2018 18:34:27 +0300 Message-ID: <20180420183021-mutt-send-email-mst@kernel.org> References: <1524188524-28411-1-git-send-email-sridhar.samudrala@intel.com> <1524188524-28411-3-git-send-email-sridhar.samudrala@intel.com> <20180420045657-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: stephen@networkplumber.org, davem@davemloft.net, netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, virtio-dev@lists.oasis-open.org, jesse.brandeburg@intel.com, alexander.h.duyck@intel.com, kubakici@wp.pl, jasowang@redhat.com, loseweigh@gmail.com, jiri@resnulli.us To: "Samudrala, Sridhar" Return-path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:41842 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755550AbeDTPe3 (ORCPT ); Fri, 20 Apr 2018 11:34:29 -0400 Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Fri, Apr 20, 2018 at 08:21:00AM -0700, Samudrala, Sridhar wrote: > > > + finfo = netdev_priv(failover_dev); > > > + > > > + primary_dev = rtnl_dereference(finfo->primary_dev); > > > + standby_dev = rtnl_dereference(finfo->standby_dev); > > > + > > > + if (slave_dev != primary_dev && slave_dev != standby_dev) > > > + goto done; > > > + > > > + if ((primary_dev && failover_xmit_ready(primary_dev)) || > > > + (standby_dev && failover_xmit_ready(standby_dev))) { > > > + netif_carrier_on(failover_dev); > > > + netif_tx_wake_all_queues(failover_dev); > > > + } else { > > > + netif_carrier_off(failover_dev); > > > + netif_tx_stop_all_queues(failover_dev); > > And I think it's a good idea to get stats from device here too. > > Not sure why we need to get stats from lower devs here? link down is often indication of a hardware problem. lower dev might stop responding down the road. > > > +static const struct net_device_ops failover_dev_ops = { > > > + .ndo_open = failover_open, > > > + .ndo_stop = failover_close, > > > + .ndo_start_xmit = failover_start_xmit, > > > + .ndo_select_queue = failover_select_queue, > > > + .ndo_get_stats64 = failover_get_stats, > > > + .ndo_change_mtu = failover_change_mtu, > > > + .ndo_set_rx_mode = failover_set_rx_mode, > > > + .ndo_validate_addr = eth_validate_addr, > > > + .ndo_features_check = passthru_features_check, > > xdp support? > > I think it should be possible to add it be calling the lower dev ndo_xdp routines > with proper checks. can we add this later? I'd be concerned that if you don't xdp userspace will keep poking at lower devs. Then it will stop working if you add this later. -- MST From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: virtio-dev-return-3920-cohuck=redhat.com@lists.oasis-open.org Sender: List-Post: List-Help: List-Unsubscribe: List-Subscribe: Received: from lists.oasis-open.org (oasis-open.org [66.179.20.138]) by lists.oasis-open.org (Postfix) with ESMTP id EFBA35818E7A for ; Fri, 20 Apr 2018 08:34:39 -0700 (PDT) Date: Fri, 20 Apr 2018 18:34:27 +0300 From: "Michael S. Tsirkin" Message-ID: <20180420183021-mutt-send-email-mst@kernel.org> References: <1524188524-28411-1-git-send-email-sridhar.samudrala@intel.com> <1524188524-28411-3-git-send-email-sridhar.samudrala@intel.com> <20180420045657-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: [virtio-dev] Re: [PATCH v7 net-next 2/4] net: Introduce generic failover module To: "Samudrala, Sridhar" Cc: stephen@networkplumber.org, davem@davemloft.net, netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, virtio-dev@lists.oasis-open.org, jesse.brandeburg@intel.com, alexander.h.duyck@intel.com, kubakici@wp.pl, jasowang@redhat.com, loseweigh@gmail.com, jiri@resnulli.us List-ID: On Fri, Apr 20, 2018 at 08:21:00AM -0700, Samudrala, Sridhar wrote: > > > + finfo = netdev_priv(failover_dev); > > > + > > > + primary_dev = rtnl_dereference(finfo->primary_dev); > > > + standby_dev = rtnl_dereference(finfo->standby_dev); > > > + > > > + if (slave_dev != primary_dev && slave_dev != standby_dev) > > > + goto done; > > > + > > > + if ((primary_dev && failover_xmit_ready(primary_dev)) || > > > + (standby_dev && failover_xmit_ready(standby_dev))) { > > > + netif_carrier_on(failover_dev); > > > + netif_tx_wake_all_queues(failover_dev); > > > + } else { > > > + netif_carrier_off(failover_dev); > > > + netif_tx_stop_all_queues(failover_dev); > > And I think it's a good idea to get stats from device here too. > > Not sure why we need to get stats from lower devs here? link down is often indication of a hardware problem. lower dev might stop responding down the road. > > > +static const struct net_device_ops failover_dev_ops = { > > > + .ndo_open = failover_open, > > > + .ndo_stop = failover_close, > > > + .ndo_start_xmit = failover_start_xmit, > > > + .ndo_select_queue = failover_select_queue, > > > + .ndo_get_stats64 = failover_get_stats, > > > + .ndo_change_mtu = failover_change_mtu, > > > + .ndo_set_rx_mode = failover_set_rx_mode, > > > + .ndo_validate_addr = eth_validate_addr, > > > + .ndo_features_check = passthru_features_check, > > xdp support? > > I think it should be possible to add it be calling the lower dev ndo_xdp routines > with proper checks. can we add this later? I'd be concerned that if you don't xdp userspace will keep poking at lower devs. Then it will stop working if you add this later. -- MST --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org