Robert Love wrote:
> On Mon, 2009-03-30 at 15:20 -0700, Robert Love wrote:
>> This patch adds a debug_logging module parameter to libfcoe.ko. It
>> is an unsigned int that represents a bitmask of available debug
>> logging levels, each of which can be tuned at runtime. Currently
>> there are only two logging levels for this module-
>>
>> bit
>> LSB 0 = libfcoe general logging
>> 1 = FIP logging
>>
>> Signed-off-by: Robert Love <[email protected]>
>> ---
>>
>> drivers/scsi/fcoe/libfcoe.c | 101
>> ++++++++++++++++++++++++++-----------------
>> 1 files changed, 61 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/scsi/fcoe/libfcoe.c b/drivers/scsi/fcoe/libfcoe.c
>> index f410f4a..b0180d8 100644
>> --- a/drivers/scsi/fcoe/libfcoe.c
>> +++ b/drivers/scsi/fcoe/libfcoe.c
>> @@ -56,15 +56,28 @@ static void fcoe_ctlr_recv_work(struct work_struct *);
>>
>> static u8 fcoe_all_fcfs[ETH_ALEN] = FIP_ALL_FCF_MACS;
>>
>> -static u32 fcoe_ctlr_debug; /* 1 for basic, 2 for noisy debug */
>> +unsigned int debug_logging;
>> +module_param(debug_logging, int, S_IRUGO|S_IWUSR);
>> +MODULE_PARM_DESC(debug_logging, "a bit mask of logging levels");
>>
>> -#define FIP_DBG_LVL(level, fmt, args...) \
>> - do { \
>> - if (fcoe_ctlr_debug >= (level)) \
>> - FC_DBG(fmt, ##args); \
>> - } while (0)
>> +#define LIBFCOE_LOGGING 0x01 /* General logging, not categorized */
>> +#define LIBFCOE_FIP_LOGGING 0x02 /* FIP logging */
>>
> Joe: I'd like to know what you think about this. I collapsed your two
> levels of logging into just one FIP logging level. I think there was
> only one instance of level two logging (maybe two). Do you think the
> verbosity of your level two print statement warrants it's own logging
> level in this new scheme?
Yes. Or some other way to separately enable them.
The level 2 messages come out every 8 seconds or more frequently.
The level 1 messages come out only on selection / link reset, etc., so
the console was pretty quiet with debug at level 1.
Note that "old" FCF messages were level 1, so repeated advertisements
that you'd already heard about were not printed.
You could just delete the level 2 debug prints, though. I wouldn't miss them
much.
>> -#define FIP_DBG(fmt, args...) FIP_DBG_LVL(1, fmt, ##args)
>> +#define LIBFCOE_CHECK_LOGGING(LEVEL, CMD) \
>> + do { \
>> + if (unlikely(debug_logging & LEVEL)) \
>> + do { \
>> + CMD; \
>> + } while (0); \
>> + } while (0);
>> +
>> +#define LIBFCOE_DBG(fmt, args...) \
>> + LIBFCOE_CHECK_LOGGING(LIBFCOE_LOGGING, \
>> + printk(KERN_DEBUG "libfcoe: " fmt, ##args);)
>> +
>> +#define LIBFCOE_FIP_DBG(fmt, args...)
>> \
>> + LIBFCOE_CHECK_LOGGING(LIBFCOE_FIP_LOGGING, \
>> + printk(KERN_DEBUG "fip: " fmt, ##args);)
>>
>> /*
>> * Return non-zero if FCF fcoe_size has been validated.
>> @@ -241,7 +254,7 @@ void fcoe_ctlr_link_up(struct fcoe_ctlr *fip)
>> fip->last_link = 1;
>> fip->link = 1;
>> spin_unlock_bh(&fip->lock);
>> - FIP_DBG("%s", "setting AUTO mode.\n");
>> + LIBFCOE_FIP_DBG("%s", "setting AUTO mode.\n");
>> fc_linkup(fip->lp);
>> fcoe_ctlr_solicit(fip, NULL);
>> } else
>> @@ -601,7 +614,8 @@ static int fcoe_ctlr_parse_adv(struct sk_buff *skb,
>> struct fcoe_fcf *fcf)
>> ((struct fip_mac_desc *)desc)->fd_mac,
>> ETH_ALEN);
>> if (!is_valid_ether_addr(fcf->fcf_mac)) {
>> - FIP_DBG("invalid MAC addr in FIP adv\n");
>> + LIBFCOE_FIP_DBG("Invalid MAC address "
>> + "in FIP adv\n");
>> return -EINVAL;
>> }
>> break;
>> @@ -634,8 +648,8 @@ static int fcoe_ctlr_parse_adv(struct sk_buff *skb,
>> struct fcoe_fcf *fcf)
>> case FIP_DT_LOGO:
>> case FIP_DT_ELP:
>> default:
>> - FIP_DBG("unexpected descriptor type %x in FIP adv\n",
>> - desc->fip_dtype);
>> + LIBFCOE_FIP_DBG("unexpected descriptor type %x "
>> + "in FIP adv\n", desc->fip_dtype);
>> /* standard says ignore unknown descriptors >= 128 */
>> if (desc->fip_dtype < FIP_DT_VENDOR_BASE)
>> return -EINVAL;
>> @@ -651,8 +665,8 @@ static int fcoe_ctlr_parse_adv(struct sk_buff *skb,
>> struct fcoe_fcf *fcf)
>> return 0;
>>
>> len_err:
>> - FIP_DBG("FIP length error in descriptor type %x len %zu\n",
>> - desc->fip_dtype, dlen);
>> + LIBFCOE_FIP_DBG("FIP length error in descriptor type %x len %zu\n",
>> + desc->fip_dtype, dlen);
>> return -EINVAL;
>> }
>>
>> @@ -715,9 +729,13 @@ static void fcoe_ctlr_recv_adv(struct fcoe_ctlr *fip,
>> struct sk_buff *skb)
>> }
>> mtu_valid = fcoe_ctlr_mtu_valid(fcf);
>> fcf->time = jiffies;
>> - FIP_DBG_LVL(found ? 2 : 1, "%s FCF for fab %llx map %x val %d\n",
>> - found ? "old" : "new",
>> - fcf->fabric_name, fcf->fc_map, mtu_valid);
>> + if (found) {
>> + LIBFCOE_FIP_DBG("Old FCF for fab %llx map %x val %d\n",
>> + fcf->fabric_name, fcf->fc_map, mtu_valid);
>> + } else {
>> + LIBFCOE_FIP_DBG("New FCF for fab %llx map %x val %d\n",
>> + fcf->fabric_name, fcf->fc_map, mtu_valid);
>> + }
>>
>> /*
>> * If this advertisement is not solicited and our max receive size
>> @@ -794,7 +812,8 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip,
>> struct sk_buff *skb)
>> ((struct fip_mac_desc *)desc)->fd_mac,
>> ETH_ALEN);
>> if (!is_valid_ether_addr(granted_mac)) {
>> - FIP_DBG("invalid MAC addrs in FIP ELS\n");
>> + LIBFCOE_FIP_DBG("Invalid MAC address "
>> + "in FIP ELS\n");
>> goto drop;
>> }
>> break;
>> @@ -812,8 +831,8 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip,
>> struct sk_buff *skb)
>> els_dtype = desc->fip_dtype;
>> break;
>> default:
>> - FIP_DBG("unexpected descriptor type %x "
>> - "in FIP adv\n", desc->fip_dtype);
>> + LIBFCOE_FIP_DBG("unexpected descriptor type %x "
>> + "in FIP adv\n", desc->fip_dtype);
>> /* standard says ignore unknown descriptors >= 128 */
>> if (desc->fip_dtype < FIP_DT_VENDOR_BASE)
>> goto drop;
>> @@ -854,8 +873,8 @@ static void fcoe_ctlr_recv_els(struct fcoe_ctlr *fip,
>> struct sk_buff *skb)
>> return;
>>
>> len_err:
>> - FIP_DBG("FIP length error in descriptor type %x len %zu\n",
>> - desc->fip_dtype, dlen);
>> + LIBFCOE_FIP_DBG("FIP length error in descriptor type %x len %zu\n",
>> + desc->fip_dtype, dlen);
>> drop:
>> kfree_skb(skb);
>> }
>> @@ -881,7 +900,7 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr
>> *fip,
>> struct fc_lport *lp = fip->lp;
>> u32 desc_mask;
>>
>> - FIP_DBG("Clear Virtual Link received\n");
>> + LIBFCOE_FIP_DBG("Clear Virtual Link received\n");
>> if (!fcf)
>> return;
>> if (!fcf || !fc_host_port_id(lp->host))
>> @@ -939,9 +958,9 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr
>> *fip,
>> * reset only if all required descriptors were present and valid.
>> */
>> if (desc_mask) {
>> - FIP_DBG("missing descriptors mask %x\n", desc_mask);
>> + LIBFCOE_FIP_DBG("missing descriptors mask %x\n", desc_mask);
>> } else {
>> - FIP_DBG("performing Clear Virtual Link\n");
>> + LIBFCOE_FIP_DBG("performing Clear Virtual Link\n");
>> fcoe_ctlr_reset(fip, FIP_ST_ENABLED);
>> }
>> }
>> @@ -989,9 +1008,9 @@ static int fcoe_ctlr_recv_handler(struct fcoe_ctlr
>> *fip, struct sk_buff *skb)
>> op = ntohs(fiph->fip_op);
>> sub = fiph->fip_subcode;
>>
>> - FIP_DBG_LVL(2, "ver %x op %x/%x dl %x fl %x\n",
>> - FIP_VER_DECAPS(fiph->fip_ver), op, sub,
>> - ntohs(fiph->fip_dl_len), ntohs(fiph->fip_flags));
>> + LIBFCOE_FIP_DBG("ver %x op %x/%x dl %x fl %x\n",
>> + FIP_VER_DECAPS(fiph->fip_ver), op, sub,
>> + ntohs(fiph->fip_dl_len), ntohs(fiph->fip_flags));
>>
>> if (FIP_VER_DECAPS(fiph->fip_ver) != FIP_VER)
>> goto drop;
>> @@ -1004,7 +1023,7 @@ static int fcoe_ctlr_recv_handler(struct fcoe_ctlr
>> *fip, struct sk_buff *skb)
>> fip->map_dest = 0;
>> fip->state = FIP_ST_ENABLED;
>> state = FIP_ST_ENABLED;
>> - FIP_DBG("using FIP mode\n");
>> + LIBFCOE_FIP_DBG("using FIP mode\n");
>> }
>> spin_unlock_bh(&fip->lock);
>> if (state != FIP_ST_ENABLED)
>> @@ -1039,14 +1058,15 @@ static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
>> struct fcoe_fcf *best = NULL;
>>
>> list_for_each_entry(fcf, &fip->fcfs, list) {
>> - FIP_DBG("consider FCF for fab %llx VFID %d map %x val %d\n",
>> - fcf->fabric_name, fcf->vfid,
>> - fcf->fc_map, fcoe_ctlr_mtu_valid(fcf));
>> + LIBFCOE_FIP_DBG("consider FCF for fab %llx VFID %d map %x "
>> + "val %d\n", fcf->fabric_name, fcf->vfid,
>> + fcf->fc_map, fcoe_ctlr_mtu_valid(fcf));
>> if (!fcoe_ctlr_fcf_usable(fcf)) {
>> - FIP_DBG("FCF for fab %llx map %x %svalid %savailable\n",
>> - fcf->fabric_name, fcf->fc_map,
>> - (fcf->flags & FIP_FL_SOL) ? "" : "in",
>> - (fcf->flags & FIP_FL_AVAIL) ? "" : "un");
>> + LIBFCOE_FIP_DBG("FCF for fab %llx map %x %svalid "
>> + "%savailable\n", fcf->fabric_name,
>> + fcf->fc_map, (fcf->flags & FIP_FL_SOL)
>> + ? "" : "in", (fcf->flags & FIP_FL_AVAIL)
>> + ? "" : "un");
>> continue;
>> }
>> if (!best) {
>> @@ -1056,7 +1076,8 @@ static void fcoe_ctlr_select(struct fcoe_ctlr *fip)
>> if (fcf->fabric_name != best->fabric_name ||
>> fcf->vfid != best->vfid ||
>> fcf->fc_map != best->fc_map) {
>> - FIP_DBG("conflicting fabric, VFID, or FC-MAP\n");
>> + LIBFCOE_FIP_DBG("Conflicting fabric, VFID, "
>> + "or FC-MAP\n");
>> return;
>> }
>> if (fcf->pri < best->pri)
>> @@ -1100,7 +1121,7 @@ static void fcoe_ctlr_timeout(unsigned long arg)
>> if (sel != fcf) {
>> fcf = sel; /* the old FCF may have been freed */
>> if (sel) {
>> - printk(KERN_INFO "host%d: FIP selected "
>> + printk(KERN_INFO "libfcoe: host%d: FIP selected "
>> "Fibre-Channel Forwarder MAC %s\n",
>> fip->lp->host->host_no,
>> print_mac(buf, sel->fcf_mac));
>> @@ -1110,7 +1131,7 @@ static void fcoe_ctlr_timeout(unsigned long arg)
>> fip->ctlr_ka_time = jiffies + sel->fka_period;
>> fip->link = 1;
>> } else {
>> - printk(KERN_NOTICE "host%d: "
>> + printk(KERN_NOTICE "libfcoe: host%d: "
>> "FIP Fibre-Channel Forwarder timed out. "
>> "Starting FCF discovery.\n",
>> fip->lp->host->host_no);
>> @@ -1234,7 +1255,7 @@ int fcoe_ctlr_recv_flogi(struct fcoe_ctlr *fip, struct
>> fc_frame *fp, u8 *sa)
>> return -EINVAL;
>> }
>> fip->state = FIP_ST_NON_FIP;
>> - FIP_DBG("received FLOGI LS_ACC using non-FIP mode\n");
>> + LIBFCOE_FIP_DBG("received FLOGI LS_ACC using non-FIP mode\n");
>>
>> /*
>> * FLOGI accepted.
>> @@ -1263,7 +1284,7 @@ int fcoe_ctlr_recv_flogi(struct fcoe_ctlr *fip, struct
>> fc_frame *fp, u8 *sa)
>> memcpy(fip->dest_addr, sa, ETH_ALEN);
>> fip->map_dest = 0;
>> if (fip->state == FIP_ST_NON_FIP)
>> - FIP_DBG("received FLOGI REQ, "
>> + LIBFCOE_FIP_DBG("received FLOGI REQ, "
>> "using non-FIP mode\n");
>> fip->state = FIP_ST_NON_FIP;
>> }
>>
>> _______________________________________________
>> devel mailing list
>> [email protected]
>> http://www.open-fcoe.org/mailman/listinfo/devel
>
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
