lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 22 Jan 2022 16:30:55 -0800 From: Dany Madden <drt@...ux.ibm.com> To: Sukadev Bhattiprolu <sukadev@...ux.ibm.com> Cc: netdev@...r.kernel.org, Brian King <brking@...ux.ibm.com>, Rick Lindsley <ricklind@...ux.ibm.com> Subject: Re: [PATCH net 2/4] ibmvnic: init ->running_cap_crqs early On 2022-01-21 18:59, Sukadev Bhattiprolu wrote: > We use ->running_cap_crqs to determine when the ibmvnic_tasklet() > should > send out the next protocol message type. i.e when we get back responses > to all our QUERY_CAPABILITY CRQs we send out REQUEST_CAPABILITY crqs. > Similiary, when we get responses to all the REQUEST_CAPABILITY crqs, we > send out the QUERY_IP_OFFLOAD CRQ. > > We currently increment ->running_cap_crqs as we send out each CRQ and > have the ibmvnic_tasklet() send out the next message type, when this > running_cap_crqs count drops to 0. > > This assumes that all the CRQs of the current type were sent out before > the count drops to 0. However it is possible that we send out say 6 > CRQs, > get preempted and receive all the 6 responses before we send out the > remaining CRQs. This can result in ->running_cap_crqs count dropping to > zero before all messages of the current type were sent and we end up > sending the next protocol message too early. > > Instead initialize the ->running_cap_crqs upfront so the tasklet will > only send the next protocol message after all responses are received. > > Use the cap_reqs local variable to also detect any discrepancy (either > now or in future) in the number of capability requests we actually > send. > > Currently only send_query_cap() is affected by this behavior (of > sending > next message early) since it is called from the worker thread (during > reset) and from application thread (during ->ndo_open()) and they can > be > preempted. send_request_cap() is only called from the tasklet which > processes CRQ responses sequentially, is not be affected. But to > maintain the existing symmtery with send_query_capability() we update > send_request_capability() also. > > Fixes: 249168ad07cd ("ibmvnic: Make CRQ interrupt tasklet wait for all > capabilities crqs") > Signed-off-by: Sukadev Bhattiprolu <sukadev@...ux.ibm.com> Reviewed-by: Dany Madden <drt@...ux.ibm.com> > --- > drivers/net/ethernet/ibm/ibmvnic.c | 106 +++++++++++++++++++---------- > 1 file changed, 71 insertions(+), 35 deletions(-) > > > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c > b/drivers/net/ethernet/ibm/ibmvnic.c > index 9b2d16ad76f1..acd488310bbc 100644 > --- a/drivers/net/ethernet/ibm/ibmvnic.c > +++ b/drivers/net/ethernet/ibm/ibmvnic.c > @@ -3849,11 +3849,25 @@ static void send_request_cap(struct > ibmvnic_adapter *adapter, int retry) > struct device *dev = &adapter->vdev->dev; > union ibmvnic_crq crq; > int max_entries; > + int cap_reqs; > + > + /* We send out 6 or 7 REQUEST_CAPABILITY CRQs below (depending on > + * the PROMISC flag). Initialize this count upfront. When the tasklet > + * receives a response to all of these, it will send the next > protocol > + * message (QUERY_IP_OFFLOAD). > + */ > + if (!(adapter->netdev->flags & IFF_PROMISC) || > + adapter->promisc_supported) > + cap_reqs = 7; > + else > + cap_reqs = 6; > > if (!retry) { > /* Sub-CRQ entries are 32 byte long */ > int entries_page = 4 * PAGE_SIZE / (sizeof(u64) * 4); > > + atomic_set(&adapter->running_cap_crqs, cap_reqs); > + > if (adapter->min_tx_entries_per_subcrq > entries_page || > adapter->min_rx_add_entries_per_subcrq > entries_page) { > dev_err(dev, "Fatal, invalid entries per sub-crq\n"); > @@ -3914,44 +3928,45 @@ static void send_request_cap(struct > ibmvnic_adapter *adapter, int retry) > adapter->opt_rx_comp_queues; > > adapter->req_rx_add_queues = adapter->max_rx_add_queues; > + } else { > + atomic_add(cap_reqs, &adapter->running_cap_crqs); > } > - > memset(&crq, 0, sizeof(crq)); > crq.request_capability.first = IBMVNIC_CRQ_CMD; > crq.request_capability.cmd = REQUEST_CAPABILITY; > > crq.request_capability.capability = cpu_to_be16(REQ_TX_QUEUES); > crq.request_capability.number = cpu_to_be64(adapter->req_tx_queues); > - atomic_inc(&adapter->running_cap_crqs); > + cap_reqs--; > ibmvnic_send_crq(adapter, &crq); > > crq.request_capability.capability = cpu_to_be16(REQ_RX_QUEUES); > crq.request_capability.number = cpu_to_be64(adapter->req_rx_queues); > - atomic_inc(&adapter->running_cap_crqs); > + cap_reqs--; > ibmvnic_send_crq(adapter, &crq); > > crq.request_capability.capability = cpu_to_be16(REQ_RX_ADD_QUEUES); > crq.request_capability.number = > cpu_to_be64(adapter->req_rx_add_queues); > - atomic_inc(&adapter->running_cap_crqs); > + cap_reqs--; > ibmvnic_send_crq(adapter, &crq); > > crq.request_capability.capability = > cpu_to_be16(REQ_TX_ENTRIES_PER_SUBCRQ); > crq.request_capability.number = > cpu_to_be64(adapter->req_tx_entries_per_subcrq); > - atomic_inc(&adapter->running_cap_crqs); > + cap_reqs--; > ibmvnic_send_crq(adapter, &crq); > > crq.request_capability.capability = > cpu_to_be16(REQ_RX_ADD_ENTRIES_PER_SUBCRQ); > crq.request_capability.number = > cpu_to_be64(adapter->req_rx_add_entries_per_subcrq); > - atomic_inc(&adapter->running_cap_crqs); > + cap_reqs--; > ibmvnic_send_crq(adapter, &crq); > > crq.request_capability.capability = cpu_to_be16(REQ_MTU); > crq.request_capability.number = cpu_to_be64(adapter->req_mtu); > - atomic_inc(&adapter->running_cap_crqs); > + cap_reqs--; > ibmvnic_send_crq(adapter, &crq); > > if (adapter->netdev->flags & IFF_PROMISC) { > @@ -3959,16 +3974,21 @@ static void send_request_cap(struct > ibmvnic_adapter *adapter, int retry) > crq.request_capability.capability = > cpu_to_be16(PROMISC_REQUESTED); > crq.request_capability.number = cpu_to_be64(1); > - atomic_inc(&adapter->running_cap_crqs); > + cap_reqs--; > ibmvnic_send_crq(adapter, &crq); > } > } else { > crq.request_capability.capability = > cpu_to_be16(PROMISC_REQUESTED); > crq.request_capability.number = cpu_to_be64(0); > - atomic_inc(&adapter->running_cap_crqs); > + cap_reqs--; > ibmvnic_send_crq(adapter, &crq); > } > + > + /* Keep at end to catch any discrepancy between expected and actual > + * CRQs sent. > + */ > + WARN_ON(cap_reqs != 0); > } > > static int pending_scrq(struct ibmvnic_adapter *adapter, > @@ -4362,118 +4382,132 @@ static void send_query_map(struct > ibmvnic_adapter *adapter) > static void send_query_cap(struct ibmvnic_adapter *adapter) > { > union ibmvnic_crq crq; > + int cap_reqs; > + > + /* We send out 25 QUERY_CAPABILITY CRQs below. Initialize this count > + * upfront. When the tasklet receives a response to all of these, it > + * can send out the next protocol messaage (REQUEST_CAPABILITY). > + */ > + cap_reqs = 25; > + > + atomic_set(&adapter->running_cap_crqs, cap_reqs); > > - atomic_set(&adapter->running_cap_crqs, 0); > memset(&crq, 0, sizeof(crq)); > crq.query_capability.first = IBMVNIC_CRQ_CMD; > crq.query_capability.cmd = QUERY_CAPABILITY; > > crq.query_capability.capability = cpu_to_be16(MIN_TX_QUEUES); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(MIN_RX_QUEUES); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(MIN_RX_ADD_QUEUES); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(MAX_TX_QUEUES); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(MAX_RX_QUEUES); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(MAX_RX_ADD_QUEUES); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = > cpu_to_be16(MIN_TX_ENTRIES_PER_SUBCRQ); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = > cpu_to_be16(MIN_RX_ADD_ENTRIES_PER_SUBCRQ); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = > cpu_to_be16(MAX_TX_ENTRIES_PER_SUBCRQ); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = > cpu_to_be16(MAX_RX_ADD_ENTRIES_PER_SUBCRQ); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(TCP_IP_OFFLOAD); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(PROMISC_SUPPORTED); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(MIN_MTU); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(MAX_MTU); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(MAX_MULTICAST_FILTERS); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(VLAN_HEADER_INSERTION); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = > cpu_to_be16(RX_VLAN_HEADER_INSERTION); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(MAX_TX_SG_ENTRIES); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(RX_SG_SUPPORTED); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = > cpu_to_be16(OPT_TX_COMP_SUB_QUEUES); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(OPT_RX_COMP_QUEUES); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = > cpu_to_be16(OPT_RX_BUFADD_Q_PER_RX_COMP_Q); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = > cpu_to_be16(OPT_TX_ENTRIES_PER_SUBCRQ); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = > cpu_to_be16(OPT_RXBA_ENTRIES_PER_SUBCRQ); > - atomic_inc(&adapter->running_cap_crqs); > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > > crq.query_capability.capability = cpu_to_be16(TX_RX_DESC_REQ); > - atomic_inc(&adapter->running_cap_crqs); > + > ibmvnic_send_crq(adapter, &crq); > + cap_reqs--; > + > + /* Keep at end to catch any discrepancy between expected and actual > + * CRQs sent. > + */ > + WARN_ON(cap_reqs != 0); > } > > static void send_query_ip_offload(struct ibmvnic_adapter *adapter) > @@ -4777,6 +4811,8 @@ static void handle_request_cap_rsp(union > ibmvnic_crq *crq, > char *name; > > atomic_dec(&adapter->running_cap_crqs); > + netdev_dbg(adapter->netdev, "Outstanding request-caps: %d\n", > + atomic_read(&adapter->running_cap_crqs)); > switch (be16_to_cpu(crq->request_capability_rsp.capability)) { > case REQ_TX_QUEUES: > req_value = &adapter->req_tx_queues;
Powered by blists - more mailing lists