* [PATCH v2 net-next 0/2] ibmvnic: Tunable parameter support @ 2017-10-23 16:30 John Allen 2017-10-23 16:33 ` [PATCH v2 net-next 1/2] ibmvnic: Update reset infrastructure to support tunable parameters John Allen 2017-10-23 16:34 ` [PATCH v2 net-next 2/2] ibmvnic: Fix failover error path for non-fatal resets John Allen 0 siblings, 2 replies; 5+ messages in thread From: John Allen @ 2017-10-23 16:30 UTC (permalink / raw) To: netdev, Thomas Falcon, Nathan Fontenot This series implements support for changing tunable parameters such as the mtu, number of tx/rx queues, and number of buffers per queue via ethtool and ifconfig. v2: Fix conflict with Tom's recently applied TSO/SG patches John Allen (2): ibmvnic: Update reset infrastructure to support tunable parameters ibmvnic: Fix failover error path for non-fatal resets drivers/net/ethernet/ibm/ibmvnic.c | 222 ++++++++++++++++++++++++++++++++----- drivers/net/ethernet/ibm/ibmvnic.h | 24 +++- 2 files changed, 216 insertions(+), 30 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 net-next 1/2] ibmvnic: Update reset infrastructure to support tunable parameters 2017-10-23 16:30 [PATCH v2 net-next 0/2] ibmvnic: Tunable parameter support John Allen @ 2017-10-23 16:33 ` John Allen 2017-10-24 16:04 ` Nathan Fontenot 2017-10-23 16:34 ` [PATCH v2 net-next 2/2] ibmvnic: Fix failover error path for non-fatal resets John Allen 1 sibling, 1 reply; 5+ messages in thread From: John Allen @ 2017-10-23 16:33 UTC (permalink / raw) To: netdev, Thomas Falcon, Nathan Fontenot Update ibmvnic reset infrastructure to include a new reset option that will allow changing of tunable parameters. There currently is no way to request different capabilities from the vnic server on the fly so this patch achieves this by resetting the driver and attempting to log in with the requested changes. If the reset operation fails, the old values of the tunable parameters are stored in the "fallback" struct and we attempt to login with the fallback values. Signed-off-by: John Allen <jallen@linux.vnet.ibm.com> --- diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 11eba82..c2c4a5b 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -115,6 +115,7 @@ static union sub_crq *ibmvnic_next_scrq(struct ibmvnic_adapter *, static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter); static int ibmvnic_init(struct ibmvnic_adapter *); static void release_crq_queue(struct ibmvnic_adapter *); +static int __ibmvnic_set_mac(struct net_device *netdev, struct sockaddr *p); struct ibmvnic_stat { char name[ETH_GSTRING_LEN]; @@ -926,6 +927,11 @@ static int ibmvnic_open(struct net_device *netdev) mutex_lock(&adapter->reset_lock); + if (adapter->desired->mac) { + __ibmvnic_set_mac(netdev, adapter->desired->mac); + adapter->desired->mac = NULL; + } + if (adapter->state != VNIC_CLOSED) { rc = ibmvnic_login(netdev); if (rc) { @@ -1426,7 +1432,7 @@ static void ibmvnic_set_multi(struct net_device *netdev) } } -static int ibmvnic_set_mac(struct net_device *netdev, void *p) +static int __ibmvnic_set_mac(struct net_device *netdev, struct sockaddr *p) { struct ibmvnic_adapter *adapter = netdev_priv(netdev); struct sockaddr *addr = p; @@ -1444,6 +1450,21 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p) return 0; } +static int ibmvnic_set_mac(struct net_device *netdev, void *p) +{ + struct ibmvnic_adapter *adapter = netdev_priv(netdev); + struct sockaddr *addr = p; + + if (adapter->state != VNIC_OPEN) { + memcpy(adapter->desired->mac, addr, sizeof(struct sockaddr)); + return 0; + } + + __ibmvnic_set_mac(netdev, addr); + + return 0; +} + /** * do_reset returns zero if we are able to keep processing reset events, or * non-zero if we hit a fatal error and must halt. @@ -1470,6 +1491,13 @@ static int do_reset(struct ibmvnic_adapter *adapter, if (rc) return rc; + if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM || + adapter->wait_for_reset) { + release_resources(adapter); + release_sub_crqs(adapter); + release_crq_queue(adapter); + } + if (adapter->reset_reason != VNIC_RESET_NON_FATAL) { /* remove the closed state so when we call open it appears * we are coming from the probed state. @@ -1492,16 +1520,23 @@ static int do_reset(struct ibmvnic_adapter *adapter, return 0; } - rc = reset_tx_pools(adapter); - if (rc) - return rc; + if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM || + adapter->wait_for_reset) { + rc = init_resources(adapter); + if (rc) + return rc; + } else { + rc = reset_tx_pools(adapter); + if (rc) + return rc; - rc = reset_rx_pools(adapter); - if (rc) - return rc; + rc = reset_rx_pools(adapter); + if (rc) + return rc; - if (reset_state == VNIC_CLOSED) - return 0; + if (reset_state == VNIC_CLOSED) + return 0; + } } rc = __ibmvnic_open(netdev); @@ -1580,6 +1615,12 @@ static void __ibmvnic_reset(struct work_struct *work) rwi = get_next_rwi(adapter); } + if (adapter->wait_for_reset) { + adapter->wait_for_reset = false; + adapter->reset_done_rc = rc; + complete(&adapter->reset_done); + } + if (rc) { netdev_dbg(adapter->netdev, "Reset failed\n"); free_all_rwi(adapter); @@ -1759,9 +1800,42 @@ static void ibmvnic_netpoll_controller(struct net_device *dev) } #endif +static int wait_for_reset(struct ibmvnic_adapter *adapter) +{ + adapter->fallback->mtu = adapter->req_mtu; + adapter->fallback->rx_queues = adapter->req_rx_queues; + adapter->fallback->tx_queues = adapter->req_tx_queues; + adapter->fallback->rx_entries = adapter->req_rx_add_entries_per_subcrq; + adapter->fallback->tx_entries = adapter->req_tx_entries_per_subcrq; + + init_completion(&adapter->reset_done); + ibmvnic_reset(adapter, VNIC_RESET_CHANGE_PARAM); + adapter->wait_for_reset = true; + wait_for_completion(&adapter->reset_done); + + if (adapter->reset_done_rc) { + adapter->desired->mtu = adapter->fallback->mtu; + adapter->desired->rx_queues = adapter->fallback->rx_queues; + adapter->desired->tx_queues = adapter->fallback->tx_queues; + adapter->desired->rx_entries = adapter->fallback->rx_entries; + adapter->desired->tx_entries = adapter->fallback->tx_entries; + + init_completion(&adapter->reset_done); + ibmvnic_reset(adapter, VNIC_RESET_CHANGE_PARAM); + wait_for_completion(&adapter->reset_done); + } + adapter->wait_for_reset = false; + + return adapter->reset_done_rc; +} + static int ibmvnic_change_mtu(struct net_device *netdev, int new_mtu) { - return -EOPNOTSUPP; + struct ibmvnic_adapter *adapter = netdev_priv(netdev); + + adapter->desired->mtu = new_mtu + ETH_HLEN; + + return wait_for_reset(adapter); } static const struct net_device_ops ibmvnic_netdev_ops = { @@ -1849,6 +1923,27 @@ static void ibmvnic_get_ringparam(struct net_device *netdev, ring->rx_jumbo_pending = 0; } +static int ibmvnic_set_ringparam(struct net_device *netdev, + struct ethtool_ringparam *ring) +{ + struct ibmvnic_adapter *adapter = netdev_priv(netdev); + + if (ring->rx_pending > adapter->max_rx_add_entries_per_subcrq || + ring->tx_pending > adapter->max_tx_entries_per_subcrq) { + netdev_err(netdev, "Invalid request.\n"); + netdev_err(netdev, "Max tx buffers = %llu\n", + adapter->max_rx_add_entries_per_subcrq); + netdev_err(netdev, "Max rx buffers = %llu\n", + adapter->max_tx_entries_per_subcrq); + return -EINVAL; + } + + adapter->desired->rx_entries = ring->rx_pending; + adapter->desired->tx_entries = ring->tx_pending; + + return wait_for_reset(adapter); +} + static void ibmvnic_get_channels(struct net_device *netdev, struct ethtool_channels *channels) { @@ -1864,6 +1959,17 @@ static void ibmvnic_get_channels(struct net_device *netdev, channels->combined_count = 0; } +static int ibmvnic_set_channels(struct net_device *netdev, + struct ethtool_channels *channels) +{ + struct ibmvnic_adapter *adapter = netdev_priv(netdev); + + adapter->desired->rx_queues = channels->rx_count; + adapter->desired->tx_queues = channels->tx_count; + + return wait_for_reset(adapter); +} + static void ibmvnic_get_strings(struct net_device *dev, u32 stringset, u8 *data) { struct ibmvnic_adapter *adapter = netdev_priv(dev); @@ -1960,7 +2066,9 @@ static void ibmvnic_get_ethtool_stats(struct net_device *dev, .set_msglevel = ibmvnic_set_msglevel, .get_link = ibmvnic_get_link, .get_ringparam = ibmvnic_get_ringparam, + .set_ringparam = ibmvnic_set_ringparam, .get_channels = ibmvnic_get_channels, + .set_channels = ibmvnic_set_channels, .get_strings = ibmvnic_get_strings, .get_sset_count = ibmvnic_get_sset_count, .get_ethtool_stats = ibmvnic_get_ethtool_stats, @@ -2426,6 +2534,7 @@ static void ibmvnic_send_req_caps(struct ibmvnic_adapter *adapter, int retry) { struct device *dev = &adapter->vdev->dev; union ibmvnic_crq crq; + int max_entries; if (!retry) { /* Sub-CRQ entries are 32 byte long */ @@ -2437,21 +2546,60 @@ static void ibmvnic_send_req_caps(struct ibmvnic_adapter *adapter, int retry) return; } - /* Get the minimum between the queried max and the entries - * that fit in our PAGE_SIZE - */ - adapter->req_tx_entries_per_subcrq = - adapter->max_tx_entries_per_subcrq > entries_page ? - entries_page : adapter->max_tx_entries_per_subcrq; - adapter->req_rx_add_entries_per_subcrq = - adapter->max_rx_add_entries_per_subcrq > entries_page ? - entries_page : adapter->max_rx_add_entries_per_subcrq; - - adapter->req_tx_queues = adapter->opt_tx_comp_sub_queues; - adapter->req_rx_queues = adapter->opt_rx_comp_queues; - adapter->req_rx_add_queues = adapter->max_rx_add_queues; + if (adapter->desired->mtu) + adapter->req_mtu = adapter->desired->mtu; + else + adapter->req_mtu = adapter->netdev->mtu + ETH_HLEN; + + if (!adapter->desired->tx_entries) + adapter->desired->tx_entries = + adapter->max_tx_entries_per_subcrq; + if (!adapter->desired->rx_entries) + adapter->desired->rx_entries = + adapter->max_rx_add_entries_per_subcrq; + + max_entries = IBMVNIC_MAX_LTB_SIZE / + (adapter->req_mtu + IBMVNIC_BUFFER_HLEN); + + if ((adapter->req_mtu + IBMVNIC_BUFFER_HLEN) * + adapter->desired->tx_entries > IBMVNIC_MAX_LTB_SIZE) { + adapter->desired->tx_entries = max_entries; + } + + if ((adapter->req_mtu + IBMVNIC_BUFFER_HLEN) * + adapter->desired->rx_entries > IBMVNIC_MAX_LTB_SIZE) { + adapter->desired->rx_entries = max_entries; + } - adapter->req_mtu = adapter->netdev->mtu + ETH_HLEN; + if (adapter->desired->tx_entries) + adapter->req_tx_entries_per_subcrq = + adapter->desired->tx_entries; + else + adapter->req_tx_entries_per_subcrq = + adapter->max_tx_entries_per_subcrq; + + if (adapter->desired->rx_entries) + adapter->req_rx_add_entries_per_subcrq = + adapter->desired->rx_entries; + else + adapter->req_rx_add_entries_per_subcrq = + adapter->max_rx_add_entries_per_subcrq; + + if (adapter->desired->tx_queues) + adapter->req_tx_queues = + adapter->desired->tx_queues; + else + adapter->req_tx_queues = + adapter->opt_tx_comp_sub_queues; + + if (adapter->desired->rx_queues) + adapter->req_rx_queues = + adapter->desired->rx_queues; + else + adapter->req_rx_queues = + adapter->opt_rx_comp_queues; + + adapter->req_rx_add_queues = adapter->max_rx_add_queues; } memset(&crq, 0, sizeof(crq)); @@ -3272,6 +3420,7 @@ static int handle_login_rsp(union ibmvnic_crq *login_rsp_crq, struct ibmvnic_adapter *adapter) { struct device *dev = &adapter->vdev->dev; + struct net_device *netdev = adapter->netdev; struct ibmvnic_login_rsp_buffer *login_rsp = adapter->login_rsp_buf; struct ibmvnic_login_buffer *login = adapter->login_buf; int i; @@ -3291,6 +3440,8 @@ static int handle_login_rsp(union ibmvnic_crq *login_rsp_crq, return 0; } + netdev->mtu = adapter->req_mtu - ETH_HLEN; + netdev_dbg(adapter->netdev, "Login Response Buffer:\n"); for (i = 0; i < (adapter->login_rsp_buf_sz - 1) / 8 + 1; i++) { netdev_dbg(adapter->netdev, "%016lx\n", @@ -3846,7 +3997,7 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter) unsigned long timeout = msecs_to_jiffies(30000); int rc; - if (adapter->resetting) { + if (adapter->resetting && !adapter->wait_for_reset) { rc = ibmvnic_reset_crq(adapter); if (!rc) rc = vio_enable_interrupts(adapter->vdev); @@ -3880,7 +4031,7 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter) return -1; } - if (adapter->resetting) + if (adapter->resetting && !adapter->wait_for_reset) rc = reset_sub_crq_queues(adapter); else rc = init_sub_crqs(adapter); @@ -3931,6 +4082,11 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) adapter->vdev = dev; adapter->netdev = netdev; + adapter->desired = kzalloc(sizeof(*adapter->desired), GFP_KERNEL); + adapter->desired->mac = kzalloc(sizeof(*adapter->desired->mac), + GFP_KERNEL); + adapter->fallback = kzalloc(sizeof(*adapter->fallback), GFP_KERNEL); + ether_addr_copy(adapter->mac_addr, mac_addr_p); ether_addr_copy(netdev->dev_addr, adapter->mac_addr); netdev->irq = dev->irq; @@ -3956,6 +4112,8 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) } while (rc == EAGAIN); netdev->mtu = adapter->req_mtu - ETH_HLEN; + netdev->min_mtu = adapter->min_mtu - ETH_HLEN; + netdev->max_mtu = adapter->max_mtu - ETH_HLEN; rc = device_create_file(&dev->dev, &dev_attr_failover); if (rc) @@ -3970,6 +4128,9 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) dev_info(&dev->dev, "ibmvnic registered\n"); adapter->state = VNIC_PROBED; + + adapter->wait_for_reset = false; + return 0; ibmvnic_register_fail: @@ -3993,6 +4154,11 @@ static int ibmvnic_remove(struct vio_dev *dev) mutex_lock(&adapter->reset_lock); release_resources(adapter); + + kfree(adapter->desired->mac); + kfree(adapter->desired); + kfree(adapter->fallback); + release_sub_crqs(adapter); release_crq_queue(adapter); diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index 7aa347a..8de998a 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -42,6 +42,9 @@ #define IBMVNIC_TSO_BUF_SZ 65536 #define IBMVNIC_TSO_BUFS 64 +#define IBMVNIC_MAX_LTB_SIZE ((1 << (MAX_ORDER - 1)) * PAGE_SIZE) +#define IBMVNIC_BUFFER_HLEN 500 + struct ibmvnic_login_buffer { __be32 len; __be32 version; @@ -945,13 +948,23 @@ enum ibmvnic_reset_reason {VNIC_RESET_FAILOVER = 1, VNIC_RESET_MOBILITY, VNIC_RESET_FATAL, VNIC_RESET_NON_FATAL, - VNIC_RESET_TIMEOUT}; + VNIC_RESET_TIMEOUT, + VNIC_RESET_CHANGE_PARAM}; struct ibmvnic_rwi { enum ibmvnic_reset_reason reset_reason; struct list_head list; }; +struct ibmvnic_tunables { + u64 rx_queues; + u64 tx_queues; + u64 rx_entries; + u64 tx_entries; + u64 mtu; + struct sockaddr *mac; +}; + struct ibmvnic_adapter { struct vio_dev *vdev; struct net_device *netdev; @@ -1012,6 +1025,10 @@ struct ibmvnic_adapter { struct completion fw_done; int fw_done_rc; + struct completion reset_done; + int reset_done_rc; + bool wait_for_reset; + /* partner capabilities */ u64 min_tx_queues; u64 min_rx_queues; @@ -1056,4 +1073,7 @@ struct ibmvnic_adapter { struct work_struct ibmvnic_reset; bool resetting; bool napi_enabled, from_passive_init; + + struct ibmvnic_tunables *desired; + struct ibmvnic_tunables *fallback; }; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 net-next 1/2] ibmvnic: Update reset infrastructure to support tunable parameters 2017-10-23 16:33 ` [PATCH v2 net-next 1/2] ibmvnic: Update reset infrastructure to support tunable parameters John Allen @ 2017-10-24 16:04 ` Nathan Fontenot 0 siblings, 0 replies; 5+ messages in thread From: Nathan Fontenot @ 2017-10-24 16:04 UTC (permalink / raw) To: John Allen, netdev, Thomas Falcon On 10/23/2017 11:33 AM, John Allen wrote: > Update ibmvnic reset infrastructure to include a new reset option that will > allow changing of tunable parameters. There currently is no way to request > different capabilities from the vnic server on the fly so this patch > achieves this by resetting the driver and attempting to log in with the > requested changes. If the reset operation fails, the old values of the > tunable parameters are stored in the "fallback" struct and we attempt to > login with the fallback values. > > Signed-off-by: John Allen <jallen@linux.vnet.ibm.com> > --- > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c > index 11eba82..c2c4a5b 100644 > --- a/drivers/net/ethernet/ibm/ibmvnic.c > +++ b/drivers/net/ethernet/ibm/ibmvnic.c > @@ -115,6 +115,7 @@ static union sub_crq *ibmvnic_next_scrq(struct ibmvnic_adapter *, > static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter); > static int ibmvnic_init(struct ibmvnic_adapter *); > static void release_crq_queue(struct ibmvnic_adapter *); > +static int __ibmvnic_set_mac(struct net_device *netdev, struct sockaddr *p); > > struct ibmvnic_stat { > char name[ETH_GSTRING_LEN]; > @@ -926,6 +927,11 @@ static int ibmvnic_open(struct net_device *netdev) > > mutex_lock(&adapter->reset_lock); > > + if (adapter->desired->mac) { > + __ibmvnic_set_mac(netdev, adapter->desired->mac); > + adapter->desired->mac = NULL; Not sure if I missed it, but setting this to NULL doesn't seem right. This is allocated in ibmvnic_probe() but only gets any data in ibmvnic_set_mac() if the adapter state is closed. I think the scenario of probe -> set mac addr -> open -> close -> set mac addr, would fail in the second set mac address because the mac struct pointer is NULL. > + } > + > if (adapter->state != VNIC_CLOSED) { > rc = ibmvnic_login(netdev); > if (rc) { > @@ -1426,7 +1432,7 @@ static void ibmvnic_set_multi(struct net_device *netdev) > } > } > > -static int ibmvnic_set_mac(struct net_device *netdev, void *p) > +static int __ibmvnic_set_mac(struct net_device *netdev, struct sockaddr *p) > { > struct ibmvnic_adapter *adapter = netdev_priv(netdev); > struct sockaddr *addr = p; > @@ -1444,6 +1450,21 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p) > return 0; > } > > +static int ibmvnic_set_mac(struct net_device *netdev, void *p) > +{ > + struct ibmvnic_adapter *adapter = netdev_priv(netdev); > + struct sockaddr *addr = p; > + > + if (adapter->state != VNIC_OPEN) { > + memcpy(adapter->desired->mac, addr, sizeof(struct sockaddr)); > + return 0; > + } > + > + __ibmvnic_set_mac(netdev, addr); > + > + return 0; > +} > + > /** > * do_reset returns zero if we are able to keep processing reset events, or > * non-zero if we hit a fatal error and must halt. > @@ -1470,6 +1491,13 @@ static int do_reset(struct ibmvnic_adapter *adapter, > if (rc) > return rc; > > + if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM || > + adapter->wait_for_reset) { > + release_resources(adapter); > + release_sub_crqs(adapter); > + release_crq_queue(adapter); > + } > + > if (adapter->reset_reason != VNIC_RESET_NON_FATAL) { > /* remove the closed state so when we call open it appears > * we are coming from the probed state. > @@ -1492,16 +1520,23 @@ static int do_reset(struct ibmvnic_adapter *adapter, > return 0; > } > > - rc = reset_tx_pools(adapter); > - if (rc) > - return rc; > + if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM || > + adapter->wait_for_reset) { > + rc = init_resources(adapter); > + if (rc) > + return rc; > + } else { > + rc = reset_tx_pools(adapter); > + if (rc) > + return rc; > > - rc = reset_rx_pools(adapter); > - if (rc) > - return rc; > + rc = reset_rx_pools(adapter); > + if (rc) > + return rc; > > - if (reset_state == VNIC_CLOSED) > - return 0; > + if (reset_state == VNIC_CLOSED) > + return 0; > + } > } > > rc = __ibmvnic_open(netdev); > @@ -1580,6 +1615,12 @@ static void __ibmvnic_reset(struct work_struct *work) > rwi = get_next_rwi(adapter); > } > > + if (adapter->wait_for_reset) { > + adapter->wait_for_reset = false; > + adapter->reset_done_rc = rc; > + complete(&adapter->reset_done); > + } > + > if (rc) { > netdev_dbg(adapter->netdev, "Reset failed\n"); > free_all_rwi(adapter); > @@ -1759,9 +1800,42 @@ static void ibmvnic_netpoll_controller(struct net_device *dev) > } > #endif > > +static int wait_for_reset(struct ibmvnic_adapter *adapter) > +{ > + adapter->fallback->mtu = adapter->req_mtu; > + adapter->fallback->rx_queues = adapter->req_rx_queues; > + adapter->fallback->tx_queues = adapter->req_tx_queues; > + adapter->fallback->rx_entries = adapter->req_rx_add_entries_per_subcrq; > + adapter->fallback->tx_entries = adapter->req_tx_entries_per_subcrq; > + > + init_completion(&adapter->reset_done); > + ibmvnic_reset(adapter, VNIC_RESET_CHANGE_PARAM); > + adapter->wait_for_reset = true; > + wait_for_completion(&adapter->reset_done); > + > + if (adapter->reset_done_rc) { > + adapter->desired->mtu = adapter->fallback->mtu; > + adapter->desired->rx_queues = adapter->fallback->rx_queues; > + adapter->desired->tx_queues = adapter->fallback->tx_queues; > + adapter->desired->rx_entries = adapter->fallback->rx_entries; > + adapter->desired->tx_entries = adapter->fallback->tx_entries; > + > + init_completion(&adapter->reset_done); > + ibmvnic_reset(adapter, VNIC_RESET_CHANGE_PARAM); > + wait_for_completion(&adapter->reset_done); > + } > + adapter->wait_for_reset = false; > + > + return adapter->reset_done_rc; > +} > + > static int ibmvnic_change_mtu(struct net_device *netdev, int new_mtu) > { > - return -EOPNOTSUPP; > + struct ibmvnic_adapter *adapter = netdev_priv(netdev); > + > + adapter->desired->mtu = new_mtu + ETH_HLEN; Not really relevant to this patch but we do this calculation in many places, perhaps it's time for a macro. > + > + return wait_for_reset(adapter); > } > > static const struct net_device_ops ibmvnic_netdev_ops = { > @@ -1849,6 +1923,27 @@ static void ibmvnic_get_ringparam(struct net_device *netdev, > ring->rx_jumbo_pending = 0; > } > > +static int ibmvnic_set_ringparam(struct net_device *netdev, > + struct ethtool_ringparam *ring) > +{ > + struct ibmvnic_adapter *adapter = netdev_priv(netdev); > + > + if (ring->rx_pending > adapter->max_rx_add_entries_per_subcrq || > + ring->tx_pending > adapter->max_tx_entries_per_subcrq) { > + netdev_err(netdev, "Invalid request.\n"); > + netdev_err(netdev, "Max tx buffers = %llu\n", > + adapter->max_rx_add_entries_per_subcrq); > + netdev_err(netdev, "Max rx buffers = %llu\n", > + adapter->max_tx_entries_per_subcrq); > + return -EINVAL; > + } > + > + adapter->desired->rx_entries = ring->rx_pending; > + adapter->desired->tx_entries = ring->tx_pending; > + > + return wait_for_reset(adapter); > +} > + > static void ibmvnic_get_channels(struct net_device *netdev, > struct ethtool_channels *channels) > { > @@ -1864,6 +1959,17 @@ static void ibmvnic_get_channels(struct net_device *netdev, > channels->combined_count = 0; > } > > +static int ibmvnic_set_channels(struct net_device *netdev, > + struct ethtool_channels *channels) > +{ > + struct ibmvnic_adapter *adapter = netdev_priv(netdev); > + > + adapter->desired->rx_queues = channels->rx_count; > + adapter->desired->tx_queues = channels->tx_count; > + > + return wait_for_reset(adapter); > +} > + > static void ibmvnic_get_strings(struct net_device *dev, u32 stringset, u8 *data) > { > struct ibmvnic_adapter *adapter = netdev_priv(dev); > @@ -1960,7 +2066,9 @@ static void ibmvnic_get_ethtool_stats(struct net_device *dev, > .set_msglevel = ibmvnic_set_msglevel, > .get_link = ibmvnic_get_link, > .get_ringparam = ibmvnic_get_ringparam, > + .set_ringparam = ibmvnic_set_ringparam, > .get_channels = ibmvnic_get_channels, > + .set_channels = ibmvnic_set_channels, > .get_strings = ibmvnic_get_strings, > .get_sset_count = ibmvnic_get_sset_count, > .get_ethtool_stats = ibmvnic_get_ethtool_stats, > @@ -2426,6 +2534,7 @@ static void ibmvnic_send_req_caps(struct ibmvnic_adapter *adapter, int retry) > { > struct device *dev = &adapter->vdev->dev; > union ibmvnic_crq crq; > + int max_entries; > > if (!retry) { > /* Sub-CRQ entries are 32 byte long */ > @@ -2437,21 +2546,60 @@ static void ibmvnic_send_req_caps(struct ibmvnic_adapter *adapter, int retry) > return; > } > > - /* Get the minimum between the queried max and the entries > - * that fit in our PAGE_SIZE > - */ > - adapter->req_tx_entries_per_subcrq = > - adapter->max_tx_entries_per_subcrq > entries_page ? > - entries_page : adapter->max_tx_entries_per_subcrq; > - adapter->req_rx_add_entries_per_subcrq = > - adapter->max_rx_add_entries_per_subcrq > entries_page ? > - entries_page : adapter->max_rx_add_entries_per_subcrq; > - > - adapter->req_tx_queues = adapter->opt_tx_comp_sub_queues; > - adapter->req_rx_queues = adapter->opt_rx_comp_queues; > - adapter->req_rx_add_queues = adapter->max_rx_add_queues; > + if (adapter->desired->mtu) > + adapter->req_mtu = adapter->desired->mtu; > + else > + adapter->req_mtu = adapter->netdev->mtu + ETH_HLEN; > + > + if (!adapter->desired->tx_entries) > + adapter->desired->tx_entries = > + adapter->max_tx_entries_per_subcrq; > + if (!adapter->desired->rx_entries) > + adapter->desired->rx_entries = > + adapter->max_rx_add_entries_per_subcrq; > + > + max_entries = IBMVNIC_MAX_LTB_SIZE / > + (adapter->req_mtu + IBMVNIC_BUFFER_HLEN); > + > + if ((adapter->req_mtu + IBMVNIC_BUFFER_HLEN) * > + adapter->desired->tx_entries > IBMVNIC_MAX_LTB_SIZE) { > + adapter->desired->tx_entries = max_entries; > + } > + > + if ((adapter->req_mtu + IBMVNIC_BUFFER_HLEN) * > + adapter->desired->rx_entries > IBMVNIC_MAX_LTB_SIZE) { > + adapter->desired->rx_entries = max_entries; > + } > > - adapter->req_mtu = adapter->netdev->mtu + ETH_HLEN; > + if (adapter->desired->tx_entries) > + adapter->req_tx_entries_per_subcrq = > + adapter->desired->tx_entries; > + else > + adapter->req_tx_entries_per_subcrq = > + adapter->max_tx_entries_per_subcrq; > + > + if (adapter->desired->rx_entries) > + adapter->req_rx_add_entries_per_subcrq = > + adapter->desired->rx_entries; > + else > + adapter->req_rx_add_entries_per_subcrq = > + adapter->max_rx_add_entries_per_subcrq; > + > + if (adapter->desired->tx_queues) > + adapter->req_tx_queues = > + adapter->desired->tx_queues; > + else > + adapter->req_tx_queues = > + adapter->opt_tx_comp_sub_queues; > + > + if (adapter->desired->rx_queues) > + adapter->req_rx_queues = > + adapter->desired->rx_queues; > + else > + adapter->req_rx_queues = > + adapter->opt_rx_comp_queues; > + > + adapter->req_rx_add_queues = adapter->max_rx_add_queues; > } > > memset(&crq, 0, sizeof(crq)); > @@ -3272,6 +3420,7 @@ static int handle_login_rsp(union ibmvnic_crq *login_rsp_crq, > struct ibmvnic_adapter *adapter) > { > struct device *dev = &adapter->vdev->dev; > + struct net_device *netdev = adapter->netdev; > struct ibmvnic_login_rsp_buffer *login_rsp = adapter->login_rsp_buf; > struct ibmvnic_login_buffer *login = adapter->login_buf; > int i; > @@ -3291,6 +3440,8 @@ static int handle_login_rsp(union ibmvnic_crq *login_rsp_crq, > return 0; > } > > + netdev->mtu = adapter->req_mtu - ETH_HLEN; > + > netdev_dbg(adapter->netdev, "Login Response Buffer:\n"); > for (i = 0; i < (adapter->login_rsp_buf_sz - 1) / 8 + 1; i++) { > netdev_dbg(adapter->netdev, "%016lx\n", > @@ -3846,7 +3997,7 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter) > unsigned long timeout = msecs_to_jiffies(30000); > int rc; > > - if (adapter->resetting) { > + if (adapter->resetting && !adapter->wait_for_reset) { > rc = ibmvnic_reset_crq(adapter); > if (!rc) > rc = vio_enable_interrupts(adapter->vdev); > @@ -3880,7 +4031,7 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter) > return -1; > } > > - if (adapter->resetting) > + if (adapter->resetting && !adapter->wait_for_reset) > rc = reset_sub_crq_queues(adapter); > else > rc = init_sub_crqs(adapter); > @@ -3931,6 +4082,11 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) > adapter->vdev = dev; > adapter->netdev = netdev; > > + adapter->desired = kzalloc(sizeof(*adapter->desired), GFP_KERNEL); > + adapter->desired->mac = kzalloc(sizeof(*adapter->desired->mac), > + GFP_KERNEL); > + adapter->fallback = kzalloc(sizeof(*adapter->fallback), GFP_KERNEL); > + If we are going to allocate the structs always, would it be better to just have them in the adapter struct instead of allocating them? -Nathan > ether_addr_copy(adapter->mac_addr, mac_addr_p); > ether_addr_copy(netdev->dev_addr, adapter->mac_addr); > netdev->irq = dev->irq; > @@ -3956,6 +4112,8 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) > } while (rc == EAGAIN); > > netdev->mtu = adapter->req_mtu - ETH_HLEN; > + netdev->min_mtu = adapter->min_mtu - ETH_HLEN; > + netdev->max_mtu = adapter->max_mtu - ETH_HLEN; > > rc = device_create_file(&dev->dev, &dev_attr_failover); > if (rc) > @@ -3970,6 +4128,9 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) > dev_info(&dev->dev, "ibmvnic registered\n"); > > adapter->state = VNIC_PROBED; > + > + adapter->wait_for_reset = false; > + > return 0; > > ibmvnic_register_fail: > @@ -3993,6 +4154,11 @@ static int ibmvnic_remove(struct vio_dev *dev) > mutex_lock(&adapter->reset_lock); > > release_resources(adapter); > + > + kfree(adapter->desired->mac); > + kfree(adapter->desired); > + kfree(adapter->fallback); > + > release_sub_crqs(adapter); > release_crq_queue(adapter); > > diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h > index 7aa347a..8de998a 100644 > --- a/drivers/net/ethernet/ibm/ibmvnic.h > +++ b/drivers/net/ethernet/ibm/ibmvnic.h > @@ -42,6 +42,9 @@ > #define IBMVNIC_TSO_BUF_SZ 65536 > #define IBMVNIC_TSO_BUFS 64 > > +#define IBMVNIC_MAX_LTB_SIZE ((1 << (MAX_ORDER - 1)) * PAGE_SIZE) > +#define IBMVNIC_BUFFER_HLEN 500 > + > struct ibmvnic_login_buffer { > __be32 len; > __be32 version; > @@ -945,13 +948,23 @@ enum ibmvnic_reset_reason {VNIC_RESET_FAILOVER = 1, > VNIC_RESET_MOBILITY, > VNIC_RESET_FATAL, > VNIC_RESET_NON_FATAL, > - VNIC_RESET_TIMEOUT}; > + VNIC_RESET_TIMEOUT, > + VNIC_RESET_CHANGE_PARAM}; > > struct ibmvnic_rwi { > enum ibmvnic_reset_reason reset_reason; > struct list_head list; > }; > > +struct ibmvnic_tunables { > + u64 rx_queues; > + u64 tx_queues; > + u64 rx_entries; > + u64 tx_entries; > + u64 mtu; > + struct sockaddr *mac; > +}; > + > struct ibmvnic_adapter { > struct vio_dev *vdev; > struct net_device *netdev; > @@ -1012,6 +1025,10 @@ struct ibmvnic_adapter { > struct completion fw_done; > int fw_done_rc; > > + struct completion reset_done; > + int reset_done_rc; > + bool wait_for_reset; > + > /* partner capabilities */ > u64 min_tx_queues; > u64 min_rx_queues; > @@ -1056,4 +1073,7 @@ struct ibmvnic_adapter { > struct work_struct ibmvnic_reset; > bool resetting; > bool napi_enabled, from_passive_init; > + > + struct ibmvnic_tunables *desired; > + struct ibmvnic_tunables *fallback; > }; > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 net-next 2/2] ibmvnic: Fix failover error path for non-fatal resets 2017-10-23 16:30 [PATCH v2 net-next 0/2] ibmvnic: Tunable parameter support John Allen 2017-10-23 16:33 ` [PATCH v2 net-next 1/2] ibmvnic: Update reset infrastructure to support tunable parameters John Allen @ 2017-10-23 16:34 ` John Allen 2017-10-23 23:02 ` kbuild test robot 1 sibling, 1 reply; 5+ messages in thread From: John Allen @ 2017-10-23 16:34 UTC (permalink / raw) To: netdev, Thomas Falcon, Nathan Fontenot For all non-fatal reset conditions, the hypervisor will send a failover when we attempt to initialize the crq and the vnic client is expected to handle that failover instead of the existing non-fatal reset. To handle this, we need to return from init with a return code that indicates that we have hit this case. Signed-off-by: John Allen <jallen@linux.vnet.ibm.com> --- diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index c2c4a5b..fa6b2ad 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1506,7 +1506,7 @@ static int do_reset(struct ibmvnic_adapter *adapter, rc = ibmvnic_init(adapter); if (rc) - return 0; + return IBMVNIC_INIT_FAILED; /* If the adapter was in PROBE state prior to the reset, * exit here. @@ -1609,7 +1609,7 @@ static void __ibmvnic_reset(struct work_struct *work) while (rwi) { rc = do_reset(adapter, rwi, reset_state); kfree(rwi); - if (rc) + if (rc && rc != IBMVNIC_INIT_FAILED) break; rwi = get_next_rwi(adapter); diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index 8de998a..b5aaa6e 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -30,6 +30,8 @@ #define IBMVNIC_DRIVER_VERSION "1.0.1" #define IBMVNIC_INVALID_MAP -1 #define IBMVNIC_STATS_TIMEOUT 1 +#define IBMVNIC_INIT_FAILED 2 + /* basic structures plus 100 2k buffers */ #define IBMVNIC_IO_ENTITLEMENT_DEFAULT 610305 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 net-next 2/2] ibmvnic: Fix failover error path for non-fatal resets 2017-10-23 16:34 ` [PATCH v2 net-next 2/2] ibmvnic: Fix failover error path for non-fatal resets John Allen @ 2017-10-23 23:02 ` kbuild test robot 0 siblings, 0 replies; 5+ messages in thread From: kbuild test robot @ 2017-10-23 23:02 UTC (permalink / raw) To: John Allen; +Cc: kbuild-all, netdev, Thomas Falcon, Nathan Fontenot [-- Attachment #1: Type: text/plain, Size: 4274 bytes --] Hi John, [auto build test WARNING on net-next/master] url: https://github.com/0day-ci/linux/commits/John-Allen/ibmvnic-Update-reset-infrastructure-to-support-tunable-parameters/20171024-062007 config: powerpc-allmodconfig (attached as .config) compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=powerpc Note: it may well be a FALSE warning. FWIW you are at least aware of it now. http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings All warnings (new ones prefixed by >>): drivers/net//ethernet/ibm/ibmvnic.c: In function '__ibmvnic_reset': >> drivers/net//ethernet/ibm/ibmvnic.c:1620:26: warning: 'rc' may be used uninitialized in this function [-Wmaybe-uninitialized] adapter->reset_done_rc = rc; ~~~~~~~~~~~~~~~~~~~~~~~^~~~ vim +/rc +1620 drivers/net//ethernet/ibm/ibmvnic.c ed651a108 Nathan Fontenot 2017-05-03 1592 ed651a108 Nathan Fontenot 2017-05-03 1593 static void __ibmvnic_reset(struct work_struct *work) ed651a108 Nathan Fontenot 2017-05-03 1594 { ed651a108 Nathan Fontenot 2017-05-03 1595 struct ibmvnic_rwi *rwi; ed651a108 Nathan Fontenot 2017-05-03 1596 struct ibmvnic_adapter *adapter; ed651a108 Nathan Fontenot 2017-05-03 1597 struct net_device *netdev; ed651a108 Nathan Fontenot 2017-05-03 1598 u32 reset_state; ed651a108 Nathan Fontenot 2017-05-03 1599 int rc; ed651a108 Nathan Fontenot 2017-05-03 1600 ed651a108 Nathan Fontenot 2017-05-03 1601 adapter = container_of(work, struct ibmvnic_adapter, ibmvnic_reset); ed651a108 Nathan Fontenot 2017-05-03 1602 netdev = adapter->netdev; ed651a108 Nathan Fontenot 2017-05-03 1603 ed651a108 Nathan Fontenot 2017-05-03 1604 mutex_lock(&adapter->reset_lock); ed651a108 Nathan Fontenot 2017-05-03 1605 adapter->resetting = true; ed651a108 Nathan Fontenot 2017-05-03 1606 reset_state = adapter->state; ed651a108 Nathan Fontenot 2017-05-03 1607 ed651a108 Nathan Fontenot 2017-05-03 1608 rwi = get_next_rwi(adapter); ed651a108 Nathan Fontenot 2017-05-03 1609 while (rwi) { ed651a108 Nathan Fontenot 2017-05-03 1610 rc = do_reset(adapter, rwi, reset_state); ed651a108 Nathan Fontenot 2017-05-03 1611 kfree(rwi); 73daa92e4 John Allen 2017-10-23 1612 if (rc && rc != IBMVNIC_INIT_FAILED) ed651a108 Nathan Fontenot 2017-05-03 1613 break; ed651a108 Nathan Fontenot 2017-05-03 1614 ed651a108 Nathan Fontenot 2017-05-03 1615 rwi = get_next_rwi(adapter); ed651a108 Nathan Fontenot 2017-05-03 1616 } ed651a108 Nathan Fontenot 2017-05-03 1617 ce7a66cc8 John Allen 2017-10-23 1618 if (adapter->wait_for_reset) { ce7a66cc8 John Allen 2017-10-23 1619 adapter->wait_for_reset = false; ce7a66cc8 John Allen 2017-10-23 @1620 adapter->reset_done_rc = rc; ce7a66cc8 John Allen 2017-10-23 1621 complete(&adapter->reset_done); ce7a66cc8 John Allen 2017-10-23 1622 } ce7a66cc8 John Allen 2017-10-23 1623 ed651a108 Nathan Fontenot 2017-05-03 1624 if (rc) { d1cf33d93 Nathan Fontenot 2017-08-08 1625 netdev_dbg(adapter->netdev, "Reset failed\n"); ed651a108 Nathan Fontenot 2017-05-03 1626 free_all_rwi(adapter); 6d0af07d5 Wei Yongjun 2017-05-18 1627 mutex_unlock(&adapter->reset_lock); ed651a108 Nathan Fontenot 2017-05-03 1628 return; ed651a108 Nathan Fontenot 2017-05-03 1629 } ed651a108 Nathan Fontenot 2017-05-03 1630 ed651a108 Nathan Fontenot 2017-05-03 1631 adapter->resetting = false; ed651a108 Nathan Fontenot 2017-05-03 1632 mutex_unlock(&adapter->reset_lock); ed651a108 Nathan Fontenot 2017-05-03 1633 } ed651a108 Nathan Fontenot 2017-05-03 1634 :::::: The code at line 1620 was first introduced by commit :::::: ce7a66cc8fceb75d2a95e5e68d21c1889821ff08 ibmvnic: Update reset infrastructure to support tunable parameters :::::: TO: John Allen <jallen@linux.vnet.ibm.com> :::::: CC: 0day robot <fengguang.wu@intel.com> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 55120 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-10-24 16:04 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-23 16:30 [PATCH v2 net-next 0/2] ibmvnic: Tunable parameter support John Allen 2017-10-23 16:33 ` [PATCH v2 net-next 1/2] ibmvnic: Update reset infrastructure to support tunable parameters John Allen 2017-10-24 16:04 ` Nathan Fontenot 2017-10-23 16:34 ` [PATCH v2 net-next 2/2] ibmvnic: Fix failover error path for non-fatal resets John Allen 2017-10-23 23:02 ` kbuild test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox