| Message ID | 20201123235841.6515-1-drt@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] ibmvnic: process HMC disable command | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 19 this patch: 19 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 85 exceeds 80 columns WARNING: line length of 99 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Mon, 23 Nov 2020 18:58:41 -0500 Dany Madden wrote: > Currently ibmvnic does not support the "Disable vNIC" command from > the Hardware Management Console. The HMC uses this command to disconnect > the adapter from the network if the adapter is misbehaving or sending > malicious traffic. The effect of this command is equivalent to setting > the link to the "down" state on the linux client. > > Enable support in ibmvnic driver for the Disable vNIC command. > > Signed-off-by: Dany Madden <drt@linux.ibm.com> It seems that (a) user looking at the system where NIC was disabled has no idea why netdev is not working even tho it's UP, and (b) AFAICT nothing prevents the user from bringing the device down and back up again. You said this is to disable misbehaving and/or sending malicious vnic, obviously the guest can ignore the command so it's not very dependable, anyway. Would it not be sufficient to mark the carrier state as down to cut the vnic off?
On 2020-12-02 12:02, drt wrote: > On 2020-11-30 10:19, drt wrote: >> On 2020-11-25 15:55, drt wrote: >>> On 2020-11-25 13:08, Jakub Kicinski wrote: >>>> On Mon, 23 Nov 2020 18:58:41 -0500 Dany Madden wrote: >>>>> Currently ibmvnic does not support the "Disable vNIC" command from >>>>> the Hardware Management Console. The HMC uses this command to >>>>> disconnect >>>>> the adapter from the network if the adapter is misbehaving or >>>>> sending >>>>> malicious traffic. The effect of this command is equivalent to >>>>> setting >>>>> the link to the "down" state on the linux client. >>>>> >>>>> Enable support in ibmvnic driver for the Disable vNIC command. >>>>> >>>>> Signed-off-by: Dany Madden <drt@linux.ibm.com> >>>> >>>> It seems that (a) user looking at the system where NIC was disabled >>>> has >>>> no idea why netdev is not working even tho it's UP, and (b) AFAICT >>>> nothing prevents the user from bringing the device down and back up >>>> again. >>> >>> User would see the interface as DOWN. ibmvnic_close() requests the >>> vnicserver to do a link down. The vnicserver responds with a link >>> state indication CRQ message with logical link down, client would >>> then >>> do netif_carrier_off(). >>> >>> You are correct, nothing is preventing the user from bringing the >>> device back online. >>> >>>> >>>> You said this is to disable misbehaving and/or sending malicious >>>> vnic, >>>> obviously the guest can ignore the command so it's not very >>>> dependable, >>>> anyway. >>> >>> Without this patch, ibmvnic would ignore the command. With this >>> patch, >>> it will handle the disable command from the HMC. If the guest insists >>> on being bad, the HMC does have the ability to remove vnic adapter >>> from the guest. >>> >>>> >>>> Would it not be sufficient to mark the carrier state as down to cut >>>> the >>>> vnic off? >>> Essentially, this is what ibmvnic_disable does. >> >> Hello Jakub, did I address your concern? If not, please let me know. > > Hello Jakub, > > I am pulling this patch. Suka pointed out that rwi lock is not being > held when it walks the rwi_list, also the reset bit is incorrectly > checked. We will send a v3. > > Apologize for any inconvenient. It appears that my email is not showing up in the mailing archive because of email aliases. I hope this is going thru. Please do not commit this patch. > > thanks you! > Dany >> >> Thanks!
On Wed, 02 Dec 2020 15:50:58 -0800 Dany Madden wrote: > On 2020-12-02 12:02, drt wrote: > > On 2020-11-30 10:19, drt wrote: > >> On 2020-11-25 15:55, drt wrote: > >>> On 2020-11-25 13:08, Jakub Kicinski wrote: > >>>> On Mon, 23 Nov 2020 18:58:41 -0500 Dany Madden wrote: > >>>>> Currently ibmvnic does not support the "Disable vNIC" command from > >>>>> the Hardware Management Console. The HMC uses this command to > >>>>> disconnect > >>>>> the adapter from the network if the adapter is misbehaving or > >>>>> sending > >>>>> malicious traffic. The effect of this command is equivalent to > >>>>> setting > >>>>> the link to the "down" state on the linux client. > >>>>> > >>>>> Enable support in ibmvnic driver for the Disable vNIC command. > >>>>> > >>>>> Signed-off-by: Dany Madden <drt@linux.ibm.com> > >>>> > >>>> It seems that (a) user looking at the system where NIC was disabled > >>>> has > >>>> no idea why netdev is not working even tho it's UP, and (b) AFAICT > >>>> nothing prevents the user from bringing the device down and back up > >>>> again. > >>> > >>> User would see the interface as DOWN. ibmvnic_close() requests the > >>> vnicserver to do a link down. The vnicserver responds with a link > >>> state indication CRQ message with logical link down, client would > >>> then > >>> do netif_carrier_off(). > >>> > >>> You are correct, nothing is preventing the user from bringing the > >>> device back online. > >>> > >>>> > >>>> You said this is to disable misbehaving and/or sending malicious > >>>> vnic, > >>>> obviously the guest can ignore the command so it's not very > >>>> dependable, > >>>> anyway. > >>> > >>> Without this patch, ibmvnic would ignore the command. With this > >>> patch, > >>> it will handle the disable command from the HMC. If the guest insists > >>> on being bad, the HMC does have the ability to remove vnic adapter > >>> from the guest. > >>> > >>>> > >>>> Would it not be sufficient to mark the carrier state as down to cut > >>>> the > >>>> vnic off? > >>> Essentially, this is what ibmvnic_disable does. > >> > >> Hello Jakub, did I address your concern? If not, please let me know. > > > > Hello Jakub, > > > > I am pulling this patch. Suka pointed out that rwi lock is not being > > held when it walks the rwi_list, also the reset bit is incorrectly > > checked. We will send a v3. > > > > Apologize for any inconvenient. > > It appears that my email is not showing up in the mailing archive > because of email aliases. I hope this is going thru. > > Please do not commit this patch. FWIW you can check the status of active patches in patchwork: https://patchwork.kernel.org/project/netdevbpf/list/ This one has already been made inactive so it won't be applied in its current form.
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 63b39744a07a..47446e5f8ec5 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -109,6 +109,8 @@ static void release_crq_queue(struct ibmvnic_adapter *); static int __ibmvnic_set_mac(struct net_device *, u8 *); static int init_crq_queue(struct ibmvnic_adapter *adapter); static int send_query_phys_parms(struct ibmvnic_adapter *adapter); +static void ibmvnic_disable(struct ibmvnic_adapter *adapter); +static int ibmvnic_close(struct net_device *netdev); struct ibmvnic_stat { char name[ETH_GSTRING_LEN]; @@ -1207,6 +1209,42 @@ static int ibmvnic_open(struct net_device *netdev) return rc; } +static void ibmvnic_disable(struct ibmvnic_adapter *adapter) +{ + struct list_head *entry, *tmp_entry; + struct net_device *netdev = adapter->netdev; + int rc = 0; + + /* cancel all pending resets in the queue */ + if (!list_empty(&adapter->rwi_list)) { + list_for_each_safe(entry, tmp_entry, &adapter->rwi_list) + list_del(entry); + } + + /* wait for current reset to finish */ + flush_work(&adapter->ibmvnic_reset); + flush_delayed_work(&adapter->ibmvnic_delayed_reset); + + if (test_bit(0, &adapter->resetting) || + adapter->state == VNIC_PROBED || + adapter->state == VNIC_OPEN || + adapter->state == VNIC_OPENING) { + rc = ibmvnic_close(netdev); + /* Expect -EINVAL when crq is no longer active. Set link down + * would fail. + */ + if (rc && rc != -EINVAL) { + netdev_err(netdev, "Failed to disable adapter, rc=%d\n", rc); + return; + } + } else { + netdev_dbg(netdev, "Disable adapter request ignored (state=%d)\n", adapter->state); + return; + } + + netdev_dbg(netdev, "Adapter disabled\n"); +} + static void clean_rx_pools(struct ibmvnic_adapter *adapter) { struct ibmvnic_rx_pool *rx_pool; @@ -4825,6 +4863,8 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq, } else if (gen_crq->cmd == IBMVNIC_DEVICE_FAILOVER) { dev_info(dev, "Backing device failover detected\n"); adapter->failover_pending = true; + } else if (gen_crq->cmd == IBMVNIC_DEVICE_DISABLE) { + ibmvnic_disable(adapter); } else { /* The adapter lost the connection */ dev_err(dev, "Virtual Adapter failed (rc=%d)\n", diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index b21092f5f9c1..d15866cbc2a6 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -834,10 +834,11 @@ enum ibmvnic_crq_type { IBMVNIC_CRQ_XPORT_EVENT = 0xFF, }; -enum ibmvfc_crq_format { +enum ibmvnic_crq_format { IBMVNIC_CRQ_INIT = 0x01, IBMVNIC_CRQ_INIT_COMPLETE = 0x02, IBMVNIC_PARTITION_MIGRATED = 0x06, + IBMVNIC_DEVICE_DISABLE = 0x07, IBMVNIC_DEVICE_FAILOVER = 0x08, };
Currently ibmvnic does not support the "Disable vNIC" command from the Hardware Management Console. The HMC uses this command to disconnect the adapter from the network if the adapter is misbehaving or sending malicious traffic. The effect of this command is equivalent to setting the link to the "down" state on the linux client. Enable support in ibmvnic driver for the Disable vNIC command. Signed-off-by: Dany Madden <drt@linux.ibm.com> --- V2 changes based on Jakub Kicinski's feedback: - Broke from "[PATCH net 00/15] ibmvnic: assorted bug fixes" sent by Lijun Pan. - Expanded on the description - Submitting to net-next --- drivers/net/ethernet/ibm/ibmvnic.c | 40 ++++++++++++++++++++++++++++++ drivers/net/ethernet/ibm/ibmvnic.h | 3 ++- 2 files changed, 42 insertions(+), 1 deletion(-)