[Pascal Quantin <pascal.quantin@xxxxxxxxx>]

2015-02-25 13:36 GMT+01:00 Raj sekar <mrajsekar@xxxxxxxxx>:

Hi pascal

Thanks for your time.

just to verify few things

1. Offset_payload  is guint8  is that ok?

It depends on your protocol: only you can tell whether there is a risk of overflow. That said if your pdus can be big, it would be better to use  32 bits variable.

2.  pdu_length -> is the total length of the pdu  (total length = 22 ; beginning of message 20 and end of message 2 bytes) 

I have also tried replacing pdu_length, with rem_length  (which is only particular length example end of message 2 bytes)

Please read the header file more carefully: it is stated that this parameter should be the fragment length, not the total length

3. Do I need to use table destroy function ? In the end ? 

I have no idea what you are talking about

4. Do I need to use fragment_set_total_length? I have tried already. But no change so I have commented off

 I assume you are talking about fragment_set_tot_len. As indicated in the header file:
/* to specify how much to reassemble, for fragmentation where last fragment can not be
 * identified by flags or such.
 * note that for FD_BLOCKSEQUENCE tot_len is the index for the tail fragment.
 * i.e. since the block numbers start at 0, if we specify tot_len==2, that
 * actually means we want to defragment 3 blocks, block 0, 1 and 2.
 *
 */

As you know what the last fragment is, you should not need it.

5. I have not used any GHASH table in my code. Some people used that and use of fragment table ang reassembly table.

In recent versions of Wireshark those functions do not exist anymore. As you seem to base your development on an obsolete Wireshark version (or Ethereal according to what you said ealier) I can only suggest you to compare your code with what are done in other dissectors of this specific version. Asking for help here or on ask.wireshark.org for help regarding a version that is obsolete since several years and without stating which version you are using is the best way to get partial / incomplete answers. I'm not sure you will find anyone remembering how the code was done in those old versions.

fragment_add_check(
                       &mns_reassembly_table,
                           next_tvb,
                          offset_payload,
                        pinfo,                                           mns_seqid,
                        NULL,
                          msg_seqid,

                      pdu_length,
      (stype == 0x02) ? FALSE : TRUE)  );

Now I realize that at some point in your numerous changes, you swapped from fragment_add_seq_check (that you were using in the email you sent on the 17th of February) to fragment_add_check! Then the code I provided earlier today cannot work as it is intended to be used with fragment_add_seq_check (that use sequence numbers and not fragment length, as explained in the header file).

Thanks again  and have a nice day!

Raj

On 25 Feb 2015 19:33, "Pascal Quantin" <pascal.quantin@xxxxxxxxx> wrote:

Back to the mailing list

2015-02-25 12:07 GMT+01:00 Raj sekar <mrajsekar@xxxxxxxxx>:

Hi Pascal ,

 

I have modified as per your instruction . Still iam getting Fragment not successful and reassembly unsuccessful. :(

 

You can see my code. It exactly as you said.

 

Please help.

Then it's really time for you to take a debugger and step in your code so as to verify what's wrong with your code. After all you are the only one having the capture and the code running, so confirm that the parameters given to fragment_add_seq_check() are the expected ones.

 

Raj

 

 

 

On Wed, Feb 25, 2015 at 5:36 PM, Pascal Quantin <pascal.quantin@xxxxxxxxx> wrote:

Le 25 févr. 2015 10:07, "Raj sekar" <mrajsekar@xxxxxxxxx> a écrit :

>
> i have a off-line capture file..
>
> iam developing dissector for customised protocol
>
> i have a old ethereal tool for the same protocol now iam developing in wireshark.
>
> My message pdu got 3 different message types
>
>   1. Beginning of message
>   2. continuation of message
>   3. end of message
>
> in one Frame at a time it can receive any one of this message
>
> when it End of message sequence comes i need to reassemble whole PDU
>
> i have opened in ethereal tool and my output are like this
>  
> Frame Number   
> Message type
> pollflag
> NS (sequence Number)
> NR (SEquence ID)
>  
>  
> 283
> Beginning of message
> 0
> 0
> 0
> 343
> End of message 
> 0
> 1
> 0
>  -> Reassembly done here
>  
>  
> 379
> Beginning of message 
> 0
> 2
> 1
> 414
> Continuation of message
> 0
> 3
> 1
> 416
> Continuation of message
> 0
> 4
> 1
> 417
> End of Message
> 1
> 5
> 1
>  -> Reassembly done here
>  
>  
> 536
> Beginning of message
> 0
> 6
> 2
> 541
> End of meassage
> 0
> 7
> 2
>  -> Reassembly done here

Hi,

I'm really happy to see that you did not follow the explanations and advices I gave you yesterday by private email because you contacted me directly instead of sending to the mailing list.
As I told you your code cannot work as-is because it assumes that you will always get a begin - continuation - end sequence (so 3 fragments). If this is not the case and if you are sure to always receive the fragments in order, you could use an (evil ;)) global variable to keep track of the current fragment number. See a suggestion below in your code. If you put some effort in understanding what I explained you, and ensure that you are correctly setting the parameters of the reassembly API, it should work correctly.

