Misunderstanding of spec?

Misunderstanding of spec?

[William Kucharski]

I noticed that under certain circumstances, GID 0 for some interfaces is reported as all 0s, or empty.

This seems to be correlated with code in drivers/infiniband/core/roce_gid_mgmt.c in the routine
ndev_event_link(), which does this:
	
	/*
	 * When a lower netdev is linked to its upper bonding
	 * netdev, delete lower slave netdev's default GIDs.
	 */
	cmds[0] = bonding_default_del_cmd;
	cmds[0].ndev = event_ndev;
	cmds[0].filter_ndev = changeupper_info->upper_dev;

bonding_default_del_cmd will result in a call to del_default_gids().

However, given version 1.2.1 of the IB spec, looking at page 145, line 20:

4.1.1 GID USAGE AND PROPERTIES

1) Each endport shall be assigned at least one unicast GID. The first unicast GID assigned shall be
  created using the manufacturer assigned EUI-64 identifier. This GID is referred to as GID index 0
  and is formed by techniques 3(a) or 3(b) described below.

2) The default GID prefix shall be (0xFE80::0). A packet using the default GID prefix and either a
  manufacturer assigned or SM assigned EUI-64 must always be accepted by an endnode. A packet
  containing a GRH with a destination GID with this prefix must never be forwarded by a router,
  i.e. it is restricted to the local subnet.

3) A unicast GID shall be created using one or more of the following mechanisms:

   a) Concatenation of the default GID prefix with the manufacturer as
      signed EUI-64 identifier associated with an endport. This GID is
      referred to as the default GID.

   b) Concatenation of a subnet manager assigned 64-bit GID prefix and the
      manufacturer assigned EUI-64 identifier associated with an endport.

   c) Assignment of a GID by the subnet manager. The subnet manager creates a
      GID by concatenating the GID prefix (default or assigned) with a set of
      locally assigned EUI-64 values (at GID index 1 or above).

      Each endport must be assigned at least one unicast GID using (a). Additional
      GIDs may be assigned using (b) and/or (c). Note: A subnet 2 shall only have
      one assigned GID prefix (non default) at any given time.

make_default_gid()and add default_gids() seem to have that all taken care of.

What concerns me is the code that removes GID index 0, as page 436, line 35, states:

C10-2: For each GID Table, the first entry in the table shall contain the read-only invariant value of GID index 0.

So is it actually OK to remove GID 0 when removing default GIDs via del_default_gids(), or should it be preserving GID index 0 at all times?

This is because it appears a call to rdma_query_gid() (as in ib_find_gid()) will return -EINVAL for a table if GID 0 has been deleted.

Am I misreading the spec, or is there a bug here?

Thanks,
    William Kucharski

Re: Misunderstanding of spec?

[William Kucharski]

True, but I would expect anything specified as a requirement in the older version should still be true in the latest version if for reasons of backward compatibility.

Does the newer spec now allow for the clearing of formerly read-only GID index 0?

Unless I am misunderstanding the code, which is quite possible, it still seems not to expect an empty GID index 0 in say rdma_query_gid().

Thanks in advance.

