Changeset 34:0e2b57789361 in freeDiameter

Timestamp:

Oct 30, 2009, 5:23:06 PM (15 years ago)

Author:

Sebastien Decugis <sdecugis@nict.go.jp>

Branch:

default

Phase:

public

Message:

Backup for the WE, some warnings remaining

Files:

• freeDiameter/CMakeLists.txt (modified) (1 diff)
• freeDiameter/fD.h (modified) (5 diffs)
• freeDiameter/messages.c (modified) (1 diff)
• freeDiameter/p_out.c (modified) (3 diffs)
• freeDiameter/p_psm.c (modified) (9 diffs)
• freeDiameter/p_sr.c (added)
• freeDiameter/peers.c (modified) (5 diffs)
• include/freeDiameter/freeDiameter.h (modified) (1 diff)
• include/freeDiameter/libfreeDiameter.h (modified) (4 diffs)
• libfreeDiameter/messages.c (modified) (19 diffs)

freeDiameter/CMakeLists.txt

@@ -23 +23 @@
         p_out.c
         p_psm.c
+        p_sr.c
         server.c
         tcp.c

freeDiameter/fD.h

@@ -93 +93 @@
 int fd_dict_base_protocol(struct dictionary * dict);
 
+/* Sentinel for the sent requests list */
+struct sr_list {
+        struct fd_list  srs;
+        pthread_mutex_t mtx;
+};
+
 /* Peers */
 struct fd_peer { /* The "real" definition of the peer structure */
@@ -137 +143 @@
         
         /* Sent requests (for fallback), list of struct sentreq ordered by hbh */
-        struct fd_list   p_sentreq;
+        struct sr_list   p_sr;
         
         /* connection context: socket and related information */
@@ -204 +210 @@
 };
 
-/* Structure to store a sent request */
-struct sentreq {
-        struct fd_list  chain;  /* the "o" field points directly to the hop-by-hop of the request (uint32_t *)  */
-        struct msg      *req;   /* A request that was sent and not yet answered. */
-};
-
 
 /* Functions */
@@ -220 +220 @@
 /* fd_peer_add declared in freeDiameter.h */
 int fd_peer_validate( struct fd_peer * peer );
+void fd_peer_failover_msg(struct fd_peer * peer);
 
 /* Peer expiry */
@@ -236 +237 @@
 int fd_out_start(struct fd_peer * peer);
 int fd_out_stop(struct fd_peer * peer);
+
+/* Peer sent requests cache */
+int fd_p_sr_store(struct sr_list * srlist, struct msg **req, uint32_t *hbhloc);
+int fd_p_sr_fetch(struct sr_list * srlist, uint32_t hbh, struct msg **req);
+void fd_p_sr_failover(struct sr_list * srlist);
 
 /* Active peers -- routing process should only ever take the read lock, the write lock is managed by PSMs */

freeDiameter/messages.c

@@ -265 +265 @@
 }
 
+/* Parse a message against our dictionary, and in case of error log and eventually build the error reply -- returns the parsing status */
+int fd_msg_parse_or_error( struct msg ** msg )
+{
+        int ret = 0;
+        struct msg * m;
+        struct msg_hdr * hdr = NULL;
+        struct fd_pei   pei;
+        
+        TRACE_ENTRY("%p", msg);
+        
+        CHECK_PARAMS(msg && *msg);
+        m = *msg;
+        
+        /* Parse the message against our dictionary */
+        ret = fd_msg_parse_rules ( m, fd_g_config->cnf_dict, &pei);
+        if (ret != EBADMSG)
+                return ret;
+        
+        fd_log_debug("The following message does not comply to the dictionary and rules (%s):\n", pei.pei_errcode);
+        fd_msg_dump_walk(NONE, m);
+        
+        /* Now create an answer error if the message is a query */
+        CHECK_FCT( fd_msg_hdr(m, &hdr) );
+        
+        if (hdr->msg_flags & CMD_FLAG_REQUEST) {
+                
+                /* Create the error message */
+                CHECK_FCT( fd_msg_new_answer_from_req ( fd_g_config->cnf_dict, msg, pei.pei_protoerr ? MSGFL_ANSW_ERROR : 0 ) );
+                
+                /* Set the error code */
+                CHECK_FCT( fd_msg_rescode_set(*msg, pei.pei_errcode, pei.pei_message, pei.pei_avp, 1 ) );
+                
+        } else {
+                /* Just discard */
+                CHECK_FCT( fd_msg_free( m ) );
+                *msg = NULL;
+        }
+        
+        return ret;
+}

freeDiameter/p_out.c

@@ -37 +37 @@
 
 /* Alloc a new hbh for requests, bufferize the message and send on the connection, save in sentreq if provided */
-static int do_send(struct msg ** msg, struct cnxctx * cnx, uint32_t * hbh, struct fd_list * sentreq)
+static int do_send(struct msg ** msg, struct cnxctx * cnx, uint32_t * hbh, struct sr_list * srl)
 {
-        TRACE_ENTRY("%p %p %p %p", msg, cnx, hbh, sentreq);
+        struct msg_hdr * hdr;
+        int msg_is_a_req;
+        uint8_t * buf;
+        size_t sz;
+        int ret;
         
-        TODO("If message is a request");
-                TODO("Alloc new *hbh");
+        TRACE_ENTRY("%p %p %p %p", msg, cnx, hbh, srl);
         
-        TODO("Bufferize the message, send it");
+        /* Retrieve the message header */
+        CHECK_FCT( fd_msg_hdr(*msg, &hdr) );
         
-        TODO("Save in sentreq or free")
+        msg_is_a_req = (hdr->msg_flags & CMD_FLAG_REQUEST);
+        if (msg_is_a_req) {
+                CHECK_PARAMS(hbh && srl);
+                /* Alloc the hop-by-hop id and increment the value for next message */
+                hdr->msg_hbhid = *hbh;
+                *hbh = hdr->msg_hbhid + 1;
+        }
         
-        return ENOTSUP;
+        /* Create the message buffer */
+        CHECK_FCT(fd_msg_bufferize( *msg, &buf, &sz ));
+        
+        /* Send the message */
+        pthread_cleanup_push( free, buf );
+        CHECK_FCT_DO( ret = fd_cnx_send(cnx, buf, sz), { free(buf); return ret; } );
+        pthread_cleanup_pop(1);
+        
+        /* Save a request */
+        if (msg_is_a_req) {
+                CHECK_FCT_DO( fd_p_sr_store(srl, msg, &hdr->msg_hbhid),
+                        {
+                                fd_log_debug("The following request was sent successfully but not saved locally:\n" );
+                                fd_log_debug("  (as a result the matching answer will be discarded)\n" );
+                                fd_msg_dump_walk(NONE, *msg);
+                        } );
+                                
+        }
+        
+        /* Free answers and unsaved requests */
+        if (*msg) {
+                CHECK_FCT( fd_msg_free(*msg) );
+                *msg = NULL;
+        }
+        
+        return 0;
+}
+
+static void cleanup_requeue(void * arg)
+{
+        struct msg *msg = arg;
+        CHECK_FCT_DO(fd_fifo_post(fd_g_outgoing, &msg),
+                        CHECK_FCT_DO(fd_msg_free(msg), /* What can we do more? */));
 }
 
@@ -54 +96 @@
 static void * out_thr(void * arg)
 {
-        TODO("Pick next message in peer->p_tosend");
-        TODO("do_send, log errors");
-        TODO("In case of cancellation, requeue the message");
-        return NULL;
+        struct fd_peer * peer = arg;
+        ASSERT( CHECK_PEER(peer) );
+        
+        /* Set the thread name */
+        {
+                char buf[48];
+                sprintf(buf, "OUT/%.*s", sizeof(buf) - 5, peer->p_hdr.info.pi_diamid);
+                fd_log_threadname ( buf );
+        }
+        
+        /* Loop until cancelation */
+        while (1) {
+                struct msg * msg;
+                
+                /* Retrieve next message to send */
+                CHECK_FCT_DO( fd_fifo_get(peer->p_tosend, &msg), goto error );
+                
+                /* Now if we are cancelled, we requeue this message */
+                pthread_cleanup_push(cleanup_requeue, msg);
+                
+                /* Send the message, log any error */
+                CHECK_FCT_DO( do_send(&msg, peer->p_cnxctx, &peer->p_hbh, &peer->p_sr),
+                        {
+                                fd_log_debug("An error occurred while sending this message, it is lost:\n");
+                                fd_msg_dump_walk(NONE, msg);
+                                fd_msg_free(msg);
+                        } );
+                        
+                /* Loop */
+                pthread_cleanup_pop(0);
+        }
+        
 error:
-        TODO(" Send an event to the peer ");
+        /* It is not really a connection error, but the effect is the same, we are not able to send anymore message */
+        CHECK_FCT_DO( fd_event_send(peer->p_events, FDEVP_CNX_ERROR, 0, NULL), /* What do we do if it fails? */ );
         return NULL;
 }
