On Tue, 20 Oct 2020 17:07:38 -0500 Lijun Pan wrote:
> > Please read my reply carefully.
> >
> > What's the call path that leads to the address being wrong? If you set
> > the address via ifconfig it will call ibmvnic_set_mac() of the driver.
> > ibmvnic_set_mac() does the copy.
> >
> > But it doesn't validate the address, which it should.
>
> Sorry about that. The mac addr validation is performed on vios side when it
> receives the
> change request. That’s why there is no mac validation code in vnic driver.
The problem is that there is validation in the driver:
static int __ibmvnic_set_mac(struct net_device *netdev, u8 *dev_addr)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
union ibmvnic_crq crq;
int rc;
if (!is_valid_ether_addr(dev_addr)) {
rc = -EADDRNOTAVAIL;
goto err;
}
And ibmvnic_set_mac() does this:
ether_addr_copy(adapter->mac_addr, addr->sa_data);
if (adapter->state != VNIC_PROBED)
rc = __ibmvnic_set_mac(netdev, addr->sa_data);
So if state == VNIC_PROBED, the user can assign an invalid address to
adapter->mac_addr, and ibmvnic_set_mac() will still return 0.
That's a separate issue, for a separate patch, though, so you can send
a v2 of this patch regardless.
> In handle_change_mac_rsp(), &crq->change_mac_addr_rsp.mac_addr[0]
> contains the current valid mac address, which may be different than the
> requested one,
> that is &crq->change_mac_addr.mac_addr[0].
> crq->change_mac_addr.mac_addr is the requested one.
> crq->change_mac_addr_rsp.mac_addr is the returned valid one.
>
> Hope the above answers your doubt.
Oh! The address in reply can be different than the requested one?
Please add a comment stating that in the code.