> On Aug 14, 2021, at 03:33, Rupert Dance - SFI <rsdance@soft-forge.com> wrote:
> 
> William,
> 
> This is an extremely old version of the spec and much has changed since this
> was released in July 2007. The current version is 1.5 which was released on
> August 6th 2021.
> 
> https://www.infinibandta.org/ibta-specification/
> 
> Thank you,
> 
> Rupert Dance
> IBTA CIWG Chair
> IBTA LWG Co-Chair
> Software Forge
> 2 Greenleaf Woods Dr #301
> Portsmouth, NH 03801
> Phone: 603-319-8486
> Website: www.soft-forge.com
> 
> 
> -----Original Message-----
> From: William Kucharski <william.kucharski@oracle.com> 
> Sent: Friday, August 13, 2021 11:04 PM
> To: linux-rdma@vger.kernel.org
> Subject: Misunderstanding of spec?
> 
> I noticed that under certain circumstances, GID 0 for some interfaces is
> reported as all 0s, or empty.
> 
> This seems to be correlated with code in
> drivers/infiniband/core/roce_gid_mgmt.c in the routine ndev_event_link(),
> which does this:
> 
>   /*
>    * When a lower netdev is linked to its upper bonding
>    * netdev, delete lower slave netdev's default GIDs.
>    */
>   cmds[0] = bonding_default_del_cmd;
>   cmds[0].ndev = event_ndev;
>   cmds[0].filter_ndev = changeupper_info->upper_dev;
> 
> bonding_default_del_cmd will result in a call to del_default_gids().
> 
> However, given version 1.2.1 of the IB spec, looking at page 145, line 20:
> 
> 4.1.1 GID USAGE AND PROPERTIES
> 
> 1) Each endport shall be assigned at least one unicast GID. The first
> unicast GID assigned shall be
> created using the manufacturer assigned EUI-64 identifier. This GID is
> referred to as GID index 0
> and is formed by techniques 3(a) or 3(b) described below.
> 
> 2) The default GID prefix shall be (0xFE80::0). A packet using the default
> GID prefix and either a
> manufacturer assigned or SM assigned EUI-64 must always be accepted by an
> endnode. A packet
> containing a GRH with a destination GID with this prefix must never be
> forwarded by a router,
> i.e. it is restricted to the local subnet.
> 
> 3) A unicast GID shall be created using one or more of the following
> mechanisms:
> 
>  a) Concatenation of the default GID prefix with the manufacturer as
>     signed EUI-64 identifier associated with an endport. This GID is
>     referred to as the default GID.
> 
>  b) Concatenation of a subnet manager assigned 64-bit GID prefix and the
>     manufacturer assigned EUI-64 identifier associated with an endport.
> 
>  c) Assignment of a GID by the subnet manager. The subnet manager creates
> a
>     GID by concatenating the GID prefix (default or assigned) with a set
> of
>     locally assigned EUI-64 values (at GID index 1 or above).
> 
>     Each endport must be assigned at least one unicast GID using (a).
> Additional
>     GIDs may be assigned using (b) and/or (c). Note: A subnet 2 shall only
> have
>     one assigned GID prefix (non default) at any given time.
> 
> make_default_gid()and add default_gids() seem to have that all taken care
> of.
> 
> What concerns me is the code that removes GID index 0, as page 436, line 35,
> states:
> 
> C10-2: For each GID Table, the first entry in the table shall contain the
> read-only invariant value of GID index 0.
> 
> So is it actually OK to remove GID 0 when removing default GIDs via
> del_default_gids(), or should it be preserving GID index 0 at all times?
> 
> This is because it appears a call to rdma_query_gid() (as in ib_find_gid())
> will return -EINVAL for a table if GID 0 has been deleted.
> 
> Am I misreading the spec, or is there a bug here?
> 
> Thanks,
>   William Kucharski
> 
> 

Re: Misunderstanding of spec?

[Jason Gunthorpe]

On Sat, Aug 14, 2021 at 03:03:52AM +0000, William Kucharski wrote:
> I noticed that under certain circumstances, GID 0 for some interfaces is reported as all 0s, or empty.
> 
> This seems to be correlated with code in drivers/infiniband/core/roce_gid_mgmt.c in the routine
> ndev_event_link(), which does this:
> 	
> 	/*
> 	 * When a lower netdev is linked to its upper bonding
> 	 * netdev, delete lower slave netdev's default GIDs.
> 	 */
> 	cmds[0] = bonding_default_del_cmd;
> 	cmds[0].ndev = event_ndev;
> 	cmds[0].filter_ndev = changeupper_info->upper_dev;
> 
> bonding_default_del_cmd will result in a call to del_default_gids().
> 
> However, given version 1.2.1 of the IB spec, looking at page 145, line 20:
> 
> 4.1.1 GID USAGE AND PROPERTIES
> 
> 1) Each endport shall be assigned at least one unicast GID. The first unicast GID assigned shall be
>   created using the manufacturer assigned EUI-64 identifier. This GID is referred to as GID index 0
>   and is formed by techniques 3(a) or 3(b) described below.
> 
> 2) The default GID prefix shall be (0xFE80::0). A packet using the default GID prefix and either a
>   manufacturer assigned or SM assigned EUI-64 must always be accepted by an endnode. A packet
>   containing a GRH with a destination GID with this prefix must never be forwarded by a router,
>   i.e. it is restricted to the local subnet.
> 
> 3) A unicast GID shall be created using one or more of the following mechanisms:
> 
>    a) Concatenation of the default GID prefix with the manufacturer as
>       signed EUI-64 identifier associated with an endport. This GID is
>       referred to as the default GID.
> 
>    b) Concatenation of a subnet manager assigned 64-bit GID prefix and the
>       manufacturer assigned EUI-64 identifier associated with an endport.
> 
>    c) Assignment of a GID by the subnet manager. The subnet manager creates a
>       GID by concatenating the GID prefix (default or assigned) with a set of
>       locally assigned EUI-64 values (at GID index 1 or above).
> 
>       Each endport must be assigned at least one unicast GID using (a). Additional
>       GIDs may be assigned using (b) and/or (c). Note: A subnet 2 shall only have
>       one assigned GID prefix (non default) at any given time.
> 
> make_default_gid()and add default_gids() seem to have that all taken care of.
> 
> What concerns me is the code that removes GID index 0, as page 436, line 35, states:
> 
> C10-2: For each GID Table, the first entry in the table shall contain the read-only invariant value of GID index 0.
> 
> So is it actually OK to remove GID 0 when removing default GIDs via del_default_gids(), or should it be preserving GID index 0 at all times?
> 
> This is because it appears a call to rdma_query_gid() (as in ib_find_gid()) will return -EINVAL for a table if GID 0 has been deleted.
> 
> Am I misreading the spec, or is there a bug here?

This language can not apply to rocev2 which does not have a default
GID at all.

Jason