@@ -84 +155 @@
 
                 /* Do send the message */
-                CHECK_FCT( do_send(msg, cnx, hbh, peer ? &peer->p_sentreq : NULL) );
+                CHECK_FCT( do_send(msg, cnx, hbh, peer ? &peer->p_sr : NULL) );
         }
         

freeDiameter/p_psm.c

@@ -84 +84 @@
 /************************************************************************/
 
-
 /* Enter/leave OPEN state */
 static int enter_open_state(struct fd_peer * peer)
 {
+        struct fd_list * li;
+        CHECK_PARAMS( FD_IS_LIST_EMPTY(&peer->p_actives) );
+        
+        /* Callback registered by the credential validator (fd_peer_validate_register) */
+        if (peer->p_cb2) {
+                CHECK_FCT_DO( (*peer->p_cb2)(&peer->p_hdr.info),
+                        {
+                                TRACE_DEBUG(FULL, "Validation failed, moving to state CLOSING");
+                                peer->p_hdr.info.pi_state = STATE_CLOSING;
+                                fd_psm_terminate(peer);
+                        } );
+                peer->p_cb2 = NULL;
+                return 0;
+        }
+        /* Insert in the active peers list */
         CHECK_POSIX( pthread_rwlock_wrlock(&fd_g_activ_peers_rw) );
-        TODO(" insert in fd_g_activ_peers ");
-        
+        for (li = fd_g_activ_peers.next; li != &fd_g_activ_peers; li = li->next) {
+                struct fd_peer * next_p = (struct fd_peer *)li->o;
+                int cmp = strcmp(peer->p_hdr.info.pi_diamid, next_p->p_hdr.info.pi_diamid);
+                if (cmp < 0)
+                        break;
+        }
+        fd_list_insert_before(li, &peer->p_actives);
         CHECK_POSIX( pthread_rwlock_unlock(&fd_g_activ_peers_rw) );
+        
+        /* Callback registered when the peer was added, by fd_peer_add */
+        if (peer->p_cb) {
+                TRACE_DEBUG(FULL, "Calling add callback for peer %s", peer->p_hdr.info.pi_diamid);
+                (*peer->p_cb)(&peer->p_hdr.info, peer->p_cb_data);
+                peer->p_cb = NULL;
+                peer->p_cb_data = NULL;
+        }
         
         /* Start the thread to handle outgoing messages */
         CHECK_FCT( fd_out_start(peer) );
         
-        return ENOTSUP;
+        return 0;
 }
 static int leave_open_state(struct fd_peer * peer)
 {
-        TODO("Remove from active list");
+        /* Remove from active peers list */
+        CHECK_POSIX( pthread_rwlock_wrlock(&fd_g_activ_peers_rw) );
+        fd_list_unlink( &peer->p_actives );
+        CHECK_POSIX( pthread_rwlock_unlock(&fd_g_activ_peers_rw) );
         
         /* Stop the "out" thread */
         CHECK_FCT( fd_out_stop(peer) );
         
-        TODO("Failover pending messages: requeue in global structures");
-        
-        return ENOTSUP;
-}
+        /* Failover the messages */
+        fd_peer_failover_msg(peer);
+        
+        return 0;
+}
+
 
 /************************************************************************/
@@ -165 +197 @@
         peer->p_psm_timer.tv_sec += delay;
         
-#if 0
+#ifdef SLOW_PSM
         /* temporary for debug */
         peer->p_psm_timer.tv_sec += 10;
@@ -188 +220 @@
 {
         struct fd_peer * peer = (struct fd_peer *)arg;
-        int created_started = started;
+        int created_started = started ? 1 : 0;
         int event;
         size_t ev_sz;
@@ -214 +246 @@
                 psm_next_timeout(peer, 0, INCNX_TIMEOUT);
         } else {
-                psm_next_timeout(peer, created_started ? 0 : 1, 0);
+                psm_next_timeout(peer, created_started, 0);
         }
         
@@ -236 +268 @@
                 TRACE_DEBUG(INFO, "Invalid event received in PSM '%s' : %d", peer->p_hdr.info.pi_diamid, event);
                 goto psm_loop;
-        }
-
-        /* Call the extension callback if needed */
-        if (peer->p_cb) {
-                /* Check if we must call it */
-                        /*  */
-                /* OK */
-                TODO("Call CB");
-                TODO("Clear CB");
         }
 
