[ofw] RE: [PATCH] Kill ATS registration in IPoIB

Smith, Stan stan.smith at intel.com
Thu Jul 3 09:04:31 PDT 2008


Tzachi Dar wrote:
> It seems to me that DAPL is still using the ATS registrations.
> 
> Do we all agree that ATS can be removed from the code?

Not until after WinOF 2.0 release.
Thank you.

> 
> Thanks
> Tzachi
> 
>> -----Original Message-----
>> From: ofw-bounces at lists.openfabrics.org
>> [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Fab Tillier
>> Sent: Wednesday, July 02, 2008 10:28 PM
>> To: ofw at lists.openfabrics.org
>> Subject: [ofw] RE: [PATCH] Kill ATS registration in IPoIB
>> 
>> Oops, I got a bit out of order here, since the NetworkDirect
>> proxy code still uses ATS...  Patch to fix the ND Proxy coming up...
>> 
>> -Fab
>> 
>>> -----Original Message-----
>>> From: ofw-bounces at lists.openfabrics.org [mailto:ofw-
>>> bounces at lists.openfabrics.org] On Behalf Of Fab Tillier
>>> Sent: Wednesday, July 02, 2008 12:18 PM
>>> To: ofw at lists.openfabrics.org
>>> Subject: [ofw] [PATCH] Kill ATS registration in IPoIB
>>> 
>>> This patch deletes ATS registration in IPoIB.  ATS was not reliable
>>> anyway, because not all SMs supported it - Cisco's SM did not.
>>> 
>>> ATS use should be replaced with IBAT.
>>> 
>>> Signed-off-by: Fab Tillier <ftillier at microsoft.com>
>>> 
>>> diff -up -r -X trunk\docs\dontdiff.txt -I \$Id:
>>> old\ulp\ipoib\kernel\ipoib_adapter.c
>>> trunk\ulp\ipoib\kernel\ipoib_adapter.c
>>> --- old\ulp\ipoib\kernel\ipoib_adapter.c        Mon Jun 30 11:09:26
>>> 2008 +++ trunk\ulp\ipoib\kernel\ipoib_adapter.c      Wed Jul 02
>>> 12:12:44 2008 @@ -993,9 +993,6 @@ ipoib_set_active(
>>>         switch( old_state )
>>>         {
>>>         case IB_PNP_PORT_ADD:
>>> -               ipoib_reg_addrs( p_adapter );
>>> -               /* Fall through. */
>>> -
>>>         case IB_PNP_PORT_REMOVE:
>>>                 ipoib_resume_oids( p_adapter );
>>>                 break;
>>> @@ -1009,8 +1006,6 @@ ipoib_set_active(
>>>                 }
>>> 
>>>                 /* Register all existing addresses. */
>>> -               ipoib_reg_addrs( p_adapter );
>>> -
>>>                 ipoib_resume_oids( p_adapter );
>>> 
>>>                 /*
>>> diff -up -r -X trunk\docs\dontdiff.txt -I \$Id:
>>> old\ulp\ipoib\kernel\ipoib_adapter.h
>>> trunk\ulp\ipoib\kernel\ipoib_adapter.h
>>> --- old\ulp\ipoib\kernel\ipoib_adapter.h        Mon Jun 30 11:09:26
>>> 2008 +++ trunk\ulp\ipoib\kernel\ipoib_adapter.h      Wed Jul 02
>>> 12:12:44 2008 @@ -287,25 +287,8 @@ typedef struct _ipoib_adapter 
>>> *********/ 
>>> 
>>> 
>>> -typedef struct _ats_reg
>>> -{
>>> -       ipoib_adapter_t         *p_adapter;
>>> -       ib_reg_svc_handle_t     h_reg_svc;
>>> -
>>> -}      ats_reg_t;
>>> -/*
>>> -* FIELDS
>>> -*      p_adapter
>>> -*              Pointer to the adapter to which this address is
>>> assigned. -*
>>> -*      h_reg_svc
>>> -*              Service registration handle.
>>> -*********/
>>> -
>>> -
>>>  typedef struct _net_address_item
>>>  {
>>> -       ats_reg_t                       *p_reg;
>>>         union _net_address_item_address
>>>         {
>>>                 ULONG                   as_ulong;
>>> @@ -412,14 +395,6 @@ ipoib_set_inactive(
>>> 
>>>  ib_api_status_t
>>>  ipoib_reset_adapter(
>>> -       IN                              ipoib_adapter_t* const
>>> p_adapter ); -
>>> -void
>>> -ipoib_reg_addrs(
>>> -       IN                              ipoib_adapter_t* const
>>> p_adapter ); -
>>> -void
>>> -ipoib_dereg_addrs(
>>>         IN                              ipoib_adapter_t* const
>>> p_adapter ); 
>>> 
>>>  #endif /* _IPOIB_ADAPTER_H_ */
>>> diff -up -r -X trunk\docs\dontdiff.txt -I \$Id:
>>> old\ulp\ipoib\kernel\ipoib_driver.c
>>> trunk\ulp\ipoib\kernel\ipoib_driver.c
>>> --- old\ulp\ipoib\kernel\ipoib_driver.c Thu Jun 26 20:35:13 2008
>>> +++ trunk\ulp\ipoib\kernel\ipoib_driver.c       Wed Jul 02 12:14:01
>>> 2008 @@ -213,14 +213,6 @@ __ipoib_get_tcp_task_offload(
>>>         IN                              ipoib_adapter_t*
>>> p_adapter,
>>>         IN                              pending_oid_t* const
>>> p_oid_info ); 
>>> 
>>> -static void
>>> -__ipoib_ats_reg_cb(
>>> -       IN                              ib_reg_svc_rec_t
>>> *p_reg_svc_rec ); -
>>> -static void
>>> -__ipoib_ats_dereg_cb(
>>> -       IN                              void
>>> *context );
>>> -
>>>  static NTSTATUS
>>>  __ipoib_read_registry(
>>>         IN                              UNICODE_STRING* const
>>> p_registry_path ); @@ -1632,20 +1624,9 @@ ipoib_set_info(
>>>                                 {
>>>                                         cl_qlist_insert_tail(
>>> 
>> &g_ipoib.adapter_list,
>>> &p_adapter->entry );
>>> -
>>> -                                       /*
>>> -                                        * Filter was zero, now
>>> non- zero.  Register IP addresses 
>>> -                                        * with SA.
>>> -                                        */
>>> -                                       ipoib_reg_addrs( p_adapter
>>>                                 ); }
>>>                                 else if( p_adapter->packet_filter
>>>                                 && !(*(uint32_t*)info_buf) ) {
>>> -                                       /*
>>> -                                        * Filter was non-zero, now
>>> zero.  Deregister IP addresses.
>>> -                                        */
>>> -                                       ipoib_dereg_addrs(
>>>                                         p_adapter ); - ASSERT(
>>>                                         cl_qlist_count(
>>> &g_ipoib.adapter_list ) ); cl_qlist_remove_item( 
>>> 
>> &g_ipoib.adapter_list,
>>> &p_adapter->entry );
>>> @@ -1977,17 +1958,9 @@ ipoib_resume_oids(
>>>                         {
>>>                                 cl_qlist_insert_tail(
>>>                                         &g_ipoib.adapter_list,
>>> &p_adapter->entry ); 
>>> -                               /*
>>> -                                * Filter was zero, now non-zero.
>>> Register IP addresses 
>>> -                                * with SA.
>>> -                                */
>>> -                               ipoib_reg_addrs( p_adapter );      
>>>                         } else if( p_adapter->packet_filter &&
>>>                         !(*(PULONG)set_oid.p_buf) ) {
>>> -                               /* Filter was non-zero, now zero.
>>> Deregister IP addresses. */ 
>>> -                               ipoib_dereg_addrs( p_adapter ); -
>>>                                 ASSERT( cl_qlist_count(
>>>                                 &g_ipoib.adapter_list ) );
>>>                                         cl_qlist_remove_item(
>>> &g_ipoib.adapter_list, &p_adapter->entry ); @@ -2189,19 +2162,6 @@
>>>                                         __ipoib_set_net_addr(
>>> p_addr_item- 
>>>> address.as_bytes[2],
>>>                                         p_addr_item-
>>>> address.as_bytes[3]));
>>> 
>>> -                       if( p_addr_item->p_reg )
>>> -                       {
>>> -                               if( p_addr_item->p_reg->h_reg_svc )
>>> -                               {
>>> -                                       p_adapter->p_ifc->dereg_svc(
>>> -                                               p_addr_item->p_reg-
>>>> h_reg_svc, __ipoib_ats_dereg_cb );
>>> -                               }
>>> -                               else
>>> -                               {
>>> -                                       cl_free( p_addr_item->p_reg
>>> ); 
>>> -                               }
>>> -                               p_addr_item->p_reg = NULL;
>>> -                       }
>>>                         p_addr_item->address.as_ulong = 0;         
>>>         } }
>>> @@ -2251,20 +2211,6 @@ __ipoib_set_net_addr(
>>>                  * Copy the address information, but don't register
>>> yet - the port 
>>>                  * could be down.
>>>                  */
>>> -               if( p_addr_item->p_reg )
>>> -               {
>>> -                       /* If in use by some other address,
>>> deregister. */ 
>>> -                       if( p_addr_item->p_reg->h_reg_svc )
>>> -                       {
>>> -                               p_adapter->p_ifc->dereg_svc(
>>> -
>> p_addr_item->p_reg->h_reg_svc,
>>> __ipoib_ats_dereg_cb );
>>> -                       }
>>> -                       else
>>> -                       {
>>> -                               cl_free( p_addr_item->p_reg );
>>> -                       }
>>> -                       p_addr_item->p_reg = NULL;
>>> -               }
>>>                 memcpy ((void
>> *)&p_addr_item->address.as_ulong, (const
>>> void *)&p_ip_addr->in_addr, sizeof(ULONG) );
>>>                 IPOIB_PRINT( TRACE_LEVEL_INFORMATION,IPOIB_DBG_OID,
>>>                         ("Port %d OID_GEN_NETWORK_LAYER_ADDRESSES -
>>> Adding Address %d.%d.%d.%d\n",
>>> @@ -2283,252 +2229,15 @@ __ipoib_set_net_addr(
>>>                         cl_vector_get_ptr( &p_adapter->ip_vector,
>>>                         cl_vector_get_size( &p_adapter->ip_vector )
>>> - 1 ); 
>>> 
>>> -               if( p_addr_item->p_reg )
>>> -               {
>>> -                       if( p_addr_item->p_reg->h_reg_svc )
>>> -                       {
>>> -                               p_adapter->p_ifc->dereg_svc(
>>> -
>> p_addr_item->p_reg->h_reg_svc,
>>> __ipoib_ats_dereg_cb );
>>> -                       }
>>> -                       else
>>> -                       {
>>> -                               cl_free( p_addr_item->p_reg );
>>> -                       }
>>> -                       p_addr_item->p_reg = NULL;
>>> -                       p_addr_item->address.as_ulong = 0;
>>> -               }
>>> +               p_addr_item->address.as_ulong = 0;
>>> 
>>>                 /* No need to check return value - shrinking always
>>>                 succeeds. */ cl_vector_set_size(
>>>                         &p_adapter->ip_vector, cl_vector_get_size(
>>>         &p_adapter->ip_vector ) - 1 ); }
>>> 
>>> -       if( p_adapter->state == IB_PNP_PORT_ACTIVE && p_adapter-
>>>> packet_filter )
>>> -               ipoib_reg_addrs( p_adapter );
>>> -
>>>         cl_obj_unlock( &p_adapter->obj );
>>> 
>>>         IPOIB_EXIT( IPOIB_DBG_OID );
>>>         return NDIS_STATUS_SUCCESS;
>>> -}
>>> -
>>> -
>>> -/* Object lock is held when this function is called. */
>>> -void
>>> -ipoib_reg_addrs(
>>> -       IN                              ipoib_adapter_t* const
>>> p_adapter ) -{
>>> -       net_address_item_t              *p_addr_item; -
>>> -       size_t                                  idx; -
>>> -       uint8_t                                 port_num; -
>>> -       ib_api_status_t                 ib_status;
>>> -       ib_reg_svc_req_t                ib_service;
>>> -       ib_gid_t                                port_gid; -
>>> -       IPOIB_ENTER( IPOIB_DBG_OID );
>>> -
>>> -       if(p_adapter->guids.port_guid.pkey != IB_DEFAULT_PKEY)
>>> -       {
>>> -               IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR,IPOIB_DBG_ERROR,
>>> -               ("ATS Service available for default pkey only\n"));
>>> -               return;
>>> -       }
>>> -       port_num = p_adapter->guids.port_num;
>>> -
>>> -       /* Setup our service call with things common to all calls */
>>> -       cl_memset( &ib_service, 0, sizeof(ib_service) ); -
>>> -       /* BUGBUG Only register local subnet GID prefix for now */
>>> -       ib_gid_set_default( &port_gid,
>> p_adapter->guids.port_guid.guid
>>> );
>>> -       ib_service.svc_rec.service_gid          = port_gid; -
>>> -       ib_service.svc_rec.service_pkey         = IB_DEFAULT_PKEY;
>>> -       ib_service.svc_rec.service_lease        =
>>> IB_INFINITE_SERVICE_LEASE;
>>> -
>>> -       /* Must cast here because the service name is an array of
>>> unsigned chars but 
>>> -        * strcpy want a pointer to a signed char */
>>> -       strcpy( (char *)ib_service.svc_rec.service_name, ATS_NAME
>>> ); - 
>>> -       /* IP Address in question will be put in below */
>>> -       ib_service.port_guid            = p_adapter-
>>>> guids.port_guid.guid;
>>> -       ib_service.timeout_ms           =
>>> p_adapter->params.sa_timeout; 
>>> -       ib_service.retry_cnt            = p_adapter-
>>>> params.sa_retry_cnt;
>>> -
>>> -       /* Can't set IB_FLAGS_SYNC here because I can't wait at
>>> dispatch */ 
>>> -       ib_service.flags                        = 0; -
>>> -       /* Service context will be put in below */
>>> -
>>> -       ib_service.svc_data_mask        = IB_SR_COMPMASK_SID
>>>> 
>>> -
>>> IB_SR_COMPMASK_SGID           |
>>> -
>>> IB_SR_COMPMASK_SPKEY          |
>>> -
>>> IB_SR_COMPMASK_SLEASE         |
>>> -
>>> IB_SR_COMPMASK_SNAME          |
>>> -
>>> IB_SR_COMPMASK_SDATA8_12      |
>>> -
>>> IB_SR_COMPMASK_SDATA8_13      |
>>> -
>>> IB_SR_COMPMASK_SDATA8_14      |
>>> -
>>> IB_SR_COMPMASK_SDATA8_15;
>>> -       ib_service.pfn_reg_svc_cb = __ipoib_ats_reg_cb; -
>>> -       for( idx = 0; idx < cl_vector_get_size(
>>> &p_adapter->ip_vector); idx++ ) 
>>> -       {
>>> -               p_addr_item = (net_address_item_t*)
>>> -                       cl_vector_get_ptr( &p_adapter->ip_vector,
>>> idx ); -
>>> -               if( p_addr_item->p_reg )
>>> -                       continue;
>>> -
>>> -               p_addr_item->p_reg = cl_zalloc( sizeof(ats_reg_t) );
>>> -               if( !p_addr_item->p_reg )
>>> -                       break;
>>> -
>>> -               p_addr_item->p_reg->p_adapter = p_adapter; -
>>> -               ib_service.svc_context          =
>>> p_addr_item->p_reg; - 
>>> -               ib_service.svc_rec.service_id =
>>> -                       ATS_SERVICE_ID &
>>> CL_HTON64(0xFFFFFFFFFFFFFF00); 
>>> -               /* ATS service IDs start at 0x10000CE100415453 */
>>> -               ib_service.svc_rec.service_id |= ((uint64_t)(idx +
>>> 0x53)) << 56; -
>>> -               cl_memcpy(
>>> &ib_service.svc_rec.service_data8[ATS_IPV4_OFFSET],
>>> -                       p_addr_item->address.as_bytes,
>>> IPV4_ADDR_SIZE ); -
>>> -               /* Take a reference for each service request. */
>>> -               cl_obj_ref(&p_adapter->obj);
>>> -               ib_status = p_adapter->p_ifc->reg_svc(
>>> -                       p_adapter->h_al, &ib_service, &p_addr_item-
>>>> p_reg->h_reg_svc );
>>> -               if( ib_status != IB_SUCCESS )
>>> -               {
>>> -                       if( ib_status == IB_INVALID_GUID )
>>> -                       {
>>> -                               /* If this occurs, we log the error
>>> but do not fail the OID yet */ 
>>> -                               IPOIB_PRINT( TRACE_LEVEL_WARNING,
>>> IPOIB_DBG_OID, 
>>> -                                       ("Port %d
>>> OID_GEN_NETWORK_LAYER_ADDRESSES - "
>>> -                                       "Failed to register IP
>>> Address " 
>>> -                                       "of %d.%d.%d.%d with error
>>> IB_INVALID_GUID\n", 
>>> -                                       port_num,
>>> -                                       p_addr_item-
>>>> address.as_bytes[0],
>>> -                                       p_addr_item-
>>>> address.as_bytes[1],
>>> -                                       p_addr_item-
>>>> address.as_bytes[2],
>>> -                                       p_addr_item-
>>>> address.as_bytes[3]) );
>>> -                       }
>>> -                       else
>>> -                       {
>>> -                               /* Fatal error. */
>>> -                               IPOIB_PRINT( TRACE_LEVEL_ERROR,
>>> IPOIB_DBG_ERROR, 
>>> -                                       ("Port %d
>>> OID_GEN_NETWORK_LAYER_ADDRESSES - Failed to register IP Address "
>>> -                                       "of %d.%d.%d.%d with error
>>> %s\n", 
>>> -                                       port_num,
>>> -                                       p_addr_item-
>>>> address.as_bytes[0],
>>> -                                       p_addr_item-
>>>> address.as_bytes[1],
>>> -                                       p_addr_item-
>>>> address.as_bytes[2],
>>> -                                       p_addr_item-
>>>> address.as_bytes[3],
>>> -
>> p_adapter->p_ifc->get_err_str(
>>> ib_status )) );
>>> -                               p_adapter->hung = TRUE;
>>> -                       }
>>> -                       cl_obj_deref(&p_adapter->obj);
>>> -                       cl_free( p_addr_item->p_reg );
>>> -                       p_addr_item->p_reg = NULL;
>>> -               }
>>> -       }
>>> -
>>> -       IPOIB_EXIT( IPOIB_DBG_OID );
>>> -}
>>> -
>>> -
>>> -/* Object lock is held when this function is called. */
>>> -void
>>> -ipoib_dereg_addrs(
>>> -       IN                              ipoib_adapter_t* const
>>> p_adapter ) -{
>>> -       net_address_item_t              *p_addr_item; -
>>> -       size_t                                  idx; -
>>> -       IPOIB_ENTER( IPOIB_DBG_OID );
>>> -
>>> -       for( idx = 0; idx < cl_vector_get_size(
>>> &p_adapter->ip_vector); idx++ ) 
>>> -       {
>>> -               p_addr_item = (net_address_item_t*)
>>> -                       cl_vector_get_ptr( &p_adapter->ip_vector,
>>> idx ); -
>>> -               if( !p_addr_item->p_reg )
>>> -                       continue;
>>> -
>>> -               if( p_addr_item->p_reg->h_reg_svc )
>>> -               {
>>> -                       p_adapter->p_ifc->dereg_svc(
>>> -                               p_addr_item->p_reg->h_reg_svc,
>>> __ipoib_ats_dereg_cb ); 
>>> -               }
>>> -               else
>>> -               {
>>> -                       cl_free( p_addr_item->p_reg );
>>> -               }
>>> -               p_addr_item->p_reg = NULL;
>>> -       }
>>> -
>>> -       IPOIB_EXIT( IPOIB_DBG_OID );
>>> -}
>>> -
>>> -
>>> -static void
>>> -__ipoib_ats_reg_cb(
>>> -       IN                              ib_reg_svc_rec_t
>>> *p_reg_svc_rec ) -{
>>> -       ats_reg_t                               *p_reg;
>>> -       uint8_t                                 port_num; -
>>> -       IPOIB_ENTER( IPOIB_DBG_OID );
>>> -
>>> -       CL_ASSERT( p_reg_svc_rec );
>>> -       CL_ASSERT( p_reg_svc_rec->svc_context );
>>> -
>>> -       p_reg = (ats_reg_t* VOID_PTR64)p_reg_svc_rec->svc_context;
>>> -       port_num = p_reg->p_adapter->guids.port_num; -
>>> -       cl_obj_lock( &p_reg->p_adapter->obj );
>>> -
>>> -       if( p_reg_svc_rec->req_status == IB_SUCCESS &&
>>> -               !p_reg_svc_rec->resp_status )
>>> -       {
>>> -               IPOIB_PRINT( TRACE_LEVEL_INFORMATION,IPOIB_DBG_OID,
>>> -                                        ("Port %d
>>> OID_GEN_NETWORK_LAYER_ADDRESSES - Registered IP Address "
>>> -                                         "of %d.%d.%d.%d\n",
>>> -                                         port_num,
>>> -                                         p_reg_svc_rec-
>>>> svc_rec.service_data8[ATS_IPV4_OFFSET],
>>> -                                         p_reg_svc_rec-
>>>> svc_rec.service_data8[ATS_IPV4_OFFSET+1],
>>> -                                         p_reg_svc_rec-
>>>> svc_rec.service_data8[ATS_IPV4_OFFSET+2],
>>> -                                         p_reg_svc_rec-
>>>> svc_rec.service_data8[ATS_IPV4_OFFSET+3]) );
>>> -       }
>>> -       else if( p_reg_svc_rec->req_status != IB_CANCELED )
>>> -       {
>>> -               IPOIB_PRINT( TRACE_LEVEL_ERROR, IPOIB_DBG_OID,
>>> -                                        ("Port %d
>>> OID_GEN_NETWORK_LAYER_ADDRESSES - Failed to register IP Address "
>>> -                                         "of %d.%d.%d.%d with
>>> error %s\n", 
>>> -                                         port_num,
>>> -                                         p_reg_svc_rec-
>>>> svc_rec.service_data8[ATS_IPV4_OFFSET],
>>> -                                         p_reg_svc_rec-
>>>> svc_rec.service_data8[ATS_IPV4_OFFSET+1],
>>> -                                         p_reg_svc_rec-
>>>> svc_rec.service_data8[ATS_IPV4_OFFSET+2],
>>> -                                         p_reg_svc_rec-
>>>> svc_rec.service_data8[ATS_IPV4_OFFSET+3],
>>> -                                         p_reg->p_adapter->p_ifc-
>>>> get_err_str( p_reg_svc_rec->resp_status )) );
>>> -               p_reg->p_adapter->hung = TRUE;
>>> -               p_reg->h_reg_svc = NULL;
>>> -       }
>>> -
>>> -       cl_obj_unlock( &p_reg->p_adapter->obj );
>>> -       cl_obj_deref(&p_reg->p_adapter->obj);
>>> -
>>> -       IPOIB_EXIT( IPOIB_DBG_OID );
>>> -}
>>> -
>>> -
>>> -static void
>>> -__ipoib_ats_dereg_cb(
>>> -       IN                              void
>>> *context )
>>> -{
>>> -       cl_free( context );
>>>  }
>>> diff -up -r -X trunk\docs\dontdiff.txt -I \$Id:
>>> old\ulp\ipoib\kernel\ipoib_port.c
>> trunk\ulp\ipoib\kernel\ipoib_port.c
>>> --- old\ulp\ipoib\kernel\ipoib_port.c   Mon Jun 30 11:09:26 2008
>>> +++ trunk\ulp\ipoib\kernel\ipoib_port.c Wed Jul 02 12:12:44 2008
>>> @@ -5240,10 +5240,6 @@ ipoib_port_down(
>>> 
>>>         __pending_list_destroy(p_port);
>>> 
>>> -       cl_obj_lock( &p_port->p_adapter->obj );
>>> -       ipoib_dereg_addrs( p_port->p_adapter );
>>> -       cl_obj_unlock( &p_port->p_adapter->obj );
>>> -
>>>         IPOIB_EXIT( IPOIB_DBG_INIT );
>>>  }
>> 
>> _______________________________________________
>> ofw mailing list
>> ofw at lists.openfabrics.org
>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
>> 
> _______________________________________________
> ofw mailing list
> ofw at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw




More information about the ofw mailing list