>
>
> I have used my code structure like this
>
>
> guint32 rem_length;
> guint8 iflag,pf,stype,flag_sel,num_sel,i,sflag;  //
> guint32 pdu_len;
> guint8 save_fragmented;
> gboolean more_frags = FALSE;
> gboolean need_frag = FALSE;
> const *data = ""
> //tvbuff_t *try_tvb = NULL;
> //proto_item *frag_tree_item;
> tvbuff_t  *rass_tvb = NULL;
> tvbuff_t  *mns_tvb = NULL;
> guint32 msg_seqid;

replace it by:

static guint32 msg_seqid = 0;

> guint32 mns_seqid = 0;
> guint32 mns_seqnum = 0;
> fragment_head *frag_msg = NULL;
> gboolean reassembled = FALSE;
> //guint32  reassembled_in = 0;
> tvbuff_t * res_tvb = NULL;
> fragment_head * frag_head = NULL; 
> proto_tree *ptree = NULL;
> ptree = proto_tree_get_parent(tree);
>
>         
>          pf = (tvb_get_guint8(next_tvb, offset_payload) & 0x40) >>6 ; // Bit 7 pf
>          mns_seqnum = (tvb_get_ntohs(next_tvb, offset_payload) & 0x3ff00000) >>20; // 10 Bits are ns
>          offset_payload +=1;
>          mns_seqid = (tvb_get_ntohs(next_tvb, offset_payload) & 0x0ffc0000) >>18; // 10 bits are nr
>          offset_payload +=1;
>          stype = (tvb_get_guint8(next_tvb, offset_payload) & 0x03) ; // 2 bits LSB are Stype
>          offset_payload -=2;
>          
>          FT_BCnPDU_item = proto_tree_add_text(tree, next_tvb, offset_payload, bctsdu_length, "BCnPDU (Formatted) : Information, I flag = 0x%02x, Pf = 0x%02x, Ns = 0x%02x, Nr = 0x%02x ", iflag,pf,mns_seqnum,mns_seqid );
>          FT_BCnPDU_tree = proto_item_add_subtree(FT_BCnPDU_item, ett_FT_BCnPDU);
>          proto_tree_add_text(FT_BCnPDU_tree, next_tvb, offset_payload, 1, " bctsdu_length: %d", bctsdu_length);
>     
>          proto_tree_add_text(FT_BCnPDU_tree, next_tvb, offset_payload, 1, "I flag : 0x%02x (%s)", iflag,val_to_str(iflag,true_false_vals,"%s"));
>          proto_tree_add_text(FT_BCnPDU_tree, next_tvb, offset_payload, 1, "Pf : 0x%02x (%d)", pf,pf);
>          proto_tree_add_text(FT_BCnPDU_tree, next_tvb, offset_payload, 2, "Ns : 0x%02x (%d)", mns_seqnum,mns_seqnum);
>          offset_payload +=1;
>          bctsdu_length-=1;
>          proto_tree_add_text(FT_BCnPDU_tree, next_tvb, offset_payload, 2, "Nr : 0x%02x (%d)", mns_seqid,mns_seqid);
>          offset_payload +=1;
>          bctsdu_length-=1;
>          proto_tree_add_text(FT_BCnPDU_tree, next_tvb, offset_payload, 1, "BCnSegType : 0x%02x (%s)", stype,val_to_str(stype,BCn_Seg_Type_vals,"%s"));
>          offset_payload +=1;
>          bctsdu_length-=1;
>          
>          
>          switch (stype){
>          
>          case 0x00: // Continuation of Message
>            
>            msg_seqid=1;

replace it by:

msg_seqid++;

>           
>            bctsdu_length+=1;
>            
>            rem_length = bctsdu_length;
>            proto_tree_add_text(FT_BCnPDU_tree, next_tvb, offset_payload, rem_length , "PDU data : %d", rem_length);
>            
>                            
>            
>            
>          break;
>          
>          case 0x01: // Beginning of Message
>             
>             msg_seqid=0;
>             
>             
>             BCnPDU_stype_item = proto_tree_add_text(FT_BCnPDU_tree, next_tvb, offset_payload, 1, "%s",val_to_str(stype,BCn_Seg_Type_vals,"%s"));
>             BCnPDU_stype_tree = proto_item_add_subtree(BCnPDU_stype_item, ett_BCnPDU_stype);
>             
>             BCnPDU_bom_item = proto_tree_add_text(BCnPDU_stype_tree, next_tvb, offset_payload, 1, "MACSAPFLAGS");
>             BCnPDU_bom_tree = proto_item_add_subtree(BCnPDU_bom_item, ett_BCnPDU_bom);
>             
>             temp_val = (tvb_get_guint8(next_tvb, offset_payload) & 0x80) >>7 ; // Bit 8 - Flow Control
>             proto_tree_add_text(BCnPDU_bom_tree, next_tvb, offset_payload, 1, "Flow Control : 0x%02x (%s)", temp_val,val_to_str(temp_val, true_false_vals,"%s"));
>             temp_val = (tvb_get_guint8(next_tvb, offset_payload) & 0x40) >>6 ; // Bit 7 - Reserved l
>             proto_tree_add_text(BCnPDU_bom_tree, next_tvb, offset_payload, 1, "Reserved l : 0x%02x (%d)", temp_val,temp_val);
>             temp_val = (tvb_get_guint8(next_tvb, offset_payload) & 0x20) >>5 ; // Bit 6 - Expedited
>             proto_tree_add_text(BCnPDU_bom_tree, next_tvb, offset_payload, 1, "Expedited : 0x%02x (%s)", temp_val,val_to_str(temp_val, true_false_vals,"%s"));
>             temp_val = (tvb_get_guint8(next_tvb, offset_payload) & 0x10) >>4 ; // Bit 5 - OAM PDU Flag
>             proto_tree_add_text(BCnPDU_bom_tree, next_tvb, offset_payload, 1, "OAM PDU Flag : 0x%02x (%s)", temp_val,val_to_str(temp_val, true_false_vals,"%s"));
>             temp_val = (tvb_get_guint8(next_tvb, offset_payload) & 0x08) >>3 ; // Bit 4 - Reserved 2
>             proto_tree_add_text(BCnPDU_bom_tree, next_tvb, offset_payload, 1, "Reserved 2 : 0x%02x (%d)", temp_val,temp_val);
>             pdu_len = (tvb_get_ntohl(next_tvb, offset_payload) & 0x07ff0000) >>16; // PDU Length - 11 Bits
>             
>             proto_tree_add_text(BCnPDU_bom_tree, next_tvb, offset_payload, 2, "PDU Length : 0x%02x (%d)", pdu_len,pdu_len);
>             
>             offset_payload+=2;
>             bctsdu_length-=2;
>            
>                         
>             if (rem_length >= pdu_len){
>             
>             rem_length-=2;
>             
>             proto_tree_add_text(BCnPDU_bom_tree, next_tvb, offset_payload, rem_length, "PDU data (if) : %d ", rem_length);
>             
>             } else {
>           
>             proto_tree_add_text(BCnPDU_bom_tree, next_tvb, offset_payload, rem_length, "PDU data (else) : %d ", rem_length);
>             
>             }
>             
>             
>          break;
>          
>          case 0x02: // End of Message
>            
>            msg_seqid=2;

replace it by:

msg_seqid++;

>            
>            bctsdu_length+=2;
>            
>            more_frags=TRUE;
>            
>            
>            rem_length =  bctsdu_length;
>            rem_length-=2;
>            
>            proto_tree_add_text(FT_BCnPDU_tree, next_tvb, offset_payload, rem_length , "PDU data : %d", rem_length);
>           
>            disable_CRC=1;
>            
>            
>          break;
>          
>          case 0x03: // Single Segment Message
>             // No need of fragmentation and reassembly
>             offset_payload+=1;
>             bctsdu_length-=1;
>             if (bctsdu_length>0){
>            
>              ALSIGPDU(next_tvb, pinfo, tree);
>                }
>                
>            
>            break;
>            
>            }
>             
>             if ( stype==0x01 || stype==0x02 || stype==0x00  ){
>                        
>                    
>                        pinfo->fragmented = TRUE;
>                        save_fragmented = pinfo->fragmented;
>                        
>                        frag_head = fragment_add_check(&mns_reassembly_table,
>                           next_tvb,
>                           offset_payload,
>                           pinfo,
>                           msg_seqid, 

this should be mns_seqid

>                           NULL,
>                           mns_seqid,

this should be msg_seqid

>                           pdu_length,
>                           ((msg_seqid == 2)?0:1));

this should be:

(stype == 0x02) ? FALSE : TRUE)