@@ -277 +300 @@
         /* A message was received */
         if (event == FDEVP_CNX_MSG_RECV) {
-                TODO("Parse the buffer into a message");
-                /* parse_and_get_local_ccode */
+                struct msg * msg = NULL;
+                struct msg_hdr * hdr;
+                
+                /* Parse the received buffer */
+                CHECK_FCT_DO( fd_msg_parse_buffer( (void *)&ev_data, ev_sz, &msg), 
+                        {
+                                fd_log_debug("Received invalid data from peer '%s', closing the connection\n", peer->p_hdr.info.pi_diamid);
+                                CHECK_FCT_DO( fd_event_send(peer->p_events, FDEVP_CNX_ERROR, 0, NULL), goto psm_end );
+                                goto psm_loop;
+                        } );
+                
+                TRACE_DEBUG(FULL, "Received this message from '%s':", peer->p_hdr.info.pi_diamid);
+                fd_msg_dump_walk(FULL, msg);
+        
+                /* Extract the header */
+                CHECK_FCT_DO( fd_msg_hdr(msg, &hdr), goto psm_end );
+                
+                /* If it is an answer, associate with the request */
+                if (!(hdr->msg_flags & CMD_FLAG_REQUEST)) {
+                        struct msg * req;
+                        /* Search matching request (same hbhid) */
+                        CHECK_FCT_DO( fd_p_sr_fetch(&peer->p_sr, hdr->msg_hbhid, &req), goto psm_end );
+                        if (req == NULL) {
+                                fd_log_debug("Received a Diameter answer message with no corresponding sent request, discarding...\n");
+                                fd_msg_dump_walk(NONE, msg);
+                                fd_msg_free(msg);
+                                goto psm_loop;
+                        }
+                        
+                        /* Associate */
+                        CHECK_FCT_DO( fd_msg_answ_associate( msg, req ), goto psm_end );
+                }
+                
+                /* We received a valid message, update the expiry timer */
+                CHECK_FCT_DO( fd_p_expi_update(peer), goto psm_end );
+
+                /* Now handle non-link-local messages */
+                if (fd_msg_is_routable(msg)) {
+                        /* If we are not in OPEN state, discard the message */
+                        if (peer->p_hdr.info.pi_state != STATE_OPEN) {
+                                fd_log_debug("Received a routable message while not in OPEN state from peer '%s', discarded.\n", peer->p_hdr.info.pi_diamid);
+                                fd_msg_dump_walk(NONE, msg);
+                                fd_msg_free(msg);
+                        } else {
+                                /* Set the message source and add the Route-Record */
+                                CHECK_FCT_DO( fd_msg_source_set( msg, peer->p_hdr.info.pi_diamid, 1, fd_g_config->cnf_dict ), goto psm_end);
+
+                                /* Requeue to the global incoming queue */
+                                CHECK_FCT_DO(fd_fifo_post(fd_g_incoming, &msg), goto psm_end );
+                                
+                                /* Update the peer timer */
+                                if (!peer->p_flags.pf_dw_pending) {
+                                        psm_next_timeout(peer, 1, peer->p_hdr.info.pi_twtimer ?: fd_g_config->cnf_timer_tw);
+                                }
+                        }
+                        goto psm_loop;
+                }
+                
+                /* Link-local message: They must be understood by our dictionary */
+                
                 TODO("Check if it is a local message (CER, DWR, ...)");
                 TODO("If not, check we are in OPEN state");
@@ -319 +400 @@
                         case STATE_CLOSED:
                                 TODO("Handle the CER, validate the peer if needed (and set expiry), set the alt_fifo in the connection, reply a CEA, eventually handshake, move to OPEN or REOPEN state");
+                                /* In case of error : DIAMETER_UNKNOWN_PEER */
                                 break;
                                 
@@ -353 +435 @@
         
         goto psm_loop;
-        
+
 psm_end:
         pthread_cleanup_pop(1); /* set STATE_ZOMBIE */
@@ -398 +480 @@
 {
         TRACE_ENTRY("%p", peer);
-        TODO("Cancel PSM thread");
-        TODO("Cancel OUT thread");
-        TODO("Cleanup the peer connection object");
-        TODO("Cleanup the message queues (requeue)");
-        TODO("Call p_cb with NULL parameter if needed");
-        
+        
+        /* Cancel PSM thread */
+        CHECK_FCT_DO( fd_thr_term(&peer->p_psm), /* continue */ );
+        
+        /* Cancel the OUT thread */
+        CHECK_FCT_DO( fd_out_stop(peer), /* continue */ );
+        
+        /* Cleanup the connection */
+        if (peer->p_cnxctx) {
+                fd_cnx_destroy(peer->p_cnxctx);
+        }
+        
+        /* Failover the messages */
+        fd_peer_failover_msg(peer);
+        
+        /* Empty the events list, this might leak some memory, but we only do it on exit, so... */
+        fd_event_destroy(&peer->p_events, free);
+        
+        /* More cleanups are performed in fd_peer_free */
         return;
 }

freeDiameter/peers.c

@@ -78 +78 @@
         CHECK_FCT( fd_fifo_new(&p->p_events) );
         CHECK_FCT( fd_fifo_new(&p->p_tosend) );
-        fd_list_init(&p->p_sentreq, p);
+        fd_list_init(&p->p_sr.srs, p);
+        CHECK_POSIX( pthread_mutex_init(&p->p_sr.mtx, NULL) );
         
         return 0;
@@ -180 +181 @@
         }
 
+/* Empty the lists of p_tosend and p_sentreq messages */
+void fd_peer_failover_msg(struct fd_peer * peer)
+{
+        struct msg *m;
+        TRACE_ENTRY("%p", peer);
+        CHECK_PARAMS_DO(CHECK_PEER(peer), return);
+        
+        /* Requeue all messages in the "out" queue */
+        while ( fd_fifo_tryget(peer->p_tosend, &m) == 0 ) {
+                CHECK_FCT_DO(fd_fifo_post(fd_g_outgoing, &m), 
+                                /* fallback: destroy the message */
+                                CHECK_FCT_DO(fd_msg_free(m), /* What can we do more? */));
+        }
+        
+        /* Requeue all routable sent requests */
+        fd_p_sr_failover(&peer->p_sr);
+        
+        /* Done */
+        return;
+}
+
 /* Destroy a structure once all cleanups have been performed */
 int fd_peer_free(struct fd_peer ** ptr)
@@ -197 +219 @@
         free_null(p->p_hdr.info.pi_realm); 
         free_list( &p->p_hdr.info.pi_endpoints );
-        /* Assume the security data is already freed */
+        TODO("Free the security data if any ?");
         free_null(p->p_hdr.info.pi_prodname);
         free_list( &p->p_hdr.info.pi_apps );
@@ -214 +236 @@
         
         CHECK_FCT( fd_thr_term(&p->p_outthr) );
-        while ( fd_fifo_tryget(p->p_tosend, &t) == 0 ) {
-                struct msg * m = t;
-                TRACE_DEBUG(FULL, "Found message %p in outgoing queue of peer %p being destroyed, requeue", m, p);
-                /* We simply requeue in global, the routing thread will re-handle it. */
-                CHECK_FCT(fd_fifo_post(fd_g_outgoing, &m));
-        }
-        CHECK_FCT( fd_fifo_del(&p->p_tosend) );
-        
-        while (!FD_IS_LIST_EMPTY(&p->p_sentreq)) {
-                struct sentreq * sr = (struct sentreq *)(p->p_sentreq.next);
-                fd_list_unlink(&sr->chain);
-                TRACE_DEBUG(FULL, "Found message %p in list of sent requests to peer %p being destroyed, requeue (fallback)", sr->req, p);
-                CHECK_FCT(fd_fifo_post(fd_g_outgoing, &sr->req));
-                free(sr);
-        }
         
         if (p->p_cnxctx) {
@@ -234 +241 @@
         }
         
+        /* Requeue any remaining message into global structures if possible */
+        fd_peer_failover_msg(p);
+        CHECK_FCT_DO( fd_fifo_del(&p->p_tosend), /* continue */ );
+        CHECK_POSIX_DO( pthread_mutex_destroy(&p->p_sr.mtx), /* continue */);
+        
+        /* If the callback is still around... */
         if (p->p_cb)
                 (*p->p_cb)(NULL, p->p_cb_data);
         
+        /* Free the structure */
         free(p);
-        
         return 0;
 }

include/freeDiameter/freeDiameter.h

@@ -492 +492 @@
 int fd_msg_add_origin ( struct msg * msg, int osi ); 
 
+/* Parse a message against our dictionary, and in case of error log and eventually build the error reply (on return and EBADMSG, *msg == NULL or *msg is the error message ready to send) */
+int fd_msg_parse_or_error( struct msg ** msg );
 
 

include/freeDiameter/libfreeDiameter.h

@@ -1990 +1990 @@
  *  !0          : an error occurred.
  */
-int fd_msg_source_set( struct msg * msg, char * diamid, uint32_t hash, int add_rr, struct dictionary * dict );
-int fd_msg_source_get( struct msg * msg, char ** diamid, uint32_t *hash );
+int fd_msg_source_set( struct msg * msg, char * diamid, int add_rr, struct dictionary * dict );
+int fd_msg_source_get( struct msg * msg, char ** diamid );
 
 /*
@@ -2138 +2138 @@
 int fd_msg_parse_dict ( msg_or_avp * object, struct dictionary * dict );
 
+/* Parsing Error Information structure */
+struct fd_pei {
+        char *          pei_errcode;    /* name of the error code to use */
+        struct avp *    pei_avp;        /* pointer to invalid or missing AVP (to be freed) */
+        char *          pei_message;    /* Overwrite default message if needed */
+        int             pei_protoerr;   /* do we set the 'E' bit in the error message ? */
+};
+
 /*
  * FUNCTION:    fd_msg_parse_rules
@@ -2144 +2152 @@
  *  object      : A msg or grouped avp object that must be verified.
  *  dict        : The dictionary containing the rules definitions.
- *  rule        : If not NULL, the first conflicting rule will be saved here if a conflict is found.
+ *  error_info  : If not NULL, the first problem information will be saved here.
  *
  * DESCRIPTION: 
@@ -2155 +2163 @@
  *  ENOMEM      : Unable to allocate enough memory to complete the operation.
  */
-int fd_msg_parse_rules ( msg_or_avp * object, struct dictionary * dict, struct dict_object ** rule);
+int fd_msg_parse_rules ( msg_or_avp * object, struct dictionary * dict, struct fd_pei *error_info);
+
 
 

libfreeDiameter/messages.c

@@ -124 +124 @@
                 }                msg_cb;                /* Callback to be called when an answer is received, if not NULL */
         char *                   msg_src_id;            /* Diameter Id of the peer this message was received from. This string is malloc'd and must be freed */
-        uint32_t                 msg_src_hash;          /* Hash of the msg_src_id value */
 };
 
@@ -668 +667 @@
                 msg->msg_public.msg_eteid
                 );
-        fd_log_debug(INOBJHDR "intern: rwb:%p rt:%d cb:%p(%p) qry:%p h:%x src:%s\n", 
-                        INOBJHDRVAL, msg->msg_rawbuffer, msg->msg_routable, msg->msg_cb.fct, msg->msg_cb.data, msg->msg_query, msg->msg_src_hash, msg->msg_src_id?:"(nil)");
+        fd_log_debug(INOBJHDR "intern: rwb:%p rt:%d cb:%p(%p) qry:%p src:%s\n", 
+                        INOBJHDRVAL, msg->msg_rawbuffer, msg->msg_routable, msg->msg_cb.fct, msg->msg_cb.data, msg->msg_query, msg->msg_src_id?:"(nil)");
 }
 
@@ -1077 +1076 @@
 
 /* Associate source peer */
-int fd_msg_source_set( struct msg * msg, char * diamid, uint32_t hash, int add_rr, struct dictionary * dict )
-{
-        TRACE_ENTRY( "%p %p %x %d %p", msg, diamid, hash, add_rr, dict);
+int fd_msg_source_set( struct msg * msg, char * diamid, int add_rr, struct dictionary * dict )
+{
+        TRACE_ENTRY( "%p %p %d %p", msg, diamid, add_rr, dict);
         
         /* Check we received a valid message */
@@ -1089 +1088 @@
         /* If the request is to cleanup the source, we are done */
         if (diamid == NULL) {
-                msg->msg_src_hash = 0;
                 return 0;
         }
@@ -1095 +1093 @@
         /* Otherwise save the new informations */
         CHECK_MALLOC( msg->msg_src_id = strdup(diamid) );
-        msg->msg_src_hash = hash;
         
         if (add_rr) {
@@ -1123 +1120 @@
 }
 
-int fd_msg_source_get( struct msg * msg, char ** diamid, uint32_t *hash )
-{
-        TRACE_ENTRY( "%p %p %p", msg, diamid, hash);
+int fd_msg_source_get( struct msg * msg, char ** diamid )
+{
+        TRACE_ENTRY( "%p %p", msg, diamid);
         
         /* Check we received valid parameters */
@@ -1133 +1130 @@
         /* Copy the informations */
         *diamid = msg->msg_src_id;
-        if (hash)
-                *hash = msg->msg_src_hash;
         
         /* done */
@@ -1863 +1858 @@
 /* We use this structure as parameter for the next function */
 struct parserules_data {
-        struct fd_list     * sentinel;  /* Sentinel of the list of children AVP */
-        struct dict_object * ruleavp;   /* If the rule conflicts, save the rule_avp here (we don't have direct access to the rule but it can be searched) */
+        struct fd_list  * sentinel;     /* Sentinel of the list of children AVP */
+        struct fd_pei   * pei;          /* If the rule conflicts, save the error here */
 };
+
+/* Create an empty AVP of a given model (to use in Failed-AVP) */
+static struct avp * empty_avp(struct dict_object * model_avp)
+{
+        TODO("Create the AVP instance and set a 0 value");
+        return NULL;
+}
 
 /* Check that a list of AVPs is compliant with a given rule -- will be iterated on the list of rules */
 static int parserules_check_one_rule(void * data, struct dict_rule_data *rule)
 {
-        int ret = 0, count, first, last, min;
-        struct parserules_data * pr_data = (struct parserules_data *) data;
+        int count, first, last, min;
+        struct parserules_data * pr_data = data;
         
         TRACE_ENTRY("%p %p", data, rule);
         
-        /* Get statistics of the AVP concerned by this rule in the message instance */
+        /* Get statistics of the AVP concerned by this rule in the parent instance */
         parserules_stat_avps( rule->rule_avp, pr_data->sentinel, &count, &first, &last);
         
@@ -1881 +1883 @@
         {
                 struct dict_avp_data avpdata;
+                int ret;
                 ret = fd_dict_getval(rule->rule_avp, &avpdata);
                 
@@ -1896 +1899 @@
         
         /* Now check the rule is not conflicting */
-        ret = 0;
         
         /* Check the "min" value */
@@ -1907 +1909 @@
         if (count < min) {
                 TRACE_DEBUG(INFO, "Conflicting rule: the number of occurences (%d) is < the rule min (%d).", count, min);
-                ret = EBADMSG;
-                goto end;
+                if (pr_data->pei) {
+                        pr_data->pei->pei_errcode = "DIAMETER_MISSING_AVP";
+                        pr_data->pei->pei_avp = empty_avp(rule->rule_avp);
+                }
+                return EBADMSG;
         }
         
@@ -1914 +1919 @@
         if ((rule->rule_max != -1) && (count > rule->rule_max)) {
                 TRACE_DEBUG(INFO, "Conflicting rule: the number of occurences (%d) is > the rule max (%d).", count, rule->rule_max);
-                ret = EBADMSG;
-                goto end;
+                if (pr_data->pei) {
+                        if (rule->rule_max == 0)
+                                pr_data->pei->pei_errcode = "DIAMETER_AVP_NOT_ALLOWED";
+                        else
+                                pr_data->pei->pei_errcode = "DIAMETER_AVP_OCCURS_TOO_MANY_TIMES";
+                        pr_data->pei->pei_avp = empty_avp(rule->rule_avp); /* Well we are supposed to return the (max + 1)th instance of the AVP instead... Pfff... */ TODO("Improve...");
+                }
+                return EBADMSG;
         }
                 
@@ -1929 +1940 @@
                         if (first > rule->rule_order) {
                                 TRACE_DEBUG(INFO, "Conflicting rule: the FIXED_HEAD AVP appears first in (%d) position, the rule requires (%d).", first, rule->rule_order);
-                                ret = EBADMSG;
-                                goto end;
+                                if (pr_data->pei) {
+                                        pr_data->pei->pei_errcode = "DIAMETER_MISSING_AVP";
+                                        pr_data->pei->pei_message = "AVP was not in its fixed position";
+                                        pr_data->pei->pei_avp = empty_avp(rule->rule_avp);
+                                }
+                                return EBADMSG;
                         }
                         break;
@@ -1938 +1953 @@
                         if (last > rule->rule_order) {  /* We have a ">" here because we count in reverse order (i.e. from the end) */
                                 TRACE_DEBUG(INFO, "Conflicting rule: the FIXED_TAIL AVP appears last in (%d) position, the rule requires (%d).", last, rule->rule_order);
-                                ret = EBADMSG;
-                                goto end;
+                                if (pr_data->pei) {
+                                        pr_data->pei->pei_errcode = "DIAMETER_MISSING_AVP";
+                                        pr_data->pei->pei_message = "AVP was not in its fixed position";
+                                        pr_data->pei->pei_avp = empty_avp(rule->rule_avp);
+                                }
+                                return EBADMSG;
                         }
                         break;
@@ -1946 +1965 @@
                         /* What is this position ??? */
                         ASSERT(0);
-                        ret = ENOTSUP;
+                        return ENOTSUP;
         }
         
         /* We've checked all the parameters */
-end:
-        if (ret == EBADMSG) {
-                pr_data->ruleavp = rule->rule_avp;
-        }
-
-        return ret;
+        return 0;
 }
 
 /* Check the rules recursively */
-static int parserules_do ( struct dictionary * dict, msg_or_avp * object, struct dict_object ** conflict_rule, int mandatory)
-{
-        int ret = 0;
+static int parserules_do ( struct dictionary * dict, msg_or_avp * object, struct fd_pei *error_info, int mandatory)
+{
         struct parserules_data data;
         struct dict_object * model = NULL;
         
-        TRACE_ENTRY("%p %p %p %d", dict, object, conflict_rule, mandatory);
+        TRACE_ENTRY("%p %p %p %d", dict, object, error_info, mandatory);
         
         /* object has already been checked and dict-parsed when we are called. */
@@ -1981 +1994 @@
                         if (model == NULL) {
                                 TRACE_DEBUG(INFO, "Message with no dictionary model. EBADMSG");
+                                if (error_info) {
+                                        error_info->pei_errcode = "DIAMETER_COMMAND_UNSUPPORTED";
+                                        error_info->pei_protoerr = 1;
+                                }
                                 return EBADMSG;
                         }
@@ -1990 +2007 @@
                                 /* Return an error in this case */
                                 TRACE_DEBUG(INFO, "Mandatory AVP with no dictionary model. EBADMSG");
+                                if (error_info) {
+                                        error_info->pei_errcode = "DIAMETER_AVP_UNSUPPORTED";
+                                        error_info->pei_avp = object;
+                                }
                                 return EBADMSG;
                         } else {
@@ -2019 +2040 @@
                         is_child_mand = 1;
                 for (ch = _C(object)->children.next; ch != &_C(object)->children; ch = ch->next) {
-                        CHECK_FCT(  parserules_do ( dict, _C(ch->o), conflict_rule, is_child_mand )  );
+                        CHECK_FCT(  parserules_do ( dict, _C(ch->o), error_info, is_child_mand )  );
                 }
         }
@@ -2025 +2046 @@
         /* Now check all rules of this object */
         data.sentinel = &_C(object)->children;
-        data.ruleavp  = NULL;
-        ret = fd_dict_iterate_rules ( model, &data, parserules_check_one_rule );
-        
-        /* Save the reference to the eventual conflicting rule; otherwise set to NULL */
-        if (conflict_rule && data.ruleavp) {
-                /* data.ruleavp contains the AVP, and model is the parent */
-                struct dict_object * rule = NULL;
-                struct dict_rule_request req = { model, data.ruleavp };
-                
-                CHECK_FCT_DO( fd_dict_search ( dict, DICT_RULE, RULE_BY_AVP_AND_PARENT, &req, &rule, ENOENT),  rule = NULL );
-                
-                *conflict_rule = rule;
-        }
-        
-        return ret;
-}
-
-int fd_msg_parse_rules ( msg_or_avp * object, struct dictionary * dict, struct dict_object ** rule)
-{
-        TRACE_ENTRY("%p %p", object, rule);
+        data.pei  = error_info;
+        CHECK_FCT( fd_dict_iterate_rules ( model, &data, parserules_check_one_rule ) );
+        
+        return 0;
+}
+
+int fd_msg_parse_rules ( msg_or_avp * object, struct dictionary * dict, struct fd_pei *error_info)
+{
+        TRACE_ENTRY("%p %p %p", object, dict, error_info);
         
         /* Resolve the dictionary objects when missing. This also validates the object. */
         CHECK_FCT(  fd_msg_parse_dict ( object, dict )  );
         
+        if (error_info)
+                memset(error_info, 0, sizeof(struct fd_pei));
+        
         /* Call the recursive function */
-        return parserules_do ( dict, object, rule, 1 ) ;
+        return parserules_do ( dict, object, error_info, 1 ) ;
 }