>                           
>                           
>                           
>                       
>                         if (frag_head != NULL) {
>                          
>                            col_append_str(pinfo->cinfo, COL_INFO, " [Fragment Successful]");
>                           } else {
>                           
>                           col_append_str(pinfo->cinfo, COL_INFO, " [Fragment Not Successful]");
>                           }     
>                              
>                           save_fragmented = pinfo->fragmented;
>                        pinfo->fragmented = FALSE;
>                    
>
>                        
>                        res_tvb = process_reassembled_data( next_tvb, offset_payload, pinfo, "Reassembled PDU", frag_head, &mns_frag_items, NULL, FT_BCnPDU_tree );
>                        save_fragmented = pinfo->fragmented;
>                        pinfo->fragmented = FALSE;
>                           
>                        if (res_tvb) {
>                           
>                           col_append_str(pinfo->cinfo, COL_INFO, " [mns reassembled]");
>                           } else {
>                             col_append_str(pinfo->cinfo, COL_INFO, "[mns not reassembled ]");
>                             
>                             }
>                          offset_payload+=rem_length; 
>                          bctsdu_length-=rem_length;
>            
>                    }
>
>  
>
>     I dont have any clue my fragmentation always not succesful. please help.
>  
> Thanks
>  
> Best Regards
> Raj
>
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe